8000 Fix logical decoding error when system table w/ toast is repeatedly r… · postgrespro/postgres@88670a4 · GitHub
[go: up one dir, main page]
Skip to content

Commit 88670a4

Browse files
anarazelanarazel
committed
Fix logical decoding error when system table w/ toast is repeatedly rewritten.
Repeatedly rewriting a mapped catalog table with VACUUM FULL or CLUSTER could cause logical decoding to fail with: ERROR, "could not map filenode \"%s\" to relation OID" To trigger the problem the rewritten catalog had to have live tuples with toasted columns. The problem was triggered as during catalog table rewrites the heap_insert() check that prevents logical decoding information to be emitted for system catalogs, failed to treat the new heap's toast table as a system catalog (because the new heap is not recognized as a catalog table via RelationIsLogicallyLogged()). The relmapper, in contrast to the normal catalog contents, does not contain historical information. After a single rewrite of a mapped table the new relation is known to the relmapper, but if the table is rewritten twice before logical decoding occurs, the relfilenode cannot be mapped to a relation anymore. Which then leads us to error out. This only happens for toast tables, because the main table contents aren't re-inserted with heap_insert(). The fix is simple, add a new heap_insert() flag that prevents logical decoding information from being emitted, and accept during decoding that there might not be tuple data for toast tables. Unfortunately that does not fix pre-existing logical decoding errors. Doing so would require not throwing an error when a filenode cannot be mapped to a relation during decoding, and that seems too likely to hide bugs. If it's crucial to fix decoding for an existing slot, temporarily changing the ERROR in ReorderBufferCommit() to a WARNING appears to be the best fix. Author: Andres Freund Discussion: https://postgr.es/m/20180914021046.oi7dm4ra3ot2g2kt@alap3.anarazel.de Backpatch: 9.4-, where logical decoding was introduced
1 parent 4c67618 commit 88670a4

File tree

6 files changed

