On 11/19/18 11:44 AM, Tomas Vondra wrote:
> On 11/19/18 10:28 AM, Masahiko Sawada wrote:
>> On Mon, Nov 19, 2018 at 6:52 AM Tomas Vondra
>> <tomas.von...@2ndquadrant.com> wrote:
>>>
>>> ...
>>>
>>> This does fix the issue, because we still decode the TOAST changes but
>>> there are no data and so
>>>
>>> if (change->data.tp.newtuple != NULL)
>>> {
>>> dlist_delete(&change->node);
>>> ReorderBufferToastAppendChunk(rb, txn, relation,
>>> change);
>>> }
>>>
>>> ends up not stashing the change in the hash table.
>>
>> With the below change you proposed can we remove the above condition
>> because toast-insertion changes being processed by the reorder buffer
>> always have a new tuple? If a toast-insertion record doesn't have a
>> new tuple it has already ignored when decoding.
>>
>
> Good point. I think you're right the reorderbuffer part may be
> simplified as you propose.
>
OK, here's an updated patch, tweaking the reorderbuffer part. I plan to
push this sometime mid next week.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index d5bd282f8c..44caeca336 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -659,12 +659,11 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
options |= HEAP_INSERT_SKIP_WAL;
/*
- * The new relfilenode's relcache entrye doesn't have the necessary
- * information to determine whether a relation should emit data for
- * logical decoding. Force it to off if necessary.
+ * While rewriting the heap for VACUUM FULL / CLUSTER, make sure data
+ * for the TOAST table are not logically decoded. The main heap is
+ * WAL-logged as XLOG FPI records, which are not logically decoded.
*/
- if (!RelationIsLogicallyLogged(state->rs_old_rel))
- options |= HEAP_INSERT_NO_LOGICAL;
+ options |= HEAP_INSERT_NO_LOGICAL;
heaptup = toast_insert_or_update(state->rs_new_rel, tup, NULL,
options);
diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
index afb497227e..e3b05657f8 100644
--- a/src/backend/replication/logical/decode.c
+++ b/src/backend/replication/logical/decode.c
@@ -665,6 +665,9 @@ DecodeAbort(LogicalDecodingContext *ctx, XLogRecordBuffer *buf,
static void
DecodeInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
{
+ Size datalen;
+ char *tupledata;
+ Size tuplelen;
XLogReaderState *r = buf->record;
xl_heap_insert *xlrec;
ReorderBufferChange *change;
@@ -672,6 +675,13 @@ DecodeInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
xlrec = (xl_heap_insert *) XLogRecGetData(r);
+ /*
+ * Ignore insert records without new tuples (this does happen when
+ * raw_heap_insert marks the TOAST record as HEAP_INSERT_NO_LOGICAL).
+ */
+ if (!(xlrec->flags & XLH_INSERT_CONTAINS_NEW_TUPLE))
+ return;
+
/* only interested in our database */
XLogRecGetBlockTag(r, 0, &target_node, NULL, NULL);
if (target_node.dbNode != ctx->slot->data.database)
@@ -690,17 +700,13 @@ DecodeInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
memcpy(&change->data.tp.relnode, &target_node, sizeof(RelFileNode));
- if (xlrec->flags & XLH_INSERT_CONTAINS_NEW_TUPLE)
- {
- Size datalen;
- char *tupledata = XLogRecGetBlockData(r, 0, &datalen);
- Size tuplelen = datalen - SizeOfHeapHeader;
+ tupledata = XLogRecGetBlockData(r, 0, &datalen);
+ tuplelen = datalen - SizeOfHeapHeader;
- change->data.tp.newtuple =
- ReorderBufferGetTupleBuf(ctx->reorder, tuplelen);
+ change->data.tp.newtuple =
+ ReorderBufferGetTupleBuf(ctx->reorder, tuplelen);
- DecodeXLogTuple(tupledata, datalen, change->data.tp.newtuple);
- }
+ DecodeXLogTuple(tupledata, datalen, change->data.tp.newtuple);
change->data.tp.clear_toast_afterwards = true;
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index bed63c768e..23466bade2 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1598,17 +1598,12 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
* transaction's changes. Otherwise it will get
* freed/reused while restoring spooled data from
* disk.
- *
- * But skip doing so if there's no tuple-data. That
- * happens if a non-mapped system catalog with a toast
- * table is rewritten.
*/
- if (change->data.tp.newtuple != NULL)
- {
- dlist_delete(&change->node);
- ReorderBufferToastAppendChunk(rb, txn, relation,
- change);
- }
+ Assert(change->data.tp.newtuple != NULL);
+
+ dlist_delete(&change->node);
+ ReorderBufferToastAppendChunk(rb, txn, relation,
+ change);
}
change_done:
