Re: logical decoding and replication of sequences, take 2

From: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
To: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(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-27 01:11:14
Message-ID: 28e0279f-d02a-f7d7-568d-13f70ee8f60b@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

I've been cleaning up the first two patches to get them committed soon
(adding the decoding infrastructure + test_decoding), cleaning up stale
comments, updating commit messages etc. And I think it's ready to go,
but it's too late over, so I plan going over once more tomorrow and then
likely push. But if someone wants to take a look, I'd welcome that.

The one issue I found during this cleanup is that the patch was missing
the changes introduced by 29d0a77fa660 for decoding of other stuff.

commit 29d0a77fa6606f9c01ba17311fc452dabd3f793d
Author: Amit Kapila <akapila(at)postgresql(dot)org>
Date: Thu Oct 26 06:54:16 2023 +0530

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

I fixed that, but perhaps someone might want to double check ...

0003 is here just for completeness - that's the part adding sequences to
built-in replication. I haven't done much with it, it needs some cleanup
too to get it committable. I don't intend to push that right after
0001+0002, though.

While going over 0001, I realized there might be an optimization for
ReorderBufferSequenceIsTransactional. As coded in 0001, it always
searches through all top-level transactions, and if there's many of them
that might be expensive, even if very few of them have any relfilenodes
in the hash table. It's still linear search, and it needs to happen for
each sequence change.

But can the relfilenode even be in some other top-level transaction? How
could it be - our transaction would not see it, and wouldn't be able to
generate the sequence change. So we should be able to simply check *our*
transaction (or if it's a subxact, the top-level transaction). Either
it's there (and it's transactional change), or not (and then it's
non-transactional change). The 0004 does this.

This of course hinges on when exactly the transactions get created, and
assignments processed. For example if this would fire before the txn
gets assigned to the top-level one, this would break. I don't think this
can happen thanks to the immediate logging of assignments, but I'm too
tired to think about it now.

regards

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

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2023-11-27 01:22:38 Re: pg_upgrade and logical replication
Previous Message Alexander Korotkov 2023-11-27 01:04:38 Re: Assert failure on 'list_member_ptr(rel->joininfo, restrictinfo)'