Horton

Re: logical decoding and replication of sequences, take 2

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Masahiko Sawada <sawada.mshk@gmail.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Dilip Kumar <dilipbalaut@gmail.com>
Date: 2023-11-28T11:32:03Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Migrate logical slots to the new node during an upgrade.

  2. Make test_decoding ddl.out shorter

  3. Fix snapshot handling in logicalmsg_decode

  4. doc: Adjust a few more references to "postmaster"

  5. Revert "Logical decoding of sequences"

On Mon, Nov 27, 2023 at 11:45 PM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
>
> I spent a bit of time looking at the proposed change, and unfortunately
> logging just the boolean flag does not work. A good example is this bit
> from a TAP test added by the patch for built-in replication (which was
> not included with the WIP patch):
>
>   BEGIN;
>   ALTER SEQUENCE s RESTART WITH 1000;
>   SAVEPOINT sp1;
>   INSERT INTO seq_test SELECT nextval('s') FROM generate_series(1,100);
>   ROLLBACK TO sp1;
>   COMMIT;
>
> This is expected to produce:
>
>   1131|0|t
>
> but produces
>
>   1000|0|f
>
> instead. The reason is very simple - as implemented, the patch simply
> checks if the relfilenode is from the same top-level transaction, which
> it is, and sets the flag to "true". So we know the sequence changes need
> to be queued and replayed as part of this transaction.
>
> But then during decoding, we still queue the changes into the subxact,
> which then aborts, and the changes are discarded. That is not how it's
> supposed to work, because the new relfilenode is still valid, someone
> might do nextval() and commit. And the nextval() may not get WAL-logged,
> so we'd lose this.
>
> What I guess we might do is log not just a boolean flag, but the XID of
> the subtransaction that created the relfilenode. And then during
> decoding we'd queue the changes into this subtransaction ...
>
> 0006 in the attached patch series does this, and it seems to fix the TAP
> test failure. I left it at the end, to make it easier to run tests
> without the patch applied.
>

Offhand, I don't have any better idea than what you have suggested for
the problem but this needs some thoughts including the questions asked
by you. I'll spend some time on it and respond back.

-- 
With Regards,
Amit Kapila.