Horton

Re: logical decoding and replication of sequences

Thomas Munro <thomas.munro@gmail.com>

View thread View on postgr.es
From: Thomas Munro <thomas.munro@gmail.com>
To: Noah Misch <noah@leadboat.com>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>, Amit Kapila <amit.kapila16@gmail.com>, Petr Jelinek <petr.jelinek@enterprisedb.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-08-08T06:15:46Z
Lists: pgsql-hackers

Attachments

On Mon, Aug 8, 2022 at 9:09 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> Thanks for the repro patch and bisection work.  Looking...

I don't have the complete explanation yet, but it's something like
this.  We hit the following branch in xlogrecovery.c...

        if (StandbyMode &&
                !XLogReaderValidatePageHeader(xlogreader,
targetPagePtr, readBuf))
        {
                /*
                 * Emit this error right now then retry this page
immediately. Use
                 * errmsg_internal() because the message was already translated.
                 */
                if (xlogreader->errormsg_buf[0])
                        ereport(emode_for_corrupt_record(emode,
xlogreader->EndRecPtr),
                                        (errmsg_internal("%s",
xlogreader->errormsg_buf)));

                /* reset any error XLogReaderValidatePageHeader()
might have set */
                xlogreader->errormsg_buf[0] = '\0';
                goto next_record_is_invalid;
        }

... but, even though there was a (suppressed) error, nothing
invalidates the reader's page buffer.  Normally,
XLogReadValidatePageHeader() failure or any other kind of error
encountered by xlogreader.c'd decoding logic would do that, but here
the read_page callback is directly calling the header validation.
Without prefetching, that doesn't seem to matter, but reading ahead
can cause us to have the problem page in our buffer at the wrong time,
and then not re-read it when we should.  Or something like that.

The attached patch that simply moves the cache invalidation into
report_invalid_record(), so that it's reached by the above code and
everything else that reports an error, seems to fix the problem in
src/bin/pg_ctl/t/003_promote.pl with Noah's spanner-in-the-works patch
applied, and passes check-world without it.  I need to look at this
some more, though, and figure out if it's the right fix.

Commits

  1. Fix cache invalidation bug in recovery_prefetch.

  2. Revert "Logical decoding of sequences"

  3. Minor improvements in sequence decoding code and docs

  4. Handle sequences in preprocess_pubobj_list

  5. Update tab-completion for CREATE PUBLICATION with sequences

  6. Add decoding of sequences to built-in replication

  7. Stabilize test_decoding touching with sequences

  8. Call ReorderBufferProcessXid from sequence_decode

  9. Add decoding of sequences to test_decoding

  10. Replace Test::More plans with done_testing

  11. Logical decoding of sequences

  12. Respect permissions within logical replication.