On Tue, Oct 18, 2022 at 7:56 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Oct 17, 2022 at 7:05 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > On Thu, Oct 13, 2022 at 4:08 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > --- a/src/backend/replication/logical/decode.c > > > +++ b/src/backend/replication/logical/decode.c > > > @@ -113,6 +113,15 @@ > > > LogicalDecodingProcessRecord(LogicalDecodingContext *ctx, > > > XLogReaderState *recor > > > buf.origptr); > > > } > > > > > > +#ifdef USE_ASSERT_CHECKING > > > + /* > > > + * Check the order of transaction LSNs when we reached the start decoding > > > + * LSN. See the comments in AssertTXNLsnOrder() for details. > > > + */ > > > + if (SnapBuildGetStartDecodingAt(ctx->snapshot_builder) == buf.origptr) > > > + AssertTXNLsnOrder(ctx->reorder); > > > +#endif > > > + > > > rmgr = GetRmgr(XLogRecGetRmid(record)); > > > > > > > > > > I am not able to think how/when this check will be useful. Because we > > > skipped assert checking only for records that are prior to > > > start_decoding_at point, I think for those records ordering should > > > have been checked before the restart. start_decoding_at point will be > > > either (a) confirmed_flush location, or (b) lsn sent by client, and > > > any record prior to that must have been processed before restart. > > > > Good point. I was considering the case where the client sets far ahead > > LSN but it's not worth considering this case in this context. I've > > updated the patch accoringly. > > > > One minor comment: > Can we slightly change the comment: ". The ordering of the records > prior to the LSN, we should have been checked before the restart." to > ". The ordering of the records prior to the start_decoding_at LSN > should have been checked before the restart."?
Agreed. I'll update the patch accordingly. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
