public inbox for [email protected]
help / color / mirror / Atom feedFrom: Zhijie Hou (Fujitsu) <[email protected]>
To: Tomas Vondra <[email protected]>
Cc: Ashutosh Bapat <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Dilip Kumar <[email protected]>
Subject: RE: logical decoding and replication of sequences, take 2
Date: Tue, 24 Oct 2023 11:31:29 +0000
Message-ID: <OS0PR01MB5716328A2A85F612C963CAD694DFA@OS0PR01MB5716.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<CAA4eK1KAP0BmfZG4Pcvt6=vcdWT7p5J0-MtJySPhnfj4aDYE-g@mail.gmail.com>
<CAA4eK1L+2bie=Yj5VSZqTRnoRCsvH=5sGh4bD76fnqSkLTkNLQ@mail.gmail.com>
<[email protected]>
<CAExHW5uCUSNTn421eH5d2pVDh-DM=SJ3sERsKu-aQrP6ZPh08g@mail.gmail.com>
<[email protected]>
<CAA4eK1+ZrdEy1GW3cDTYG4tyNFihGbM8SyYFKn=JFaKgxxXrRQ@mail.gmail.com>
<[email protected]>
<CAA4eK1+KTL+hug2-Mcmcr_nsshwaLRjRa6arW79u5R8Jx49=uQ@mail.gmail.com>
<[email protected]>
<CAExHW5vN=aVYOFikt6cqpsezwYErGW6aqaDJ0jUdu++B0eA0Dw@mail.gmail.com>
<[email protected]>
<CAFiTN-sA0fs6xAsqDGOO7i5avz-jQ-i9-xZR4Wk7J9C=4n0BQg@mail.gmail.com>
<CAFiTN-tr6571vBph5fGLqCEuOpeU-soau0nZ55AvZbY2o02W0A@mail.gmail.com>
<[email protected]>
On Thursday, October 12, 2023 11:06 PM Tomas Vondra <[email protected]> wrote:
>
Hi,
I have been reviewing the patch set, and here are some initial comments.
1.
I think we need to mark the RBTXN_HAS_STREAMABLE_CHANGE flag for transactional
sequence change in ReorderBufferQueueChange().
2.
ReorderBufferSequenceIsTransactional
It seems we call the above function once in sequence_decode() and call it again
in ReorderBufferQueueSequence(), would it better to avoid the second call as
the hashtable search looks not cheap.
3.
The patch cleans up the sequence hash table when COMMIT or ABORT a transaction
(via ReorderBufferAbort() and ReorderBufferReturnTXN()), while it doesn't seem
destory the hash table when PREPARE the transaction. It's not a big porblem but
would it be better to release the memory earlier by destory the table for
prepare ?
4.
+pg_decode_stream_sequence(LogicalDecodingContext *ctx, ReorderBufferTXN *txn,
...
+ /* output BEGIN if we haven't yet, but only for the transactional case */
+ if (transactional)
+ {
+ if (data->skip_empty_xacts && !txndata->xact_wrote_changes)
+ {
+ pg_output_begin(ctx, data, txn, false);
+ }
+ txndata->xact_wrote_changes = true;
+ }
I think we should call pg_output_stream_start() instead of pg_output_begin()
for streaming sequence changes.
5.
+ /*
+ * Schema should be sent using the original relation because it
+ * also sends the ancestor's relation.
+ */
+ maybe_send_schema(ctx, txn, relation, relentry);
The comment seems a bit misleading here, I think it was used for the partition
logic in pgoutput_change().
Best Regards,
Hou zj
view thread (46+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: RE: logical decoding and replication of sequences, take 2
In-Reply-To: <OS0PR01MB5716328A2A85F612C963CAD694DFA@OS0PR01MB5716.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox