Skip to content

Commit e9edc1b

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 ef49305 commit e9edc1b

File tree

6 files changed

+163
-10
lines changed

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
@@ -2435,6 +2435,11 @@ ReleaseBulkInsertStatePin(BulkInsertState bistate)
24352435
* Speculatively inserted tuples behave as "value locks" of short duration,
24362436
* used to implement INSERT .. ON CONFLICT.
24372437
*
2438+
* HEAP_INSERT_NO_LOGICAL force-disables the emitting of logical decoding
2439+
* information for the tuple. This should solely be used during table rewrites
2440+
* where RelationIsLogicallyLogged(relation) is not yet accurate for the new
2441+
* relation.
2442+
*
24382443
* Note that most of these options will be applied when inserting into the
24392444
* heap's TOAST table, too, if the tuple requires any out-of-line data. Only
24402445
* HEAP_INSERT_SPECULATIVE is explicitly ignored, as the toast data does not
@@ -2563,7 +2568,8 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
25632568
* page write, so make sure it's included even if we take a full-page
25642569
* image. (XXX We could alternatively store a pointer into the FPW).
25652570
*/
2566-
if (RelationIsLogicallyLogged(relation))
2571+
if (RelationIsLogicallyLogged(relation) &&
2572+
!(options & HEAP_INSERT_NO_LOGICAL))
25672573
{
25682574
xlrec.flags |= XLH_INSERT_CONTAINS_NEW_TUPLE;
25692575
bufflags |= REGBUF_KEEP_DATA;
@@ -2728,6 +2734,9 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
27282734
bool need_tuple_data = RelationIsLogicallyLogged(relation);
27292735
bool need_cids = RelationIsAccessibleInLogicalDecoding(relation);
27302736

2737+
/* currently not needed (thus unsupported) for heap_multi_insert() */
2738+
AssertArg(!(options & HEAP_INSERT_NO_LOGICAL));
2739+
27312740
needwal = !(options & HEAP_INSERT_SKIP_WAL) && RelationNeedsWAL(relation);
27322741
saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
27332742
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)