Re: logical decoding and replication of sequences, take 2

From: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Subject: Re: logical decoding and replication of sequences, take 2
Date: 2023-11-29 13:28:47
Message-ID: 547f6488-2365-770d-6801-a676ba433e5f@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi!

Considering my findings about issues with the rd_newRelfilelocatorSubid
field and how it makes that approach impossible, I decided to rip out
those patches, and go back to the approach where reorderbuffer tracks
new relfilenodes. This means the open questions I listed two days ago
disappear, because all of that was about the alternative approach.

I've also added a couple more tests into 034_sequences.pl, testing the
basic cases with substransactions that rollback (or not), etc. The
attached patch also addresses the review comments by Peter Smith.

The one remaining open question is ReorderBufferSequenceIsTransactional
and whether it can do better than searching through all top-level
transactions. The idea of 0002 was to only search the current top-level
xact, but Amit pointed out we can't rely on seeing the assignment until
we know we're in a consistent snapshot.

I'm yet to try doing some tests to measure how expensive this lookup can
be in practice. But let's assume it's measurable and significant enough
to matter. I wonder if we could salvage this optimization somehow. I'm
thinking about three options:

1) Could ReorderBufferSequenceIsTransactional check the snapshot is
already consistent etc. and use the optimized variant (looking only at
the same top-level xact) in that case? And if not, fallback to the
search of all top-level xacts. In practice, the full search would be
used only for a short initial period.

2) We could also make ReorderBufferSequenceIsTransactional to always
check the same top-level transaction first and then fallback, no matter
whether the snapshot is consistent or not. The problem is this doesn't
really optimize the common case where there are no new relfilenodes, so
we won't find a match in the top-level xact, and will always search
everything anyway.

3) Alternatively, we could maintain a global hash table, instead of in
the top-level transaction. So there'd always be two copies, one in the
xact itself and then in the global hash. Now there's either one (in
current top-level xact), or two (subxact + top-level xact).

I kinda like (3), because it just works and doesn't require the snapshot
being consistent etc.

Opinions?

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
v20231128-0001-Logical-decoding-of-sequences.patch text/x-patch 68.9 KB
v20231128-0002-tweak-ReorderBufferSequenceIsTransactional.patch text/x-patch 4.9 KB
v20231128-0003-Add-decoding-of-sequences-to-test_decoding.patch text/x-patch 20.4 KB
v20231128-0004-Add-decoding-of-sequences-to-built-in-repl.patch text/x-patch 269.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2023-11-29 13:31:21 Re: Change GUC hashtable to use simplehash?
Previous Message Andrew Dunstan 2023-11-29 13:23:53 Re: Python installation selection in Meson