Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit e9edc1b

Browse filesBrowse 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
Copy full SHA for e9edc1b

File tree

Expand file treeCollapse file tree

6 files changed

+163
-10
lines changed
Filter options
Expand file treeCollapse file tree

6 files changed

+163
-10
lines changed

‎contrib/test_decoding/expected/rewrite.out

Copy file name to clipboardExpand all lines: contrib/test_decoding/expected/rewrite.out
+75Lines 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

Copy file name to clipboardExpand all lines: contrib/test_decoding/sql/rewrite.sql
+41-1Lines 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

Copy file name to clipboardExpand all lines: src/backend/access/heap/heapam.c
+10-1Lines 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

Copy file name to clipboardExpand all lines: src/backend/access/heap/rewriteheap.c
+16-3Lines 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

Copy file name to clipboardExpand all lines: src/backend/replication/logical/reorderbuffer.c
+20-5Lines 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

Copy file name to clipboardExpand all lines: src/include/access/heapam.h
+1Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.