Skip to content

Commit 4daa140

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 0a1e80c commit 4daa140

File tree

3 files changed

+48
-24
lines changed

3 files changed

+48
-24
lines changed

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

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

1043-
/*
1044-
* Super deletions are irrelevant for logical decoding, it's driven by the
1045-
* confirmation records.
1046-
*/
1047-
if (xlrec->flags & XLH_DELETE_IS_SUPER)
1048-
return;
1049-
10501043
/* output plugin doesn't look for this origin, no need to queue */
10511044
if (FilterByOrigin(ctx, XLogRecGetOrigin(r)))
10521045
return;
10531046

10541047
change = ReorderBufferGetChange(ctx->reorder);
1055-
change->action = REORDER_BUFFER_CHANGE_DELETE;
1048+
1049+
if (xlrec->flags & XLH_DELETE_IS_SUPER)
1050+
change->action = REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT;
1051+
else
1052+
change->action = REORDER_BUFFER_CHANGE_DELETE;
1053+
10561054
change->origin_id = XLogRecGetOrigin(r);
10571055

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

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

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,9 @@ ReorderBufferReturnTXN(ReorderBuffer *rb, ReorderBufferTXN *txn)
443443
txn->invalidations = NULL;
444444
}
445445

446+
/* Reset the toast hash */
447+
ReorderBufferToastReset(rb, txn);
448+
446449
pfree(txn);
447450
}
448451

@@ -520,6 +523,7 @@ ReorderBufferReturnChange(ReorderBuffer *rb, ReorderBufferChange *change,
520523
}
521524
break;
522525
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM:
526+
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT:
523527
case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID:
524528
case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID:
525529
break;
@@ -2211,8 +2215,8 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
22112215
change_done:
22122216

22132217
/*
2214-
* Either speculative insertion was confirmed, or it was
2215-
* unsuccessful and the record isn't needed anymore.
2218+
* If speculative insertion was confirmed, the record isn't
2219+
* needed anymore.
22162220
*/
22172221
if (specinsert != NULL)
22182222
{
@@ -2254,6 +2258,32 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
22542258
specinsert = change;
22552259
break;
22562260

2261+
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT:
2262+
2263+
/*
2264+
* Abort for speculative insertion arrived. So cleanup the
2265+
* specinsert tuple and toast hash.
2266+
*
2267+
* Note that we get the spec abort change for each toast
2268+
* entry but we need to perform the cleanup only the first
2269+
* time we get it for the main table.
2270+
*/
2271+
if (specinsert != NULL)
2272+
{
2273+
/*
2274+
* We must clean the toast hash before processing a
2275+
* completely new tuple to avoid confusion about the
2276+
* previous tuple's toast chunks.
2277+
*/
2278+
Assert(change->data.tp.clear_toast_afterwards);
2279+
ReorderBufferToastReset(rb, txn);
2280+
2281+
/* We don't need this record anymore. */
2282+
ReorderBufferReturnChange(rb, specinsert, true);
2283+
specinsert = NULL;
2284+
}
2285+
break;
2286+
22572287
case REORDER_BUFFER_CHANGE_TRUNCATE:
22582288
{
22592289
int i;
@@ -2360,16 +2390,8 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
23602390
}
23612391
}
23622392

2363-
/*
2364-
* There's a speculative insertion remaining, just clean in up, it
2365-
* can't have been successful, otherwise we'd gotten a confirmation
2366-
* record.
2367-
*/
2368-
if (specinsert)
2369-
{
2370-
ReorderBufferReturnChange(rb, specinsert, true);
2371-
specinsert = NULL;
2372-
}
2393+
/* speculative insertion record must be freed by now */
2394+
Assert(!specinsert);
23732395

23742396
/* clean up the iterator */
23752397
ReorderBufferIterTXNFinish(rb, iterstate);
@@ -3754,6 +3776,7 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
37543776
break;
37553777
}
37563778
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM:
3779+
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT:
37573780
case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID:
37583781
case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID:
37593782
/* ReorderBufferChange contains everything important */
@@ -4017,6 +4040,7 @@ ReorderBufferChangeSize(ReorderBufferChange *change)
40174040
break;
40184041
}
40194042
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM:
4043+
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT:
40204044
case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID:
40214045
case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID:
40224046
/* ReorderBufferChange contains everything important */
@@ -4315,6 +4339,7 @@ ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
43154339
break;
43164340
}
43174341
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM:
4342+
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT:
43184343
case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID:
43194344
case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID:
43204345
break;

‎src/include/replication/reorderbuffer.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,10 @@ typedef struct ReorderBufferTupleBuf
4646
* changes. Users of the decoding facilities will never see changes with
4747
* *_INTERNAL_* actions.
4848
*
49-
* The INTERNAL_SPEC_INSERT and INTERNAL_SPEC_CONFIRM changes concern
50-
* "speculative insertions", and their confirmation respectively. They're
51-
* used by INSERT .. ON CONFLICT .. UPDATE. Users of logical decoding don't
52-
* have to care about these.
49+
* The INTERNAL_SPEC_INSERT and INTERNAL_SPEC_CONFIRM, and INTERNAL_SPEC_ABORT
50+
* changes concern "speculative insertions", their confirmation, and abort
51+
* respectively. They're used by INSERT .. ON CONFLICT .. UPDATE. Users of
52+
* logical decoding don't have to care about these.
5353
*/
5454
enum ReorderBufferChangeType
5555
{
@@ -63,6 +63,7 @@ enum ReorderBufferChangeType
6363
REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID,
6464
REORDER_BUFFER_CHANGE_INTERNAL_SPEC_INSERT,
6565
REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM,
66+
REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT,
6667
REORDER_BUFFER_CHANGE_TRUNCATE
6768
};
6869

0 commit comments

Comments
 (0)