From 27b4def5b0cdc3df8a1f32058e65021db6b9c06c Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Thu, 16 Mar 2023 17:03:19 +0100 Subject: [PATCH 6/6] john's review --- src/backend/replication/logical/decode.c | 2 +- .../replication/logical/reorderbuffer.c | 20 ++++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c index aece336612e..cd2f88e0bde 100644 --- a/src/backend/replication/logical/decode.c +++ b/src/backend/replication/logical/decode.c @@ -1359,7 +1359,7 @@ sequence_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) /* extract the WAL record, with "created" flag */ xlrec = (xl_seq_rec *) XLogRecGetData(r); - /* XXX how could we have sequence change without data? */ + /* the sequence should not have changed without data */ if(!datalen || !tupledata) elog(ERROR, "sequence decode missing tuple data"); diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 0c8cca3029c..16fcbed42c9 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -95,10 +95,10 @@ * * To differentiate which sequences are "old" and which were created * in a still-running transaction, we track sequences created in running - * transactions in a hash table. Sequences are identified by relfilenode, + * transactions in a hash table. Sequences are identified by relfilelocator, * and we track XID of the (sub)transaction that created it. This means - * that if a transaction does something that changes the relfilenode - * (like an alter / reset of a sequence), the new relfilenode will be + * that if a transaction does something that changes the relfilelocator + * (like an alter / reset of a sequence), the new relfilelocator will be * treated as if created in the transaction. The list of sequences gets * discarded when the transaction completes (commit/rollback). * @@ -391,7 +391,7 @@ ReorderBufferAllocate(void) buffer->by_txn = hash_create("ReorderBufferByXid", 1000, &hash_ctl, HASH_ELEM | HASH_BLOBS | HASH_CONTEXT); - /* hash table of sequences, mapping relfilenode to XID of transaction */ + /* hash table of sequences, mapping relfilelocator to XID of transaction */ hash_ctl.keysize = sizeof(RelFileLocator); hash_ctl.entrysize = sizeof(ReorderBufferSequenceEnt); hash_ctl.hcxt = buffer->context; @@ -1043,7 +1043,7 @@ ReorderBufferQueueSequence(ReorderBuffer *rb, TransactionId xid, MemoryContext oldcontext; ReorderBufferChange *change; - /* lookup sequence by relfilenode */ + /* lookup sequence by relfilelocator */ ReorderBufferSequenceEnt *ent; bool found; @@ -1079,9 +1079,7 @@ ReorderBufferQueueSequence(ReorderBuffer *rb, TransactionId xid, if (created) ent->xid = xid; - /* XXX Maybe check that we're still in the same top-level xact? */ - - /* OK, allocate and queue the change */ + /* allocate and queue the transactional sequence change */ oldcontext = MemoryContextSwitchTo(rb->context); change = ReorderBufferGetChange(rb); @@ -2319,7 +2317,11 @@ ReorderBufferApplySequence(ReorderBuffer *rb, ReorderBufferTXN *txn, tuple = &change->data.sequence.tuple->tuple; seq = (Form_pg_sequence_data) GETSTRUCT(tuple); - /* Only ever called from ReorderBufferApplySequence, so transational. */ + /* + * When called from ReorderBufferApplySequence, we're applying changes + * accumulated in a ReorderBufferTXN, so all those are transactional + * changes of sequences. + */ if (streaming) rb->stream_sequence(rb, txn, change->lsn, relation, true, seq->last_value, seq->log_cnt, seq->is_called); -- 2.39.2