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

Commit 1914c5e

Browse files
committed
Avoid having vacuum set reltuples to 0 on non-empty relations in the
presence of page pins, which leads to serious estimation errors in the planner. This particularly affects small heavily-accessed tables, especially where locking (e.g. from FK constraints) forces frequent vacuums for mxid cleanup. Fix by keeping separate track of pages whose live tuples were actually counted vs. pages that were only scanned for freezing purposes. Thus, reltuples can only be set to 0 if all pages of the relation were actually counted. Backpatch to all supported versions. Per bug #14057 from Nicolas Baccelli, analyzed by me. Discussion: https://postgr.es/m/20160331103739.8956.94469@wrigleys.postgresql.org
1 parent 80824dd commit 1914c5e

File tree

4 files changed

+119
-4
lines changed

4 files changed

+119
-4
lines changed

src/backend/commands/vacuumlazy.c

+11-4
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,8 @@ typedef struct LVRelStats
114114
BlockNumber scanned_pages; /* number of pages we examined */
115115
BlockNumber pinskipped_pages; /* # of pages we skipped due to a pin */
116116
BlockNumber frozenskipped_pages; /* # of frozen pages we skipped */
117-
double scanned_tuples; /* counts only tuples on scanned pages */
117+
BlockNumber tupcount_pages; /* pages whose tuples we counted */
118+
double scanned_tuples; /* counts only tuples on tupcount_pages */
118119
double old_rel_tuples; /* previous value of pg_class.reltuples */
119120
double new_rel_tuples; /* new estimated total # of tuples */
120121
double new_dead_tuples; /* new estimated total # of dead tuples */
@@ -299,6 +300,10 @@ lazy_vacuum_rel(Relation onerel, int options, VacuumParams *params,
299300
* density") with nonzero relpages and reltuples=0 (which means "zero
300301
* tuple density") unless there's some actual evidence for the latter.
301302
*
303+
* It's important that we use tupcount_pages and not scanned_pages for the
304+
* check described above; scanned_pages counts pages where we could not
305+
* get cleanup lock, and which were processed only for frozenxid purposes.
306+
*
302307
* We do update relallvisible even in the corner case, since if the table
303308
* is all-visible we'd definitely like to know that. But clamp the value
304309
* to be not more than what we're setting relpages to.
@@ -308,7 +313,7 @@ lazy_vacuum_rel(Relation onerel, int options, VacuumParams *params,
308313
*/
309314
new_rel_pages = vacrelstats->rel_pages;
310315
new_rel_tuples = vacrelstats->new_rel_tuples;
311-
if (vacrelstats->scanned_pages == 0 && new_rel_pages > 0)
316+
if (vacrelstats->tupcount_pages == 0 && new_rel_pages > 0)
312317
{
313318
new_rel_pages = vacrelstats->old_rel_pages;
314319
new_rel_tuples = vacrelstats->old_rel_tuples;
@@ -496,6 +501,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
496501
nblocks = RelationGetNumberOfBlocks(onerel);
497502
vacrelstats->rel_pages = nblocks;
498503
vacrelstats->scanned_pages = 0;
504+
vacrelstats->tupcount_pages = 0;
499505
vacrelstats->nonempty_pages = 0;
500506
vacrelstats->latestRemovedXid = InvalidTransactionId;
501507

@@ -818,6 +824,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
818824
}
819825

820826
vacrelstats->scanned_pages++;
827+
vacrelstats->tupcount_pages++;
821828

822829
page = BufferGetPage(buf);
823830

@@ -1261,7 +1268,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
12611268
/* now we can compute the new value for pg_class.reltuples */
12621269
vacrelstats->new_rel_tuples = vac_estimate_reltuples(onerel, false,
12631270
nblocks,
1264-
vacrelstats->scanned_pages,
1271+
vacrelstats->tupcount_pages,
12651272
num_tuples);
12661273

12671274
/*
@@ -1625,7 +1632,7 @@ lazy_cleanup_index(Relation indrel,
16251632

16261633
ivinfo.index = indrel;
16271634
ivinfo.analyze_only = false;
1628-
ivinfo.estimated_count = (vacrelstats->scanned_pages < vacrelstats->rel_pages);
1635+
ivinfo.estimated_count = (vacrelstats->tupcount_pages < vacrelstats->rel_pages);
16291636
ivinfo.message_level = elevel;
16301637
ivinfo.num_heap_tuples = vacrelstats->new_rel_tuples;
16311638
ivinfo.strategy = vac_strategy;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
Parsed test spec with 2 sessions
2+
3+
starting permutation: modify vac stats
4+
step modify:
5+
insert into smalltbl select max(id)+1 from smalltbl;
6+
delete from smalltbl where id in (select min(id) from smalltbl);
7+
8+
step vac:
9+
vacuum smalltbl;
10+
11+
step stats:
12+
select relpages, reltuples from pg_class
13+
where oid='smalltbl'::regclass;
14+
15+
relpages reltuples
16+
17+
1 20
18+
19+
starting permutation: modify open fetch1 vac close stats
20+
step modify:
21+
insert into smalltbl select max(id)+1 from smalltbl;
22+
delete from smalltbl where id in (select min(id) from smalltbl);
23+
24+
step open:
25+
begin;
26+
declare c1 cursor for select * from smalltbl;
27+
28+
step fetch1:
29+
fetch next from c1;
30+
31+
id
32+
33+
2
34+
step vac:
35+
vacuum smalltbl;
36+
37+
step close:
38+
commit;
39+
40+
step stats:
41+
select relpages, reltuples from pg_class
42+
where oid='smalltbl'::regclass;
43+
44+
relpages reltuples
45+
46+
1 20
47+
48+
starting permutation: modify vac stats
49+
step modify:
50+
insert into smalltbl select max(id)+1 from smalltbl;
51+
delete from smalltbl where id in (select min(id) from smalltbl);
52+
53+
step vac:
54+
vacuum smalltbl;
55+
56+
step stats:
57+
select relpages, reltuples from pg_class
58+
where oid='smalltbl'::regclass;
59+
60+
relpages reltuples
61+
62+
1 20

src/test/isolation/isolation_schedule

+1
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,5 @@ test: alter-table-2
5656
test: alter-table-3
5757
test: create-trigger
5858
test: async-notify
59+
test: vacuum-reltuples
5960
test: timeouts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# Test for vacuum's handling of reltuples when pages are skipped due
2+
# to page pins. We absolutely need to avoid setting reltuples=0 in
3+
# such cases, since that interferes badly with planning.
4+
5+
setup {
6+
create table smalltbl
7+
as select i as id from generate_series(1,20) i;
8+
alter table smalltbl set (autovacuum_enabled = off);
9+
}
10+
setup {
11+
vacuum analyze smalltbl;
12+
}
13+
14+
teardown {
15+
drop table smalltbl;
16+
}
17+
18+
session "worker"
19+
step "open" {
20+
begin;
21+
declare c1 cursor for select * from smalltbl;
22+
}
23+
step "fetch1" {
24+
fetch next from c1;
25+
}
26+
step "close" {
27+
commit;
28+
}
29+
step "stats" {
30+
select relpages, reltuples from pg_class
31+
where oid='smalltbl'::regclass;
32+
}
33+
34+
session "vacuumer"
35+
step "vac" {
36+
vacuum smalltbl;
37+
}
38+
step "modify" {
39+
insert into smalltbl select max(id)+1 from smalltbl;
40+
delete from smalltbl where id in (select min(id) from smalltbl);
41+
}
42+
43+
permutation "modify" "vac" "stats"
44+
permutation "modify" "open" "fetch1" "vac" "close" "stats"
45+
permutation "modify" "vac" "stats"

0 commit comments

Comments
 (0)