Skip to content

Commit 43acadf

Browse files
author
Amit Kapila
committed
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 dd53b46 commit 43acadf

File tree

3 files changed

+48
-25
lines changed

3 files changed

+48
-25
lines changed

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

+6-8
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

+36-12
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,9 @@ ReorderBufferReturnTXN(ReorderBuffer *rb, ReorderBufferTXN *txn)
364364
txn->invalidations = NULL;
365365
}
366366

367+
/* Reset the toast hash */
368+
ReorderBufferToastReset(rb, txn);
369+
367370
/* check whether to put into the slab cache */
368371
if (rb->nr_cached_transactions < max_cached_transactions)
369372
{
@@ -449,6 +452,7 @@ ReorderBufferReturnChange(ReorderBuffer *rb, ReorderBufferChange *change)
449452
break;
450453
/* no data in addition to the struct itself */
451454
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM:
455+
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT:
452456
case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID:
453457
case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID:
454458
break;
@@ -1674,8 +1678,8 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
16741678
change_done:
16751679

16761680
/*
1677-
* Either speculative insertion was confirmed, or it was
1678-
* unsuccessful and the record isn't needed anymore.
1681+
* If speculative insertion was confirmed, the record isn't
1682+
* needed anymore.
16791683
*/
16801684
if (specinsert != NULL)
16811685
{
@@ -1717,6 +1721,32 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
17171721
specinsert = change;
17181722
break;
17191723

1724+
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT:
1725+
1726+
/*
1727+
* Abort for speculative insertion arrived. So cleanup the
1728+
* specinsert tuple and toast hash.
1729+
*
1730+
* Note that we get the spec abort change for each toast
1731+
* entry but we need to perform the cleanup only the first
1732+
* time we get it for the main table.
1733+
*/
1734+
if (specinsert != NULL)
1735+
{
1736+
/*
1737+
* We must clean the toast hash before processing a
1738+
* completely new tuple to avoid confusion about the
1739+
* previous tuple's toast chunks.
1740+
*/
1741+
Assert(change->data.tp.clear_toast_afterwards);
1742+
ReorderBufferToastReset(rb, txn);
1743+
1744+
/* We don't need this record anymore. */
1745+
ReorderBufferReturnChange(rb, specinsert);
1746+
specinsert = NULL;
1747+
}
1748+
break;
1749+
17201750
case REORDER_BUFFER_CHANGE_MESSAGE:
17211751
rb->message(rb, txn, change->lsn, true,
17221752
change->data.msg.prefix,
@@ -1792,16 +1822,8 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
17921822
}
17931823
}
17941824

1795-
/*
1796-
* There's a speculative insertion remaining, just clean in up, it
1797-
* can't have been successful, otherwise we'd gotten a confirmation
1798-
* record.
1799-
*/
1800-
if (specinsert)
1801-
{
1802-
ReorderBufferReturnChange(rb, specinsert);
1803-
specinsert = NULL;
1804-
}
1825+
/* speculative insertion record must be freed by now */
1826+
Assert(!specinsert);
18051827

18061828
/* clean up the iterator */
18071829
ReorderBufferIterTXNFinish(rb, iterstate);
@@ -2476,6 +2498,7 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
24762498
break;
24772499
}
24782500
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM:
2501+
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT:
24792502
case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID:
24802503
case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID:
24812504
/* ReorderBufferChange contains everything important */
@@ -2764,6 +2787,7 @@ ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
27642787
}
27652788
/* the base struct contains all the data, easy peasy */
27662789
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM:
2790+
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT:
27672791
case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID:
27682792
case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID:
27692793
break;

‎src/include/replication/reorderbuffer.h

+6-5
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)