8000 Fix decoding of speculative aborts. · postgres/postgres@35f56c1 · GitHub
[go: up one dir, main page]
Skip to content

Commit 35f56c1

Browse files
author
Amit Kapila
committ 8000 ed
Fix decoding of speculative aborts.
During decoding for speculative inserts, we were relying for cleaning toast hash on confirmation records or next change records. But that could lead to multiple problems (a) memory leak if there is neither a confirmation record nor any other record after toast insertion for a speculative insert in the transaction, (b) error and assertion failures if the next operation is not an insert/update on the same table. The fix is to start queuing spec abort change and clean up toast hash and change record during its processing. Currently, we are queuing the spec aborts for both toast and main table even though we perform cleanup while processing the main table's spec abort record. Later, if we have a way to distinguish between the spec abort record of toast and the main table, we can avoid queuing the change for spec aborts of toast tables. Reported-by: Ashutosh Bapat Author: Dilip Kumar Reviewed-by: Amit Kapila Backpatch-through: 9.6, where it was introduced Discussion: https://postgr.es/m/CAExHW5sPKF-Oovx_qZe4p5oM6Dvof7_P+XgsNAViug15Fm99jA@mail.gmail.com
1 parent 05fccab commit 35f56c1

File tree

3 files changed

+48
-25
lines changed

3 files changed

+48
-25
lines changed

‎src/backend/replication/logical/decode.c

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -778,19 +778,17 @@ DecodeDelete(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
778778
if (target_node.dbNode != ctx->slot->data.database)
779779
return;
780780

781-
/*
782-
* Super deletions are irrelevant for logical decoding, it's driven by the
783-
* confirmation records.
784-
*/
785-
if (xlrec->flags & XLH_DELETE_IS_SUPER)
786-
return;
787-
788781
/* output plugin doesn't look for this origin, no need to queue */
789782
if (FilterByOrigin(ctx, XLogRecGetOrigin(r)))
790783
return;
791784

792785
change = ReorderBufferGetChange(ctx->reorder);
793-
change->action = REORDER_BUFFER_CHANGE_DELETE;
786+
787+
if (xlrec->flags & XLH_DELETE_IS_SUPER)
788+
change->action = REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT;
789+
else
790+
change->action = REORDER_BUFFER_CHANGE_DELETE;
791+
794792
change->origin_id = XLogRecGetOrigin(r);
795793

796794
memcpy(&change->data.tp.relnode, &target_node, sizeof(RelFileNode));

‎src/backend/replication/logical/reorderbuffer.c

Lines changed: 36 additions & 12 deletions
< 10000 tr class="diff-line-row">
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,9 @@ ReorderBufferReturnTXN(ReorderBuffer *rb, ReorderBufferTXN *txn)
353353
txn->invalidations = NULL;
354354
}
355355

356+
/* Reset the toast hash */
357+
ReorderBufferToastReset(rb, txn);
358+
356359
pfree(txn);
357360
}
358361

@@ -413,6 +416,7 @@ ReorderBufferReturnChange(ReorderBuffer *rb, ReorderBufferChange *change)
413416
break;
414417
/* no data in addition to the struct itself */
415418
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM:
419+
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT:
416420
case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID:
417421
case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID:
418422
break;
@@ -1621,8 +1625,8 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
16211625
change_done:
16221626

16231627
/*
1624-
* Either speculative insertion was confirmed, or it was
1625-
* unsuccessful and the record isn't needed anymore.
1628+
* If speculative insertion was confirmed, the record isn't
1629+
* needed anymore.
16261630
*/
16271631
if (specinsert != NULL)
16281632
{
@@ -1664,6 +1668,32 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
16641668
specinsert = change;
16651669
break;
16661670

1671+
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT:
1672+
1673+
/*
1674+
* Abort for speculative insertion arrived. So cleanup the
1675+
* specinsert tuple and toast hash.
1676+
*
1677+
* Note that we get the spec abort change for each toast
1678+
* entry but we need to perform the cleanup only the first
1679+
* time we get it for the main table.
1680+
*/
1681+
if (specinsert != NULL)
1682+
{
1683+
/*
1684+
* We must clean the toast hash before processing a
1685+
* completely new tuple to avoid confusion about the
1686+
* previous tuple's toast chunks.
1687+
*/
1688+
Assert(change->data.tp.clear_toast_afterwards);
1689+
ReorderBufferToastReset(rb, txn);
1690+
1691+
/* We don't need this record anymore. */
1692+
ReorderBufferReturnChange(rb, specinsert);
1693+
specinsert = NULL;
1694+
}
1695+
break;
1696+
16671697
case REORDER_BUFFER_CHANGE_MESSAGE:
16681698
rb->message(rb, txn, change->lsn, true,
16691699
change->data.msg.prefix,
@@ -1739,16 +1769,8 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
17391769
}
17401770
}
17411771

1742-
/*
1743-
* There's a speculative insertion remaining, just clean in up, it
1744-
* can't have been successful, otherwise we'd gotten a confirmation
1745-
* record.
1746-
*/
1747-
if (specinsert)
1748-
{
1749-
ReorderBufferReturnChange(rb, specinsert);
1750-
specinsert = NULL;
1751-
}
1772+
/* speculative insertion record must be freed by now */
1773+
Assert(!specinsert);
17521774

17531775
/* clean up the iterator */
17541776
ReorderBufferIterTXNFinish(rb, iterstate);
@@ -2423,6 +2445,7 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
24232445
break;
24242446
}
24252447
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM:
2448+
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT:
24262449
case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID:
24272450
case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID:
24282451
/* ReorderBufferChange contains everything important */
@@ -2716,6 +2739,7 @@ ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
27162739
}
27172740
/* the base struct contains all the data, easy peasy */
27182741
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM:
2742+
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT:
27192743
case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID:
27202744
case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID:
27212745
break;

‎src/include/replication/reorderbuffer.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ typedef struct ReorderBufferTupleBuf
4444
* changes. Users of the decoding facilities will never see changes with
4545
* *_INTERNAL_* actions.
4646
*
47-
* The INTERNAL_SPEC_INSERT and INTERNAL_SPEC_CONFIRM changes concern
48-
* "speculative insertions", and their confirmation respectively. They're
49-
* used by INSERT .. ON CONFLICT .. UPDATE. Users of logical decoding don't
50-
* have to care about these.
47+
* The INTERNAL_SPEC_INSERT and INTERNAL_SPEC_CONFIRM, and INTERNAL_SPEC_ABORT
48+
* changes concern "speculative insertions", their confirmation, and abort
49+
* respectively. They're used by INSERT .. ON CONFLICT .. UPDATE. Users of
50+
* logical decoding don't have to care about these.
5151
*/
5252
enum ReorderBufferChangeType
5353
{
@@ -59,7 +59,8 @@ enum ReorderBufferChangeType
5959
REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID,
6060
REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID,
6161
REORDER_BUFFER_CHANGE_INTERNAL_SPEC_INSERT,
62-
REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM
62+
REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM,
63+
REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT
6364
};
6465

6566
/*

0 commit comments

Comments
 (0)
0