From f607e140cae21183fea2fc226029d7673478509e Mon Sep 17 00:00:00 2001 From: Dilip Kumar Date: Tue, 1 Jun 2021 19:53:47 +0530 Subject: [PATCH v2] Bug fix for speculative abort If speculative insert has a toast table insert then if that tuple is not confirmed then the toast hash is not cleaned and that is creating various problem like a) memory leak b) next insert is using these uncleaned toast data for its insertion and other error and assersion failure. So this patch handle that by queuing the spec abort changes and cleaning up the toast hash on spec abort. Currently, in this patch we are queuing up all the spec abort changes, but as an optimization we can avoid queuing the spec abort for toast tables but for that we need to log that as a flag in WAL. --- src/backend/replication/logical/decode.c | 16 +++++++----- src/backend/replication/logical/reorderbuffer.c | 34 +++++++++++++++++++++++++ src/include/replication/reorderbuffer.h | 1 + 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c index 7067016..1a0d7dc 100644 --- a/src/backend/replication/logical/decode.c +++ b/src/backend/replication/logical/decode.c @@ -1040,19 +1040,21 @@ DecodeDelete(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) if (target_node.dbNode != ctx->slot->data.database) return; + /* output plugin doesn't look for this origin, no need to queue */ + if (FilterByOrigin(ctx, XLogRecGetOrigin(r))) + return; + + change = ReorderBufferGetChange(ctx->reorder); + /* * Super deletions are irrelevant for logical decoding, it's driven by the * confirmation records. */ if (xlrec->flags & XLH_DELETE_IS_SUPER) - return; - - /* output plugin doesn't look for this origin, no need to queue */ - if (FilterByOrigin(ctx, XLogRecGetOrigin(r))) - return; + change->action = REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT; + else + change->action = REORDER_BUFFER_CHANGE_DELETE; - change = ReorderBufferGetChange(ctx->reorder); - change->action = REORDER_BUFFER_CHANGE_DELETE; change->origin_id = XLogRecGetOrigin(r); memcpy(&change->data.tp.relnode, &target_node, sizeof(RelFileNode)); diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 2d9e127..4940cf5 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -520,6 +520,7 @@ ReorderBufferReturnChange(ReorderBuffer *rb, ReorderBufferChange *change, } break; case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM: + case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT: case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID: case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID: break; @@ -2254,6 +2255,36 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, specinsert = change; break; + case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT: + + /* + * Abort for speculative insertion arrived. So cleanup the + * specinsert tuple and toast hash. If spec insert change + * is NULL then do nothing, this is possible because we + * have spec abort for each toast entry. So we just have + * to clean the specinsert and toast hash for the first + * spec abort for the main table and for the remaining + * entries we can just ignore. + * + * XXX For optimization, we may log a flag saying this is + * a spec abort for the toast table and we can avoid queuing + * that change. + */ + if (specinsert != NULL) + { + /* Clear the toast chunk */ + Assert(change->data.tp.clear_toast_afterwards); + ReorderBufferToastReset(rb, txn); + + /* + * If the speculative insertion was aborted, the record + * isn't needed anymore. + */ + ReorderBufferReturnChange(rb, specinsert, true); + specinsert = NULL; + } + break; + case REORDER_BUFFER_CHANGE_TRUNCATE: { int i; @@ -3754,6 +3785,7 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn, break; } case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM: + case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT: case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID: case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID: /* ReorderBufferChange contains everything important */ @@ -4017,6 +4049,7 @@ ReorderBufferChangeSize(ReorderBufferChange *change) break; } case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM: + case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT: case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID: case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID: /* ReorderBufferChange contains everything important */ @@ -4315,6 +4348,7 @@ ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn, break; } case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM: + case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT: case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID: case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID: break; diff --git a/src/include/replication/reorderbuffer.h b/src/include/replication/reorderbuffer.h index 0c6e9d1..9ff0986 100644 --- a/src/include/replication/reorderbuffer.h +++ b/src/include/replication/reorderbuffer.h @@ -63,6 +63,7 @@ enum ReorderBufferChangeType REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID, REORDER_BUFFER_CHANGE_INTERNAL_SPEC_INSERT, REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM, + REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT, REORDER_BUFFER_CHANGE_TRUNCATE }; -- 1.8.3.1