Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 43a33ef

Browse files
anarazelanarazel
committed
Support RBM_ZERO_AND_CLEANUP_LOCK in ExtendBufferedRelTo(), add tests
For some reason I had not implemented RBM_ZERO_AND_CLEANUP_LOCK support in ExtendBufferedRelTo(), likely thinking it not being reachable. But it is reachable, e.g. when replaying a WAL record for a page in a relation that subsequently is truncated (likely only reachable when doing crash recovery or PITR, not during ongoing streaming replication). As now all of the RBM_* modes are supported, remove assertions checking mode. As we had no test coverage for this scenario, add a new TAP test. There's plenty more that ought to be tested in this area... Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Reported-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/392271.1681238924%40sss.pgh.pa.us Discussion: https://postgr.es/m/0b5eb82b-cb99-e0a4-b932-3dc60e2e3926@gmail.com
1 parent e4d905f commit 43a33ef

File tree

3 files changed

+154
-11
lines changed

3 files changed

+154
-11
lines changed

‎src/backend/storage/buffer/bufmgr.c

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -888,8 +888,6 @@ ExtendBufferedRelTo(ExtendBufferedWhat eb,
888888
Assert((eb.rel != NULL) != (eb.smgr != NULL));
889889
Assert(eb.smgr == NULL || eb.relpersistence != 0);
890890
Assert(extend_to != InvalidBlockNumber && extend_to > 0);
891-
Assert(mode == RBM_NORMAL || mode == RBM_ZERO_ON_ERROR ||
892-
mode == RBM_ZERO_AND_LOCK);
893891

894892
if (eb.smgr == NULL)
895893
{
@@ -933,7 +931,13 @@ ExtendBufferedRelTo(ExtendBufferedWhat eb,
933931
*/
934932
current_size = smgrnblocks(eb.smgr, fork);
935933

936-
if (mode == RBM_ZERO_AND_LOCK)
934+
/*
935+
* Since no-one else can be looking at the page contents yet, there is no
936+
* difference between an exclusive lock and a cleanup-strength lock. Note
937+
* that we pass the original mode to ReadBuffer_common() below, when
938+
* falling back to reading the buffer to a concurrent relation extension.
939+
*/
940+
if (mode == RBM_ZERO_AND_LOCK || mode == RBM_ZERO_AND_CLEANUP_LOCK)
937941
flags |= EB_LOCK_TARGET;
938942

939943
while (current_size < extend_to)
@@ -1008,11 +1012,12 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
10081012
{
10091013
uint32 flags = EB_SKIP_EXTENSION_LOCK;
10101014

1011-
Assert(mode == RBM_NORMAL ||
1012-
mode == RBM_ZERO_AND_LOCK ||
1013-
mode == RBM_ZERO_ON_ERROR);
1014-
1015-
if (mode == RBM_ZERO_AND_LOCK)
1015+
/*
1016+
* Since no-one else can be looking at the page contents yet, there is
1017+
* no difference between an exclusive lock and a cleanup-strength
1018+
* lock.
1019+
*/
1020+
if (mode == RBM_ZERO_AND_LOCK || mode == RBM_ZERO_AND_CLEANUP_LOCK)
10161021
flags |= EB_LOCK_FIRST;
10171022

10181023
return ExtendBufferedRel(EB_SMGR(smgr, relpersistence),
@@ -1145,9 +1150,10 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
11451150
}
11461151

11471152
/*
1148-
* In RBM_ZERO_AND_LOCK mode, grab the buffer content lock before marking
1149-
* the page as valid, to make sure that no other backend sees the zeroed
1150-
* page before the caller has had a chance to initialize it.
1153+
* In RBM_ZERO_AND_LOCK / RBM_ZERO_AND_CLEANUP_LOCK mode, grab the buffer
1154+
* content lock before marking the page as valid, to make sure that no
1155+
* other backend sees the zeroed page before the caller has had a chance
1156+
* to initialize it.
11511157
*
11521158
* Since no-one else can be looking at the page contents yet, there is no
11531159
* difference between an exclusive lock and a cleanup-strength lock. (Note

‎src/test/recovery/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ tests += {
4141
't/033_replay_tsp_drops.pl',
4242
't/034_create_database.pl',
4343
't/035_standby_logical_decoding.pl',
44+
't/036_truncated_dropped.pl',
4445
],
4546
},
4647
}

‎src/test/recovery/t/036_truncated_dropped.pl

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
# Copyright (c) 2021-2023, PostgreSQL Global Development Group
2+
3+
# Tests recovery scenarios where the files are shorter than in the common
4+
# cases, e.g. due to replaying WAL records of a relation that was subsequently
5+
# truncated or dropped.
6+
7+
use strict;
8+
use warnings;
9+
use PostgreSQL::Test::Cluster;
10+
use Test::More;
11+
12+
my $node = PostgreSQL::Test::Cluster->new('n1');
13+
14+
$node->init();
15+
16+
# Disable autovacuum to guarantee VACUUM can remove rows / truncate relations
17+
$node->append_conf(
18+
'postgresql.conf', qq[
19+
wal_level = 'replica'
20+
autovacuum = off
21+
]);
22+
23+
$node->start();
24+
25+
26+
# Test: Replay replay of PRUNE records for a pre-existing, then dropped,
27+
# relation
28+
29+
$node->safe_psql(
30+
'postgres', qq[
31+
CREATE TABLE truncme(i int) WITH (fillfactor = 50);
32+
INSERT INTO truncme SELECT generate_series(1, 1000);
33+
UPDATE truncme SET i = 1;
34+
CHECKPOINT; -- ensure relation exists at start of recovery
35+
VACUUM truncme; -- generate prune records
36+
DROP TABLE truncme;
37+
]);
38+
39+
$node->stop('immediate');
40+
41+
ok($node->start(),
42+
'replay of PRUNE records for a pre-existing, then dropped, relation');
43+
44+
45+
# Test: Replay of PRUNE records for a newly created, then dropped, relation
46+
47+
$node->safe_psql(
48+
'postgres', qq[
49+
CREATE TABLE truncme(i int) WITH (fillfactor = 50);
50+
INSERT INTO truncme SELECT generate_series(1, 1000);
51+
UPDATE truncme SET i = 1;
52+
VACUUM truncme; -- generate prune records
53+
DROP TABLE truncme;
54+
]);
55+
56+
$node->stop('immediate');
57+
58+
ok($node->start(),
59+
'replay of PRUNE records for a newly created, then dropped, relation');
60+
61+
62+
# Test: Replay of PRUNE records affecting truncated block. With FPIs used for
63+
# PRUNE.
64+
65+
$node->safe_psql(
66+
'postgres', qq[
67+
CREATE TABLE truncme(i int) WITH (fillfactor = 50);
68+
INSERT INTO truncme SELECT generate_series(1, 1000);
69+
UPDATE truncme SET i = 1;
70+
CHECKPOINT; -- generate FPIs
71+
VACUUM truncme; -- generate prune records
72+
TRUNCATE truncme; -- make blocks non-existing
73+
INSERT INTO truncme SELECT generate_series(1, 10);
74+
]);
75+
76+
$node->stop('immediate');
77+
78+
ok($node->start(),
79+
'replay of PRUNE records affecting truncated block (FPIs)');
80+
81+
is($node->safe_psql('postgres', 'select count(*), sum(i) FROM truncme'),
82+
'10|55', 'table contents as expected after recovery');
83+
$node->safe_psql('postgres', 'DROP TABLE truncme');
84+
85+
86+
# Test replay of PRUNE records for blocks that are later truncated. Without
87+
# FPIs used for PRUNE.
88+
89+
$node->safe_psql(
90+
'postgres', qq[
91+
CREATE TABLE truncme(i int) WITH (fillfactor = 50);
92+
INSERT INTO truncme SELECT generate_series(1, 1000);
93+
UPDATE truncme SET i = 1;
94+
VACUUM truncme; -- generate prune records
95+
TRUNCATE truncme; -- make blocks non-existing
96+
INSERT INTO truncme SELECT generate_series(1, 10);
97+
]);
98+
99+
$node->stop('immediate');
100+
101+
ok($node->start(),
102+
'replay of PRUNE records affecting truncated block (no FPIs)');
103+
104+
is($node->safe_psql('postgres', 'select count(*), sum(i) FROM truncme'),
105+
'10|55', 'table contents as expected after recovery');
106+
$node->safe_psql('postgres', 'DROP TABLE truncme');
107+
108+
109+
# Test: Replay of partial truncation via VACUUM
110+
111+
$node->safe_psql(
112+
'postgres', qq[
113+
CREATE TABLE truncme(i int) WITH (fillfactor = 50);
114+
INSERT INTO truncme SELECT generate_series(1, 1000);
115+
UPDATE truncme SET i = i + 1;
116+
-- ensure a mix of pre/post truncation rows
117+
DELETE FROM truncme WHERE i > 500;
118+
119+
VACUUM truncme; -- should truncate relation
120+
121+
-- rows at TIDs that previously existed
122+
INSERT INTO truncme SELECT generate_series(1000, 1010);
123+
]);
124+
125+
$node->stop('immediate');
126+
127+
ok($node->start(), 'replay of partial truncation via VACUUM');
128+
129+
is( $node->safe_psql(
130+
'postgres', 'select count(*), sum(i), min(i), max(i) FROM truncme'),
131+
'510|136304|2|1010',
132+
'table contents as expected after recovery');
133+
$node->safe_psql('postgres', 'DROP TABLE truncme');
134+
135+
136+
done_testing();

0 commit comments

Comments
 (0)