+163
-10
lines changed
  • src

    • 6 files changed

    +163
    -10
    lines changed

    ‎contrib/test_decoding/expected/rewrite.out

    Lines changed: 75 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1,6 +1,61 @@
    11
    -- predictability
    22
    SET synchronous_commit = on;
    33
    DROP TABLE IF EXISTS replication_example;
    4+
    -- Ensure there's tables with toast datums. To do so, we dynamically
    5+
    -- create a function returning a large textblob. We want tables of
    6+
    -- different kinds: mapped catalog table, unmapped catalog table,
    7+
    -- shared catalog table and usertable.
    8+
    CREATE FUNCTION exec(text) returns void language plpgsql volatile
    9+
    AS $f$
    10+
    BEGIN
    11+
    EXECUTE $1;
    12+
    END;
    13+
    $f$;
    14+
    CREATE ROLE justforcomments NOLOGIN;
    15+
    SELECT exec(
    16+
    format($outer$CREATE FUNCTION iamalongfunction() RETURNS TEXT IMMUTABLE LANGUAGE SQL AS $f$SELECT text %L$f$$outer$,
    17+
    (SELECT repeat(string_agg(to_char(g.i, 'FM0000'), ''), 50) FROM generate_series(1, 500) g(i))));
    18+
    exec
    19+
    ------
    20+
    21+
    (1 row)
    22+
    23+
    SELECT exec(
    24+
    format($outer$COMMENT ON FUNCTION iamalongfunction() IS %L$outer$,
    25+
    iamalongfunction()));
    26+
    exec
    27+
    ------
    28+
    29+
    (1 row)
    30+
    31+
    SELECT exec(
    32+
    format($outer$COMMENT ON ROLE JUSTFORCOMMENTS IS %L$outer$,
    33+
    iamalongfunction()));
    34+
    exec
    35+
    ------
    36+
    37+
    (1 row)
    38+
    39+
    CREATE TABLE iamalargetable AS SELECT iamalongfunction() longfunctionoutput;
    40+
    -- verify toast usage
    41+
    SELECT pg_relation_size((SELECT reltoastrelid FROM pg_class WHERE oid = 'pg_proc'::regclass)) > 0;
    42+
    ?column?
    43+
    ----------
    44+
    t
    45+
    (1 row)
    46+
    47+
    SELECT pg_relation_size((SELECT reltoastrelid FROM pg_class WHERE oid = 'pg_description'::regclass)) > 0;
    48+
    ?column?
    49+
    ----------
    50+
    t
    51+
    (1 row)
    52+
    53+
    SELECT pg_relation_size((SELECT reltoastrelid FROM pg_class WHERE oid = 'pg_shdescription'::regclass)) > 0;
    54+
    ?column?
    55+
    ----------
    56+
    t
    57+
    (1 row)
    58+
    459
    SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_decoding');
    560
    ?column?
    661
    ----------
    @@ -76,10 +131,30 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc
    76131
    COMMIT
    77132
    (15 rows)
    78133

    134+
    -- trigger repeated rewrites of a system catalog with a toast table,
    135+
    -- that previously was buggy: 20180914021046.oi7dm4ra3ot2g2kt@alap3.anarazel.de
    136+
    VACUUM FULL pg_proc; VACUUM FULL pg_description; VACUUM FULL pg_shdescription; VACUUM FULL iamalargetable;
    137+
    INSERT INTO replication_example(somedata, testcolumn1, testcolumn3) VALUES (8, 6, 1);
    138+
    VACUUM FULL pg_proc; VACUUM FULL pg_description; VACUUM FULL pg_shdescription; VACUUM FULL iamalargetable;
    139+
    INSERT INTO replication_example(somedata, testcolumn1, testcolumn3) VALUES (9, 7, 1);
    140+
    SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
    141+
    data
    142+
    -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
    143+
    BEGIN
    144+
    table public.replication_example: INSERT: id[integer]:9 somedata[integer]:8 text[character varying]:null testcolumn1[integer]:6 testcolumn2[integer]:null testcolumn3[integer]:1
    145+
    COMMIT
    146+
    BEGIN
    147+
    table public.replication_example: INSERT: id[integer]:10 somedata[integer]:9 text[character varying]:null testcolumn1[integer]:7 testcolumn2[integer]:null testcolumn3[integer]:1
    148+
    COMMIT
    149+
    (6 rows)
    150+
    79151
    SELECT pg_drop_replication_slot('regression_slot');
    80152
    pg_drop_replication_slot
    81153
    --------------------------
    82154

    83155
    (1 row)
    84156

    85157
    DROP TABLE IF EXISTS replication_example;
    158+
    DROP FUNCTION iamalongfunction();
    159+
    DROP FUNCTION exec(text);
    160+
    DROP ROLE justforcomments;

    ‎contrib/test_decoding/sql/rewrite.sql

    Lines changed: 41 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -3,6 +3,35 @@ SET synchronous_commit = on;
    33

    44
    DROP TABLE IF EXISTS replication_example;
    55

    6+
    -- Ensure there's tables with toast datums. To do so, we dynamically
    7+
    -- create a function returning a large textblob. We want tables of
    8+
    -- different kinds: mapped catalog table, unmapped catalog table,
    9+
    -- shared catalog table and usertable.
    10+
    CREATE FUNCTION exec(text) returns void language plpgsql volatile
    11+
    AS $f$
    12+
    BEGIN
    13+
    EXECUTE $1;
    14+
    END;
    15+
    $f$;
    16+
    CREATE ROLE justforcomments NOLOGIN;
    17+
    18+
    SELECT exec(
    19+
    format($outer$CREATE FUNCTION iamalongfunction() RETURNS TEXT IMMUTABLE LANGUAGE SQL AS $f$SELECT text %L$f$$outer$,
    20+
    (SELECT repeat(string_agg(to_char(g.i, 'FM0000'), ''), 50) FROM generate_series(1, 500) g(i))));
    21+
    SELECT exec(
    22+
    format($outer$COMMENT ON FUNCTION iamalongfunction() IS %L$outer$,
    23+
    iamalongfunction()));
    24+
    SELECT exec(
    25+
    format($outer$COMMENT ON ROLE JUSTFORCOMMENTS IS %L$outer$,
    26+
    iamalongfunction()));
    27+
    CREATE TABLE iamalargetable AS SELECT iamalongfunction() longfunctionoutput;
    28+
    29+
    -- verify toast usage
    30+
    SELECT pg_relation_size((SELECT reltoastrelid FROM pg_class WHERE oid = 'pg_proc'::regclass)) > 0;
    31+
    SELECT pg_relation_size((SELECT reltoastrelid FROM pg_class WHERE oid = 'pg_description'::regclass)) > 0;
    32+
    SELECT pg_relation_size((SELECT reltoastrelid FROM pg_class WHERE oid = 'pg_shdescription'::regclass)) > 0;
    33+
    34+
    635
    SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_decoding');
    736
    CREATE TABLE replication_example(id SERIAL PRIMARY KEY, somedata int, text varchar(120));
    837
    INSERT INTO replication_example(somedata) VALUES (1);
    @@ -57,6 +86,17 @@ COMMIT;
    5786
    CHECKPOINT;
    5887

    5988
    SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
    60-
    SELECT pg_drop_replication_slot('regression_slot');
    6189

    90+
    -- trigger repeated rewrites of a system catalog with a toast table,
    91+
    -- that previously was buggy: 20180914021046.oi7dm4ra3ot2g2kt@alap3.anarazel.de
    92+
    VACUUM FULL pg_proc; VACUUM FULL pg_description; VACUUM FULL pg_shdescription; VACUUM FULL iamalargetable;
    93+
    INSERT INTO replication_example(somedata, testcolumn1, testcolumn3) VALUES (8, 6, 1);
    94+
    VACUUM FULL pg_proc; VACUUM FULL pg_description; VACUUM FULL pg_shdescription; VACUUM FULL iamalargetable;
    95+
    INSERT INTO replication_example(somedata, testcolumn1, testcolumn3) VALUES (9, 7, 1);
    96+
    SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
    97+
    98+
    SELECT pg_drop_replication_slot('regression_slot');
    6299
    DROP TABLE IF EXISTS replication_example;
    100+
    DROP FUNCTION iamalongfunction();
    101+
    DROP FUNCTION exec(text);
    102+
    DROP ROLE justforcomments;

    ‎src/backend/access/heap/heapam.c

    Lines changed: 10 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -2423,6 +2423,11 @@ ReleaseBulkInsertStatePin(BulkInsertState bistate)
    24232423
    * Speculatively inserted tuples behave as "value locks" of short duration,
    24242424
    * used to implement INSERT .. ON CONFLICT.
    24252425
    *
    2426+
    * HEAP_INSERT_NO_LOGICAL force-disables the emitting of logical decoding
    2427+
    * information for the tuple. This should solely be used during table rewrites
    2428+
    * where RelationIsLogicallyLogged(relation) is not yet accurate for the new
    2429+
    * relation.
    2430+
    *
    24262431
    * Note that most of these options will be applied when inserting into the
    24272432
    * heap's TOAST table, too, if the tuple requires any out-of-line data. Only
    24282433
    * HEAP_INSERT_SPECULATIVE is explicitly ignored, as the toast data does not
    @@ -2551,7 +2556,8 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
    25512556
    * page write, so make sure it's included even if we take a full-page
    25522557
    * image. (XXX We could alternatively store a pointer into the FPW).
    25532558
    */
    2554-
    if (RelationIsLogicallyLogged(relation))
    2559+
    if (RelationIsLogicallyLogged(relation) &&
    2560+
    !(options & HEAP_INSERT_NO_LOGICAL))
    25552561
    {
    25562562
    xlrec.flags |= XLH_INSERT_CONTAINS_NEW_TUPLE;
    25572563
    bufflags |= REGBUF_KEEP_DATA;
    @@ -2716,6 +2722,9 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
    27162722
    bool need_tuple_data = RelationIsLogicallyLogged(relation);
    27172723
    bool need_cids = RelationIsAccessibleInLogicalDecoding(relation);
    27182724

    2725+
    /* currently not needed (thus unsupported) for heap_multi_insert() */
    2726+
    AssertArg(!(options & HEAP_INSERT_NO_LOGICAL));
    2727+
    27192728
    needwal = !(options & HEAP_INSERT_SKIP_WAL) && RelationNeedsWAL(relation);
    27202729
    saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
    27212730
    HEAP_DEFAULT_FILLFACTOR);

    ‎src/backend/access/heap/rewriteheap.c

    Lines changed: 16 additions & 3 deletions
    Original file line numberDiff line numberDiff line change
    @@ -652,10 +652,23 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
    652652
    heaptup = tup;
    653653
    }
    654654
    else if (HeapTupleHasExternal(tup) || tup->t_len > TOAST_TUPLE_THRESHOLD)
    655+
    {
    656+
    int options = HEAP_INSERT_SKIP_FSM;
    657+
    658+
    if (!state->rs_use_wal)
    659+
    options |= HEAP_INSERT_SKIP_WAL;
    660+
    661+
    /*
    662+
    * The new relfilenode's relcache entrye doesn't have the necessary
    663+
    * information to determine whether a relation should emit data for
    664+
    * logical decoding. Force it to off if necessary.
    665+
    */
    666+
    if (!RelationIsLogicallyLogged(state->rs_old_rel))
    667+
    options |= HEAP_INSERT_NO_LOGICAL;
    668+
    655669
    heaptup = toast_insert_or_update(state->rs_new_rel, tup, NULL,
    656-
    HEAP_INSERT_SKIP_FSM |
    657-
    (state->rs_use_wal ?
    658-
    0 : HEAP_INSERT_SKIP_WAL));
    670+
    options);
    671+
    }
    659672
    else
    660673
    heaptup = tup;
    661674

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

    Lines changed: 20 additions & 5 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1527,8 +1527,16 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
    15271527
    change->data.tp.relnode.relNode);
    15281528

    15291529
    /*
    1530-
    * Catalog tuple without data, emitted while catalog was
    1531-
    * in the process of being rewritten.
    1530+
    * Mapped catalog tuple without data, emitted while
    1531+
    * catalog table was in the process of being rewritten. We
    1532+
    * can fail to look up the relfilenode, because the the
    1533+
    * relmapper has no "historic" view, in contrast to normal
    1534+
    * the normal catalog during decoding. Thus repeated
    1535+
    * rewrites can cause a lookup failure. That's OK because
    1536+
    * we do not decode catalog changes anyway. Normally such
    1537+
    * tuples would be skipped over below, but we can't
    1538+
    * identify whether the table should be logically logged
    1539+
    * without mapping the relfilenode to the oid.
    15321540
    */
    15331541
    if (reloid == InvalidOid &&
    15341542
    change->data.tp.newtuple == NULL &&
    @@ -1590,10 +1598,17 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
    15901598
    * transaction's changes. Otherwise it will get
    15911599
    * freed/reused while restoring spooled data from
    15921600
    * disk.
    1601+
    *
    1602+
    * But skip doing so if there's no tuple-data. That
    1603+
    * happens if a non-mapped system catalog with a toast
    1604+
    * table is rewritten.
    15931605
    */
    1594-
    dlist_delete(&change->node);
    1595-
    ReorderBufferToastAppendChunk(rb, txn, relation,
    1596-
    change);
    1606+
    if (change->data.tp.newtuple != NULL)
    1607+
    {
    1608+
    dlist_delete(&change->node);
    1609+
    ReorderBufferToastAppendChunk(rb, txn, relation,
    1610+
    change);
    1611+
    }
    15971612
    }
    15981613

    15991614
    change_done:

    ‎src/include/access/heapam.h

    Lines changed: 1 addition & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -29,6 +29,7 @@
    2929
    #define HEAP_INSERT_SKIP_FSM 0x0002
    3030
    #define HEAP_INSERT_FROZEN 0x0004
    3131
    #define HEAP_INSERT_SPECULATIVE 0x0008
    32+
    #define HEAP_INSERT_NO_LOGICAL 0x0010
    3233

    3334
    typedef struct BulkInsertStateData *BulkInsertState;
    3435

    0 commit comments

    Comments
     (0)
    0