Horton

Re: logical decoding and replication of sequences, take 2

Tomas Vondra <tomas.vondra@enterprisedb.com>

View thread
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: 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>
Date: 2023-08-01T15:16:02Z
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 8/1/23 04:59, Amit Kapila wrote:
> On Mon, Jul 31, 2023 at 5:04 PM Tomas Vondra
> <tomas.vondra@enterprisedb.com> wrote:
>>
>> On 7/31/23 11:25, Amit Kapila wrote:
>>> ...
>>>
>>> Yeah, I also think this needs a review. This is a sort of new concept
>>> where we don't use the LSN of the slot (for cases where copy returned
>>> a larger value of LSN) or a full_snapshot created corresponding to the
>>> sync slot by Walsender. For the case of the table, we build a full
>>> snapshot because we use that for copying the table but why do we need
>>> to build that for copying the sequence especially when we directly
>>> copy it from the sequence relation without caring for any snapshot?
>>>
>>
>> We need the slot to decode/apply changes during catchup. The main
>> subscription may get ahead, and we need to ensure the WAL is not
>> discarded or something like that. This applies even if the initial sync
>> step does not use the slot/snapshot directly.
>>
> 
> AFAIK, none of these needs a full_snapshot (see usage of
> SnapBuild->building_full_snapshot). The full_snapshot tracks both
> catalog and non-catalog xacts in the snapshot where we require to
> track non-catalog ones because we want to copy the table using that
> snapshot. It is relatively expensive to build a full snapshot and we
> don't do that unless it is required. For the current usage of this
> patch, I think using CRS_NOEXPORT_SNAPSHOT would be sufficient.
> 

Yeah, you may be right we don't need a full snapshot, because we don't
need to export it. We however still need a snapshot, and it wasn't clear
to me whether you suggest we don't need the slot / snapshot at all.

Anyway, I think this is "just" a matter of efficiency, not correctness.
IMHO there are bigger questions regarding the "going back" behavior
after apply restart.

regards

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