public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint 23+ messages / 4 participants [nested] [flat]
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint @ 2020-07-02 23:46 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Justin Pryzby @ 2020-07-02 23:46 UTC (permalink / raw) --- .../postgres_fdw/expected/postgres_fdw.out | 12 ++++----- src/backend/optimizer/path/indxpath.c | 5 ++++ src/test/regress/expected/create_index.out | 25 +++++++++++++++++++ src/test/regress/sql/create_index.sql | 10 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index dbbae1820e..e8c2af1c04 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7823,18 +7823,17 @@ insert into utrtest values (2, 'qux'); -- Check case where the foreign partition is a subplan target rel explain (verbose, costs off) update utrtest set a = 1 where a = 1 or a = 2 returning *; - QUERY PLAN ----------------------------------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------- Update on public.utrtest Output: utrtest_1.a, utrtest_1.b Foreign Update on public.remp utrtest_1 Update on public.locp utrtest_2 -> Foreign Update on public.remp utrtest_1 - Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b + Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b -> Seq Scan on public.locp utrtest_2 Output: 1, utrtest_2.b, utrtest_2.ctid - Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2)) -(9 rows) +(8 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; @@ -7855,8 +7854,7 @@ update utrtest set a = 1 where a = 2 returning *; Update on public.locp utrtest_1 -> Seq Scan on public.locp utrtest_1 Output: 1, utrtest_1.b, utrtest_1.ctid - Filter: (utrtest_1.a = 2) -(6 rows) +(5 rows) -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 2 returning *; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index bcb1bc6097..0532b3ddd0 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1305,6 +1305,11 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, Assert(!restriction_is_or_clause(rinfo)); orargs = list_make1(rinfo); + /* Avoid scanning indexes using a scan condition which is + * inconsistent with the partition constraint */ + if (predicate_refuted_by(rel->partition_qual, orargs, false)) + continue; + indlist = build_paths_for_OR(root, rel, orargs, all_clauses); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..1a976ad211 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1843,6 +1843,31 @@ SELECT count(*) FROM tenk1 10 (1 row) +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); + QUERY PLAN +-------------------------------------------------------- + Append + -> Bitmap Heap Scan on bitmapor1 bitmapor_1 + Recheck Cond: ((i = 1) OR (i = 2)) + -> BitmapOr + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 1) + -> Bitmap Index Scan on bitmapor1_i_idx + Index Cond: (i = 2) + -> Bitmap Heap Scan on bitmapor2 bitmapor_2 + Recheck Cond: (i = 11) + -> Bitmap Index Scan on bitmapor2_i_idx + Index Cond: (i = 11) +(12 rows) + +DROP TABLE bitmapor; -- -- Check behavior with duplicate index column contents -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..dd1de8ee1d 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -703,6 +703,16 @@ SELECT count(*) FROM tenk1 SELECT count(*) FROM tenk1 WHERE hundred = 42 AND (thousand = 42 OR thousand = 99); +-- Check that indexes are not scanned for "arms" of an OR with a scan condition inconsistent with the partition constraint +CREATE TABLE bitmapor (i int, j int) PARTITION BY RANGE(i); +CREATE TABLE bitmapor1 PARTITION OF bitmapor FOR VALUES FROM (0) TO (10); +CREATE TABLE bitmapor2 PARTITION OF bitmapor FOR VALUES FROM (10) TO (20); +INSERT INTO bitmapor SELECT i%20, i%2 FROM generate_series(1,55555)i; +VACUUM ANALYZE bitmapor; +CREATE INDEX ON bitmapor(i); +EXPLAIN (COSTS OFF) SELECT * FROM bitmapor WHERE (i=1 OR i=2 OR i=11); +DROP TABLE bitmapor; + -- -- Check behavior with duplicate index column contents -- -- 2.17.0 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: WIP: WAL prefetch (another approach) @ 2022-03-09 06:46 Julien Rouhaud <[email protected]> 0 siblings, 1 reply; 23+ messages in thread From: Julien Rouhaud @ 2022-03-09 06:46 UTC (permalink / raw) To: Thomas Munro <[email protected]>; +Cc: Ashutosh Sharma <[email protected]>; Daniel Gustafsson <[email protected]>; Stephen Frost <[email protected]>; Alvaro Herrera <[email protected]>; Andres Freund <[email protected]>; David Steele <[email protected]>; Dmitry Dolgov <[email protected]>; Jakub Wartak <[email protected]>; Tom Lane <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; pgsql-hackers Hi, On Tue, Mar 08, 2022 at 06:15:43PM +1300, Thomas Munro wrote: > On Wed, Dec 29, 2021 at 5:29 PM Thomas Munro <[email protected]> wrote: > > https://github.com/macdice/postgres/tree/recovery-prefetch-ii > > Here's a rebase. This mostly involved moving hunks over to the new > xlogrecovery.c file. One thing that seemed a little strange to me > with the new layout is that xlogreader is now a global variable. I > followed that pattern and made xlogprefetcher a global variable too, > for now. I for now went through 0001, TL;DR the patch looks good to me. I have a few minor comments though, mostly to make things a bit clearer (at least to me). diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c index 2340dc247b..c129df44ac 100644 --- a/src/bin/pg_waldump/pg_waldump.c +++ b/src/bin/pg_waldump/pg_waldump.c @@ -407,10 +407,10 @@ XLogDumpRecordLen(XLogReaderState *record, uint32 *rec_len, uint32 *fpi_len) * add an accessor macro for this. */ *fpi_len = 0; + for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) { if (XLogRecHasBlockImage(record, block_id)) - *fpi_len += record->blocks[block_id].bimg_len; + *fpi_len += record->record->blocks[block_id].bimg_len; } (and similar in that file, xlogutils.c and xlogreader.c) This could use XLogRecGetBlock? Note that this macro is for now never used. xlogreader.c also has some similar forgotten code that could use XLogRecMaxBlockId. + * See if we can release the last record that was returned by + * XLogNextRecord(), to free up space. + */ +void +XLogReleasePreviousRecord(XLogReaderState *state) The comment seems a bit misleading, as I first understood it as it could be optional even if the record exists. Maybe something more like "Release the last record if any"? + * Remove it from the decoded record queue. It must be the oldest item + * decoded, decode_queue_tail. + */ + record = state->record; + Assert(record == state->decode_queue_tail); + state->record = NULL; + state->decode_queue_tail = record->next; The naming is a bit counter intuitive to me, as before reading the rest of the code I wasn't expecting the item at the tail of the queue to have a next element. Maybe just inverting tail and head would make it clearer? +DecodedXLogRecord * +XLogNextRecord(XLogReaderState *state, char **errormsg) +{ [...] + /* + * state->EndRecPtr is expected to have been set by the last call to + * XLogBeginRead() or XLogNextRecord(), and is the location of the + * error. + */ + + return NULL; The comment should refer to XLogFindNextRecord, not XLogNextRecord? Also, is it worth an assert (likely at the top of the function) for that? XLogRecord * XLogReadRecord(XLogReaderState *state, char **errormsg) +{ [...] + if (decoded) + { + /* + * XLogReadRecord() returns a pointer to the record's header, not the + * actual decoded record. The caller will access the decoded record + * through the XLogRecGetXXX() macros, which reach the decoded + * recorded as xlogreader->record. + */ + Assert(state->record == decoded); + return &decoded->header; I find it a bit weird to mention XLogReadRecord() as it's the current function. +/* + * Allocate space for a decoded record. The only member of the returned + * object that is initialized is the 'oversized' flag, indicating that the + * decoded record wouldn't fit in the decode buffer and must eventually be + * freed explicitly. + * + * Return NULL if there is no space in the decode buffer and allow_oversized + * is false, or if memory allocation fails for an oversized buffer. + */ +static DecodedXLogRecord * +XLogReadRecordAlloc(XLogReaderState *state, size_t xl_tot_len, bool allow_oversized) Is it worth clearly stating that it's the reponsability of the caller to update the decode_buffer_head (with the real size) after a successful decoding of this buffer? + if (unlikely(state->decode_buffer == NULL)) + { + if (state->decode_buffer_size == 0) + state->decode_buffer_size = DEFAULT_DECODE_BUFFER_SIZE; + state->decode_buffer = palloc(state->decode_buffer_size); + state->decode_buffer_head = state->decode_buffer; + state->decode_buffer_tail = state->decode_buffer; + state->free_decode_buffer = true; + } Maybe change XLogReaderSetDecodeBuffer to also handle allocation and use it here too? Otherwise XLogReaderSetDecodeBuffer should probably go in 0002 as the only caller is the recovery prefetching. + return decoded; +} I would find it a bit clearer to explicitly return NULL here. readOff = ReadPageInternal(state, targetPagePtr, Min(targetRecOff + SizeOfXLogRecord, XLOG_BLCKSZ)); - if (readOff < 0) + if (readOff == XLREAD_WOULDBLOCK) + return XLREAD_WOULDBLOCK; + else if (readOff < 0) ReadPageInternal comment should be updated to mention the new XLREAD_WOULDBLOCK possible return value. It's also not particulary obvious why XLogFindNextRecord() doesn't check for this value. AFAICS callers don't (and should never) call it with a nonblocking == true state, maybe add an assert for that? @@ -468,7 +748,7 @@ restart: if (pageHeader->xlp_info & XLP_FIRST_IS_OVERWRITE_CONTRECORD) { state->overwrittenRecPtr = RecPtr; - ResetDecoder(state); + //ResetDecoder(state); AFAICS this is indeed not necessary anymore, so it can be removed? static void ResetDecoder(XLogReaderState *state) { [...] + /* Reset the decoded record queue, freeing any oversized records. */ + while ((r = state->decode_queue_tail)) nit: I think it's better to explicitly check for the assignment being != NULL, and existing code is more frequently written this way AFAICS. +/* Return values from XLogPageReadCB. */ +typedef enum XLogPageReadResultResult typo ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: WIP: WAL prefetch (another approach) @ 2022-03-11 05:31 Thomas Munro <[email protected]> parent: Julien Rouhaud <[email protected]> 0 siblings, 1 reply; 23+ messages in thread From: Thomas Munro @ 2022-03-11 05:31 UTC (permalink / raw) To: Julien Rouhaud <[email protected]>; +Cc: Ashutosh Sharma <[email protected]>; Daniel Gustafsson <[email protected]>; Stephen Frost <[email protected]>; Alvaro Herrera <[email protected]>; Andres Freund <[email protected]>; David Steele <[email protected]>; Dmitry Dolgov <[email protected]>; Jakub Wartak <[email protected]>; Tom Lane <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; pgsql-hackers On Wed, Mar 9, 2022 at 7:47 PM Julien Rouhaud <[email protected]> wrote: > I for now went through 0001, TL;DR the patch looks good to me. I have a few > minor comments though, mostly to make things a bit clearer (at least to me). Hi Julien, Thanks for your review of 0001! It gave me a few things to think about and some good improvements. > diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c > index 2340dc247b..c129df44ac 100644 > --- a/src/bin/pg_waldump/pg_waldump.c > +++ b/src/bin/pg_waldump/pg_waldump.c > @@ -407,10 +407,10 @@ XLogDumpRecordLen(XLogReaderState *record, uint32 *rec_len, uint32 *fpi_len) > * add an accessor macro for this. > */ > *fpi_len = 0; > + for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) > { > if (XLogRecHasBlockImage(record, block_id)) > - *fpi_len += record->blocks[block_id].bimg_len; > + *fpi_len += record->record->blocks[block_id].bimg_len; > } > (and similar in that file, xlogutils.c and xlogreader.c) > > This could use XLogRecGetBlock? Note that this macro is for now never used. Yeah, I think that is a good idea for pg_waldump.c and xlogutils.c. Done. > xlogreader.c also has some similar forgotten code that could use > XLogRecMaxBlockId. That is true, but I was thinking of it like this: most of the existing code that interacts with xlogreader.c is working with the old model, where the XLogReader object holds only one "current" record. For that reason the XLogRecXXX() macros continue to work as before, implicitly referring to the record that XLogReadRecord() most recently returned. For xlogreader.c code, I prefer not to use the XLogRecXXX() macros, even when referring to the "current" record, since xlogreader.c has switched to a new multi-record model. In other words, they're sort of 'old API' accessors provided for continuity. Does this make sense? > + * See if we can release the last record that was returned by > + * XLogNextRecord(), to free up space. > + */ > +void > +XLogReleasePreviousRecord(XLogReaderState *state) > > The comment seems a bit misleading, as I first understood it as it could be > optional even if the record exists. Maybe something more like "Release the > last record if any"? Done. > + * Remove it from the decoded record queue. It must be the oldest item > + * decoded, decode_queue_tail. > + */ > + record = state->record; > + Assert(record == state->decode_queue_tail); > + state->record = NULL; > + state->decode_queue_tail = record->next; > > The naming is a bit counter intuitive to me, as before reading the rest of the > code I wasn't expecting the item at the tail of the queue to have a next > element. Maybe just inverting tail and head would make it clearer? Yeah, after mulling this over for a day, I agree. I've flipped it around. Explanation: You're quite right, singly-linked lists traditionally have a 'tail' that points to null, so it makes sense for new items to be added there and older items to be consumed from the 'head' end, as you expected. But... it's also typical (I think?) in ring buffers AKA circular buffers to insert at the 'head', and remove from the 'tail'. This code has both a linked-list (the chain of decoded records with a ->next pointer), and the underlying storage, which is a circular buffer of bytes. I didn't want them to use opposite terminology, and since I started by writing the ring buffer part, that's where I finished up... I agree that it's an improvement to flip them. > +DecodedXLogRecord * > +XLogNextRecord(XLogReaderState *state, char **errormsg) > +{ > [...] > + /* > + * state->EndRecPtr is expected to have been set by the last call to > + * XLogBeginRead() or XLogNextRecord(), and is the location of the > + * error. > + */ > + > + return NULL; > > The comment should refer to XLogFindNextRecord, not XLogNextRecord? No, it does mean to refer to the XLogNextRecord() (ie the last time you called XLogNextRecord and successfully dequeued a record, we put its end LSN there, so if there is a deferred error, that's the corresponding LSN). Make sense? > Also, is it worth an assert (likely at the top of the function) for that? How could I assert that EndRecPtr has the right value? > XLogRecord * > XLogReadRecord(XLogReaderState *state, char **errormsg) > +{ > [...] > + if (decoded) > + { > + /* > + * XLogReadRecord() returns a pointer to the record's header, not the > + * actual decoded record. The caller will access the decoded record > + * through the XLogRecGetXXX() macros, which reach the decoded > + * recorded as xlogreader->record. > + */ > + Assert(state->record == decoded); > + return &decoded->header; > > I find it a bit weird to mention XLogReadRecord() as it's the current function. Changed to "This function ...". > +/* > + * Allocate space for a decoded record. The only member of the returned > + * object that is initialized is the 'oversized' flag, indicating that the > + * decoded record wouldn't fit in the decode buffer and must eventually be > + * freed explicitly. > + * > + * Return NULL if there is no space in the decode buffer and allow_oversized > + * is false, or if memory allocation fails for an oversized buffer. > + */ > +static DecodedXLogRecord * > +XLogReadRecordAlloc(XLogReaderState *state, size_t xl_tot_len, bool allow_oversized) > > Is it worth clearly stating that it's the reponsability of the caller to update > the decode_buffer_head (with the real size) after a successful decoding of this > buffer? Comment added. > + if (unlikely(state->decode_buffer == NULL)) > + { > + if (state->decode_buffer_size == 0) > + state->decode_buffer_size = DEFAULT_DECODE_BUFFER_SIZE; > + state->decode_buffer = palloc(state->decode_buffer_size); > + state->decode_buffer_head = state->decode_buffer; > + state->decode_buffer_tail = state->decode_buffer; > + state->free_decode_buffer = true; > + } > > Maybe change XLogReaderSetDecodeBuffer to also handle allocation and use it > here too? Otherwise XLogReaderSetDecodeBuffer should probably go in 0002 as > the only caller is the recovery prefetching. I don't think it matters much? > + return decoded; > +} > > I would find it a bit clearer to explicitly return NULL here. Done. > readOff = ReadPageInternal(state, targetPagePtr, > Min(targetRecOff + SizeOfXLogRecord, XLOG_BLCKSZ)); > - if (readOff < 0) > + if (readOff == XLREAD_WOULDBLOCK) > + return XLREAD_WOULDBLOCK; > + else if (readOff < 0) > > ReadPageInternal comment should be updated to mention the new XLREAD_WOULDBLOCK > possible return value. Yeah. Done. > It's also not particulary obvious why XLogFindNextRecord() doesn't check for > this value. AFAICS callers don't (and should never) call it with a > nonblocking == true state, maybe add an assert for that? Fair point. I have now explicitly cleared that flag. (I don't much like state->nonblocking, which might be better as an argument to page_read(), but in fact I don't like the fact that page_read callbacks are blocking in the first place, which is why I liked Horiguchi-san's patch to get rid of that... but that can be a subject for later work.) > @@ -468,7 +748,7 @@ restart: > if (pageHeader->xlp_info & XLP_FIRST_IS_OVERWRITE_CONTRECORD) > { > state->overwrittenRecPtr = RecPtr; > - ResetDecoder(state); > + //ResetDecoder(state); > > AFAICS this is indeed not necessary anymore, so it can be removed? Oops, yeah I use C++ comments when there's something I intended to remove. Done. > static void > ResetDecoder(XLogReaderState *state) > { > [...] > + /* Reset the decoded record queue, freeing any oversized records. */ > + while ((r = state->decode_queue_tail)) > > nit: I think it's better to explicitly check for the assignment being != NULL, > and existing code is more frequently written this way AFAICS. I think it's perfectly normal idiomatic C, but if you think it's clearer that way, OK, done like that. > +/* Return values from XLogPageReadCB. */ > +typedef enum XLogPageReadResultResult > > typo Fixed. I realised that this version has broken -DWAL_DEBUG. I'll fix that shortly, but I wanted to post this update ASAP, so here's a new version. The other thing I need to change is that I should turn on recovery_prefetch for platforms that support it (ie Linux and maybe NetBSD only for now), in the tests. Right now you need to put recovery_prefetch=on in a file and then run the tests with "TEMP_CONFIG=path_to_that make -C src/test/recovery check" to excercise much of 0002. Attachments: [text/x-patch] v22-0001-Add-circular-WAL-decoding-buffer-take-II.patch (50.9K, ../../CA+hUKGKusWunqOd9En+FkT+US_kuDK-MSZyXHCr=OTwRw8Ah1A@mail.gmail.com/2-v22-0001-Add-circular-WAL-decoding-buffer-take-II.patch) download | inline diff: From e08d29de5ae85746e7a9c5a5539aa2c531d9c6e5 Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Tue, 9 Nov 2021 16:33:10 +1300 Subject: [PATCH v22 1/2] Add circular WAL decoding buffer, take II. Teach xlogreader.c to decode its output into a circular buffer, to support upcoming optimizations based on looking ahead. * XLogReadRecord() works as before, consuming records one by one, and allowing them to be examined via the traditional XLogRecGetXXX() macros, and the traditional members like xlogreader->ReadRecPtr. * An alternative new interface XLogReadAhead()/XLogNextRecord() is added that returns pointers to DecodedXLogRecord objects so that it's possible to look ahead in the WAL stream. * In order to be able to use the new interface effectively, client code should provide a page_read() callback that responds to a new nonblocking mode by returning XLREAD_WOULDBLOCK to avoid waiting. No such implementation is included in this commit, and other code that is unaware of the new mechanism doesn't need to change. The buffer's size can be set by the client of xlogreader.c. Large records that don't fit in the circular buffer are called "oversized" and allocated separately with palloc(). Reviewed-by: Julien Rouhaud <[email protected]> Discussion: https://postgr.es/m/CA+hUKGJ4VJN8ttxScUFM8dOKX0BrBiboo5uz1cq=AovOddfHpA@mail.gmail.com --- src/backend/access/transam/generic_xlog.c | 6 +- src/backend/access/transam/xlog.c | 2 +- src/backend/access/transam/xlogreader.c | 656 +++++++++++++++++----- src/backend/access/transam/xlogrecovery.c | 4 +- src/backend/access/transam/xlogutils.c | 2 +- src/backend/replication/logical/decode.c | 2 +- src/bin/pg_rewind/parsexlog.c | 2 +- src/bin/pg_waldump/pg_waldump.c | 25 +- src/include/access/xlogreader.h | 153 ++++- src/tools/pgindent/typedefs.list | 2 + 10 files changed, 675 insertions(+), 179 deletions(-) diff --git a/src/backend/access/transam/generic_xlog.c b/src/backend/access/transam/generic_xlog.c index 4b0c63817f..bbb542b322 100644 --- a/src/backend/access/transam/generic_xlog.c +++ b/src/backend/access/transam/generic_xlog.c @@ -482,10 +482,10 @@ generic_redo(XLogReaderState *record) uint8 block_id; /* Protect limited size of buffers[] array */ - Assert(record->max_block_id < MAX_GENERIC_XLOG_PAGES); + Assert(XLogRecMaxBlockId(record) < MAX_GENERIC_XLOG_PAGES); /* Iterate over blocks */ - for (block_id = 0; block_id <= record->max_block_id; block_id++) + for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) { XLogRedoAction action; @@ -525,7 +525,7 @@ generic_redo(XLogReaderState *record) } /* Changes are done: unlock and release all buffers */ - for (block_id = 0; block_id <= record->max_block_id; block_id++) + for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) { if (BufferIsValid(buffers[block_id])) UnlockReleaseBuffer(buffers[block_id]); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 0d2bd7a357..2b4e591736 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -7736,7 +7736,7 @@ xlog_redo(XLogReaderState *record) * resource manager needs to generate conflicts, it has to define a * separate WAL record type and redo routine. */ - for (uint8 block_id = 0; block_id <= record->max_block_id; block_id++) + for (uint8 block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) { Buffer buffer; diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index b7c06da255..1a8c651767 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -45,6 +45,7 @@ static bool allocate_recordbuf(XLogReaderState *state, uint32 reclength); static int ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr, int reqLen); static void XLogReaderInvalReadState(XLogReaderState *state); +static XLogPageReadResult XLogDecodeNextRecord(XLogReaderState *state, bool non_blocking); static bool ValidXLogRecordHeader(XLogReaderState *state, XLogRecPtr RecPtr, XLogRecPtr PrevRecPtr, XLogRecord *record, bool randAccess); static bool ValidXLogRecord(XLogReaderState *state, XLogRecord *record, @@ -56,6 +57,12 @@ static void WALOpenSegmentInit(WALOpenSegment *seg, WALSegmentContext *segcxt, /* size of the buffer allocated for error message. */ #define MAX_ERRORMSG_LEN 1000 +/* + * Default size; large enough that typical users of XLogReader won't often need + * to use the 'oversized' memory allocation code path. + */ +#define DEFAULT_DECODE_BUFFER_SIZE (64 * 1024) + /* * Construct a string in state->errormsg_buf explaining what's wrong with * the current record being read. @@ -70,6 +77,24 @@ report_invalid_record(XLogReaderState *state, const char *fmt,...) va_start(args, fmt); vsnprintf(state->errormsg_buf, MAX_ERRORMSG_LEN, fmt, args); va_end(args); + + state->errormsg_deferred = true; +} + +/* + * Set the size of the decoding buffer. A pointer to a caller supplied memory + * region may also be passed in, in which case non-oversized records will be + * decoded there. + */ +void +XLogReaderSetDecodeBuffer(XLogReaderState *state, void *buffer, size_t size) +{ + Assert(state->decode_buffer == NULL); + + state->decode_buffer = buffer; + state->decode_buffer_size = size; + state->decode_buffer_tail = buffer; + state->decode_buffer_head = buffer; } /* @@ -92,8 +117,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir, /* initialize caller-provided support functions */ state->routine = *routine; - state->max_block_id = -1; - /* * Permanently allocate readBuf. We do it this way, rather than just * making a static array, for two reasons: (1) no need to waste the @@ -144,18 +167,11 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir, void XLogReaderFree(XLogReaderState *state) { - int block_id; - if (state->seg.ws_file != -1) state->routine.segment_close(state); - for (block_id = 0; block_id <= XLR_MAX_BLOCK_ID; block_id++) - { - if (state->blocks[block_id].data) - pfree(state->blocks[block_id].data); - } - if (state->main_data) - pfree(state->main_data); + if (state->decode_buffer && state->free_decode_buffer) + pfree(state->decode_buffer); pfree(state->errormsg_buf); if (state->readRecordBuf) @@ -251,7 +267,132 @@ XLogBeginRead(XLogReaderState *state, XLogRecPtr RecPtr) /* Begin at the passed-in record pointer. */ state->EndRecPtr = RecPtr; + state->NextRecPtr = RecPtr; state->ReadRecPtr = InvalidXLogRecPtr; + state->DecodeRecPtr = InvalidXLogRecPtr; +} + +/* + * See if we can release the last record that was returned by + * XLogNextRecord(), if any, to free up space. + */ +void +XLogReleasePreviousRecord(XLogReaderState *state) +{ + DecodedXLogRecord *record; + + if (!state->record) + return; + + /* + * Remove it from the decoded record queue. It must be the oldest item + * decoded, decode_queue_head. + */ + record = state->record; + Assert(record == state->decode_queue_head); + state->record = NULL; + state->decode_queue_head = record->next; + + /* It might also be the newest item decoded, decode_queue_tail. */ + if (state->decode_queue_tail == record) + state->decode_queue_tail = NULL; + + /* Release the space. */ + if (unlikely(record->oversized)) + { + /* It's not in the the decode buffer, so free it to release space. */ + pfree(record); + } + else + { + /* It must be the head (oldest) record in the decode buffer. */ + Assert(state->decode_buffer_head == (char *) record); + + /* + * We need to update head to point to the next record that is in the + * decode buffer, if any, being careful to skip oversized ones + * (they're not in the decode buffer). + */ + record = record->next; + while (unlikely(record && record->oversized)) + record = record->next; + + if (record) + { + /* Adjust head to release space up to the next record. */ + state->decode_buffer_head = (char *) record; + } + else + { + /* + * Otherwise we might as well just reset head and tail to the + * start of the buffer space, because we're empty. This means + * we'll keep overwriting the same piece of memory if we're not + * doing any prefetching. + */ + state->decode_buffer_head = state->decode_buffer; + state->decode_buffer_tail = state->decode_buffer; + } + } +} + +/* + * Attempt to read an XLOG record. + * + * XLogBeginRead() or XLogFindNextRecord() and then XLogReadAhead() must be + * called before the first call to XLogNextRecord(). This functions returns + * records and errors that were put into an internal queue by XLogReadAhead(). + * + * On success, a record is returned. + * + * The returned record (or *errormsg) points to an internal buffer that's + * valid until the next call to XLogNextRecord. + */ +DecodedXLogRecord * +XLogNextRecord(XLogReaderState *state, char **errormsg) +{ + /* Release the last record returned by XLogNextRecord(). */ + XLogReleasePreviousRecord(state); + + if (state->decode_queue_head == NULL) + { + *errormsg = NULL; + if (state->errormsg_deferred) + { + if (state->errormsg_buf[0] != '\0') + *errormsg = state->errormsg_buf; + state->errormsg_deferred = false; + } + + /* + * state->EndRecPtr is expected to have been set by the last call to + * XLogBeginRead() or XLogNextRecord(), and is the location of the + * error. + */ + + return NULL; + } + + /* + * Record this as the most recent record returned, so that we'll release + * it next time. This also exposes it to the traditional + * XLogRecXXX(xlogreader) macros, which work with the decoder rather than + * the record for historical reasons. + */ + state->record = state->decode_queue_head; + + /* + * Update the pointers to the beginning and one-past-the-end of this + * record, again for the benefit of historical code that expected the + * decoder to track this rather than accessing these fields of the record + * itself. + */ + state->ReadRecPtr = state->record->lsn; + state->EndRecPtr = state->record->next_lsn; + + *errormsg = NULL; + + return state->record; } /* @@ -261,17 +402,132 @@ XLogBeginRead(XLogReaderState *state, XLogRecPtr RecPtr) * to XLogReadRecord(). * * If the page_read callback fails to read the requested data, NULL is - * returned. The callback is expected to have reported the error; errormsg - * is set to NULL. + * returned. The callback is expected to have reported the error; errormsg is + * set to NULL. * * If the reading fails for some other reason, NULL is also returned, and * *errormsg is set to a string with details of the failure. * - * The returned pointer (or *errormsg) points to an internal buffer that's - * valid until the next call to XLogReadRecord. + * On success, a record is returned. + * + * The returned record (or *errormsg) points to an internal buffer that's + * valid until the next call to XLogReadlRecord. */ XLogRecord * XLogReadRecord(XLogReaderState *state, char **errormsg) +{ + DecodedXLogRecord *decoded; + + /* + * Release last returned record, if there is one. We need to do this so + * that we can check for empty decode queue accurately. + */ + XLogReleasePreviousRecord(state); + + /* + * Call XLogReadAhead() in blocking mode to make sure there is something + * in the queue, though we don't use the result. + */ + if (!XLogReaderHasQueuedRecordOrError(state)) + XLogReadAhead(state, false /* nonblocking */ ); + + /* Consume the head record or error. */ + decoded = XLogNextRecord(state, errormsg); + if (decoded) + { + /* + * This function returns a pointer to the record's header, not the + * actual decoded record. The caller will access the decoded record + * through the XLogRecGetXXX() macros, which reach the decoded + * recorded as xlogreader->record. + */ + Assert(state->record == decoded); + return &decoded->header; + } + + return NULL; +} + +/* + * Allocate space for a decoded record. The only member of the returned + * object that is initialized is the 'oversized' flag, indicating that the + * decoded record wouldn't fit in the decode buffer and must eventually be + * freed explicitly. + * + * The caller is responsible for adjusting decode_buffer_tail with the real + * size after successfully decoding a record into this space. This way, if + * decoding fails, then there is nothing to undo unless the 'oversized' flag + * was set and pfree() must be called. + * + * Return NULL if there is no space in the decode buffer and allow_oversized + * is false, or if memory allocation fails for an oversized buffer. + */ +static DecodedXLogRecord * +XLogReadRecordAlloc(XLogReaderState *state, size_t xl_tot_len, bool allow_oversized) +{ + size_t required_space = DecodeXLogRecordRequiredSpace(xl_tot_len); + DecodedXLogRecord *decoded = NULL; + + /* Allocate a circular decode buffer if we don't have one already. */ + if (unlikely(state->decode_buffer == NULL)) + { + if (state->decode_buffer_size == 0) + state->decode_buffer_size = DEFAULT_DECODE_BUFFER_SIZE; + state->decode_buffer = palloc(state->decode_buffer_size); + state->decode_buffer_head = state->decode_buffer; + state->decode_buffer_tail = state->decode_buffer; + state->free_decode_buffer = true; + } + + /* Try to allocate space in the circular decode buffer. */ + if (state->decode_buffer_tail >= state->decode_buffer_head) + { + /* Empty, or tail is to the right of head. */ + if (state->decode_buffer_tail + required_space <= + state->decode_buffer + state->decode_buffer_size) + { + /* There is space between tail and end. */ + decoded = (DecodedXLogRecord *) state->decode_buffer_tail; + decoded->oversized = false; + return decoded; + } + else if (state->decode_buffer + required_space < + state->decode_buffer_head) + { + /* There is space between start and head. */ + decoded = (DecodedXLogRecord *) state->decode_buffer; + decoded->oversized = false; + return decoded; + } + } + else + { + /* Tail is to the left of head. */ + if (state->decode_buffer_tail + required_space < + state->decode_buffer_head) + { + /* There is space between tail and heade. */ + decoded = (DecodedXLogRecord *) state->decode_buffer_tail; + decoded->oversized = false; + return decoded; + } + } + + /* Not enough space in the decode buffer. Are we allowed to allocate? */ + if (allow_oversized) + { + decoded = palloc_extended(required_space, MCXT_ALLOC_NO_OOM); + if (decoded == NULL) + return NULL; + decoded->oversized = true; + return decoded; + } + + return NULL; +} + +static XLogPageReadResult +XLogDecodeNextRecord(XLogReaderState *state, bool nonblocking) { XLogRecPtr RecPtr; XLogRecord *record; @@ -284,6 +540,8 @@ XLogReadRecord(XLogReaderState *state, char **errormsg) bool assembled; bool gotheader; int readOff; + DecodedXLogRecord *decoded; + char *errormsg; /* not used */ /* * randAccess indicates whether to verify the previous-record pointer of @@ -293,21 +551,20 @@ XLogReadRecord(XLogReaderState *state, char **errormsg) randAccess = false; /* reset error state */ - *errormsg = NULL; state->errormsg_buf[0] = '\0'; + decoded = NULL; - ResetDecoder(state); state->abortedRecPtr = InvalidXLogRecPtr; state->missingContrecPtr = InvalidXLogRecPtr; - RecPtr = state->EndRecPtr; + RecPtr = state->NextRecPtr; - if (state->ReadRecPtr != InvalidXLogRecPtr) + if (state->DecodeRecPtr != InvalidXLogRecPtr) { /* read the record after the one we just read */ /* - * EndRecPtr is pointing to end+1 of the previous WAL record. If + * NextRecPtr is pointing to end+1 of the previous WAL record. If * we're at a page boundary, no more records can fit on the current * page. We must skip over the page header, but we can't do that until * we've read in the page, since the header size is variable. @@ -318,7 +575,7 @@ XLogReadRecord(XLogReaderState *state, char **errormsg) /* * Caller supplied a position to start at. * - * In this case, EndRecPtr should already be pointing to a valid + * In this case, NextRecPtr should already be pointing to a valid * record starting position. */ Assert(XRecOffIsValid(RecPtr)); @@ -326,6 +583,7 @@ XLogReadRecord(XLogReaderState *state, char **errormsg) } restart: + state->nonblocking = nonblocking; state->currRecPtr = RecPtr; assembled = false; @@ -339,7 +597,9 @@ restart: */ readOff = ReadPageInternal(state, targetPagePtr, Min(targetRecOff + SizeOfXLogRecord, XLOG_BLCKSZ)); - if (readOff < 0) + if (readOff == XLREAD_WOULDBLOCK) + return XLREAD_WOULDBLOCK; + else if (readOff < 0) goto err; /* @@ -395,7 +655,7 @@ restart: */ if (targetRecOff <= XLOG_BLCKSZ - SizeOfXLogRecord) { - if (!ValidXLogRecordHeader(state, RecPtr, state->ReadRecPtr, record, + if (!ValidXLogRecordHeader(state, RecPtr, state->DecodeRecPtr, record, randAccess)) goto err; gotheader = true; @@ -414,6 +674,31 @@ restart: gotheader = false; } + /* + * Find space to decode this record. Don't allow oversized allocation if + * the caller requested nonblocking. Otherwise, we *have* to try to + * decode the record now because the caller has nothing else to do, so + * allow an oversized record to be palloc'd if that turns out to be + * necessary. + */ + decoded = XLogReadRecordAlloc(state, + total_len, + !nonblocking /* allow_oversized */ ); + if (decoded == NULL) + { + /* + * There is no space in the decode buffer. The caller should help + * with that problem by consuming some records. + */ + if (nonblocking) + return XLREAD_WOULDBLOCK; + + /* We failed to allocate memory for an oversized record. */ + report_invalid_record(state, + "out of memory while trying to decode a record of length %u", total_len); + goto err; + } + len = XLOG_BLCKSZ - RecPtr % XLOG_BLCKSZ; if (total_len > len) { @@ -453,7 +738,9 @@ restart: Min(total_len - gotlen + SizeOfXLogShortPHD, XLOG_BLCKSZ)); - if (readOff < 0) + if (readOff == XLREAD_WOULDBLOCK) + return XLREAD_WOULDBLOCK; + else if (readOff < 0) goto err; Assert(SizeOfXLogShortPHD <= readOff); @@ -471,7 +758,6 @@ restart: if (pageHeader->xlp_info & XLP_FIRST_IS_OVERWRITE_CONTRECORD) { state->overwrittenRecPtr = RecPtr; - ResetDecoder(state); RecPtr = targetPagePtr; goto restart; } @@ -526,7 +812,7 @@ restart: if (!gotheader) { record = (XLogRecord *) state->readRecordBuf; - if (!ValidXLogRecordHeader(state, RecPtr, state->ReadRecPtr, + if (!ValidXLogRecordHeader(state, RecPtr, state->DecodeRecPtr, record, randAccess)) goto err; gotheader = true; @@ -540,8 +826,8 @@ restart: goto err; pageHeaderSize = XLogPageHeaderSize((XLogPageHeader) state->readBuf); - state->ReadRecPtr = RecPtr; - state->EndRecPtr = targetPagePtr + pageHeaderSize + state->DecodeRecPtr = RecPtr; + state->NextRecPtr = targetPagePtr + pageHeaderSize + MAXALIGN(pageHeader->xlp_rem_len); } else @@ -549,16 +835,18 @@ restart: /* Wait for the record data to become available */ readOff = ReadPageInternal(state, targetPagePtr, Min(targetRecOff + total_len, XLOG_BLCKSZ)); - if (readOff < 0) + if (readOff == XLREAD_WOULDBLOCK) + return XLREAD_WOULDBLOCK; + else if (readOff < 0) goto err; /* Record does not cross a page boundary */ if (!ValidXLogRecord(state, record, RecPtr)) goto err; - state->EndRecPtr = RecPtr + MAXALIGN(total_len); + state->NextRecPtr = RecPtr + MAXALIGN(total_len); - state->ReadRecPtr = RecPtr; + state->DecodeRecPtr = RecPtr; } /* @@ -568,14 +856,40 @@ restart: (record->xl_info & ~XLR_INFO_MASK) == XLOG_SWITCH) { /* Pretend it extends to end of segment */ - state->EndRecPtr += state->segcxt.ws_segsize - 1; - state->EndRecPtr -= XLogSegmentOffset(state->EndRecPtr, state->segcxt.ws_segsize); + state->NextRecPtr += state->segcxt.ws_segsize - 1; + state->NextRecPtr -= XLogSegmentOffset(state->NextRecPtr, state->segcxt.ws_segsize); } - if (DecodeXLogRecord(state, record, errormsg)) - return record; + if (DecodeXLogRecord(state, decoded, record, RecPtr, &errormsg)) + { + /* Record the location of the next record. */ + decoded->next_lsn = state->NextRecPtr; + + /* + * If it's in the decode buffer, mark the decode buffer space as + * occupied. + */ + if (!decoded->oversized) + { + /* The new decode buffer head must be MAXALIGNed. */ + Assert(decoded->size == MAXALIGN(decoded->size)); + if ((char *) decoded == state->decode_buffer) + state->decode_buffer_tail = state->decode_buffer + decoded->size; + else + state->decode_buffer_tail += decoded->size; + } + + /* Insert it into the queue of decoded records. */ + Assert(state->decode_queue_tail != decoded); + if (state->decode_queue_tail) + state->decode_queue_tail->next = decoded; + state->decode_queue_tail = decoded; + if (!state->decode_queue_head) + state->decode_queue_head = decoded; + return XLREAD_SUCCESS; + } else - return NULL; + return XLREAD_FAIL; err: if (assembled) @@ -593,14 +907,46 @@ err: state->missingContrecPtr = targetPagePtr; } + if (decoded && decoded->oversized) + pfree(decoded); + /* * Invalidate the read state. We might read from a different source after * failure. */ XLogReaderInvalReadState(state); - if (state->errormsg_buf[0] != '\0') - *errormsg = state->errormsg_buf; + /* + * If an error was written to errmsg_buf, it'll be returned to the caller + * of XLogReadRecord() after all successfully decoded records from the + * read queue. + */ + + return XLREAD_FAIL; +} + +/* + * Try to decode the next available record, and return it. The record will + * also be returned to XLogNextRecord(), which must be called to 'consume' + * each record. + * + * If nonblocking is true, may return NULL due to lack of data or WAL decoding + * space. + */ +DecodedXLogRecord * +XLogReadAhead(XLogReaderState *state, bool nonblocking) +{ + XLogPageReadResult result; + + if (state->errormsg_deferred) + return NULL; + + result = XLogDecodeNextRecord(state, nonblocking); + if (result == XLREAD_SUCCESS) + { + Assert(state->decode_queue_tail != NULL); + return state->decode_queue_tail; + } return NULL; } @@ -609,8 +955,14 @@ err: * Read a single xlog page including at least [pageptr, reqLen] of valid data * via the page_read() callback. * - * Returns -1 if the required page cannot be read for some reason; errormsg_buf - * is set in that case (unless the error occurs in the page_read callback). + * Returns XLREAD_FAIL if the required page cannot be read for some + * reason; errormsg_buf is set in that case (unless the error occurs in the + * page_read callback). + * + * Returns XLREAD_WOULDBLOCK if he requested data can't be read without + * waiting. This can be returned only if the installed page_read callback + * respects the state->nonblocking flag, and cannot read the requested data + * immediately. * * We fetch the page from a reader-local cache if we know we have the required * data and if there hasn't been any error since caching the data. @@ -652,7 +1004,9 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr, int reqLen) readLen = state->routine.page_read(state, targetSegmentPtr, XLOG_BLCKSZ, state->currRecPtr, state->readBuf); - if (readLen < 0) + if (readLen == XLREAD_WOULDBLOCK) + return XLREAD_WOULDBLOCK; + else if (readLen < 0) goto err; /* we can be sure to have enough WAL available, we scrolled back */ @@ -670,7 +1024,9 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr, int reqLen) readLen = state->routine.page_read(state, pageptr, Max(reqLen, SizeOfXLogShortPHD), state->currRecPtr, state->readBuf); - if (readLen < 0) + if (readLen == XLREAD_WOULDBLOCK) + return XLREAD_WOULDBLOCK; + else if (readLen < 0) goto err; Assert(readLen <= XLOG_BLCKSZ); @@ -689,7 +1045,9 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr, int reqLen) readLen = state->routine.page_read(state, pageptr, XLogPageHeaderSize(hdr), state->currRecPtr, state->readBuf); - if (readLen < 0) + if (readLen == XLREAD_WOULDBLOCK) + return XLREAD_WOULDBLOCK; + else if (readLen < 0) goto err; } @@ -707,8 +1065,12 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr, int reqLen) return readLen; err: - XLogReaderInvalReadState(state); - return -1; + if (state->errormsg_buf[0] != '\0') + { + state->errormsg_deferred = true; + XLogReaderInvalReadState(state); + } + return XLREAD_FAIL; } /* @@ -987,6 +1349,9 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr) Assert(!XLogRecPtrIsInvalid(RecPtr)); + /* Make sure ReadPageInternal() can't return XLREAD_WOULDBLOCK. */ + state->nonblocking = false; + /* * skip over potential continuation data, keeping in mind that it may span * multiple pages @@ -1065,7 +1430,7 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr) * or we just jumped over the remaining data of a continuation. */ XLogBeginRead(state, tmpRecPtr); - while (XLogReadRecord(state, &errormsg) != NULL) + while (XLogReadRecord(state, &errormsg)) { /* past the record we've found, break out */ if (RecPtr <= state->ReadRecPtr) @@ -1187,34 +1552,83 @@ WALRead(XLogReaderState *state, * ---------------------------------------- */ -/* private function to reset the state between records */ +/* + * Private function to reset the state, forgetting all decoded records, if we + * are asked to move to a new read position. + */ static void ResetDecoder(XLogReaderState *state) { - int block_id; - - state->decoded_record = NULL; - - state->main_data_len = 0; + DecodedXLogRecord *r; - for (block_id = 0; block_id <= state->max_block_id; block_id++) + /* Reset the decoded record queue, freeing any oversized records. */ + while ((r = state->decode_queue_head) != NULL) { - state->blocks[block_id].in_use = false; - state->blocks[block_id].has_image = false; - state->blocks[block_id].has_data = false; - state->blocks[block_id].apply_image = false; + state->decode_queue_head = r->next; + if (r->oversized) + pfree(r); } - state->max_block_id = -1; + state->decode_queue_tail = NULL; + state->decode_queue_head = NULL; + state->record = NULL; + + /* Reset the decode buffer to empty. */ + state->decode_buffer_tail = state->decode_buffer; + state->decode_buffer_head = state->decode_buffer; + + /* Clear error state. */ + state->errormsg_buf[0] = '\0'; + state->errormsg_deferred = false; } /* - * Decode the previously read record. + * Compute the maximum possible amount of padding that could be required to + * decode a record, given xl_tot_len from the record's header. This is the + * amount of output buffer space that we need to decode a record, though we + * might not finish up using it all. + * + * This computation is pessimistic and assumes the maximum possible number of + * blocks, due to lack of better information. + */ +size_t +DecodeXLogRecordRequiredSpace(size_t xl_tot_len) +{ + size_t size = 0; + + /* Account for the fixed size part of the decoded record struct. */ + size += offsetof(DecodedXLogRecord, blocks[0]); + /* Account for the flexible blocks array of maximum possible size. */ + size += sizeof(DecodedBkpBlock) * (XLR_MAX_BLOCK_ID + 1); + /* Account for all the raw main and block data. */ + size += xl_tot_len; + /* We might insert padding before main_data. */ + size += (MAXIMUM_ALIGNOF - 1); + /* We might insert padding before each block's data. */ + size += (MAXIMUM_ALIGNOF - 1) * (XLR_MAX_BLOCK_ID + 1); + /* We might insert padding at the end. */ + size += (MAXIMUM_ALIGNOF - 1); + + return size; +} + +/* + * Decode a record. "decoded" must point to a MAXALIGNed memory area that has + * space for at least DecodeXLogRecordRequiredSpace(record) bytes. On + * success, decoded->size contains the actual space occupied by the decoded + * record, which may turn out to be less. + * + * Only decoded->oversized member must be initialized already, and will not be + * modified. Other members will be initialized as required. * * On error, a human-readable error message is returned in *errormsg, and * the return value is false. */ bool -DecodeXLogRecord(XLogReaderState *state, XLogRecord *record, char **errormsg) +DecodeXLogRecord(XLogReaderState *state, + DecodedXLogRecord *decoded, + XLogRecord *record, + XLogRecPtr lsn, + char **errormsg) { /* * read next _size bytes from record buffer, but check for overrun first. @@ -1229,17 +1643,20 @@ DecodeXLogRecord(XLogReaderState *state, XLogRecord *record, char **errormsg) } while(0) char *ptr; + char *out; uint32 remaining; uint32 datatotal; RelFileNode *rnode = NULL; uint8 block_id; - ResetDecoder(state); - - state->decoded_record = record; - state->record_origin = InvalidRepOriginId; - state->toplevel_xid = InvalidTransactionId; - + decoded->header = *record; + decoded->lsn = lsn; + decoded->next = NULL; + decoded->record_origin = InvalidRepOriginId; + decoded->toplevel_xid = InvalidTransactionId; + decoded->main_data = NULL; + decoded->main_data_len = 0; + decoded->max_block_id = -1; ptr = (char *) record; ptr += SizeOfXLogRecord; remaining = record->xl_tot_len - SizeOfXLogRecord; @@ -1257,7 +1674,7 @@ DecodeXLogRecord(XLogReaderState *state, XLogRecord *record, char **errormsg) COPY_HEADER_FIELD(&main_data_len, sizeof(uint8)); - state->main_data_len = main_data_len; + decoded->main_data_len = main_data_len; datatotal += main_data_len; break; /* by convention, the main data fragment is * always last */ @@ -1268,18 +1685,18 @@ DecodeXLogRecord(XLogReaderState *state, XLogRecord *record, char **errormsg) uint32 main_data_len; COPY_HEADER_FIELD(&main_data_len, sizeof(uint32)); - state->main_data_len = main_data_len; + decoded->main_data_len = main_data_len; datatotal += main_data_len; break; /* by convention, the main data fragment is * always last */ } else if (block_id == XLR_BLOCK_ID_ORIGIN) { - COPY_HEADER_FIELD(&state->record_origin, sizeof(RepOriginId)); + COPY_HEADER_FIELD(&decoded->record_origin, sizeof(RepOriginId)); } else if (block_id == XLR_BLOCK_ID_TOPLEVEL_XID) { - COPY_HEADER_FIELD(&state->toplevel_xid, sizeof(TransactionId)); + COPY_HEADER_FIELD(&decoded->toplevel_xid, sizeof(TransactionId)); } else if (block_id <= XLR_MAX_BLOCK_ID) { @@ -1287,7 +1704,11 @@ DecodeXLogRecord(XLogReaderState *state, XLogRecord *record, char **errormsg) DecodedBkpBlock *blk; uint8 fork_flags; - if (block_id <= state->max_block_id) + /* mark any intervening block IDs as not in use */ + for (int i = decoded->max_block_id + 1; i < block_id; ++i) + decoded->blocks[i].in_use = false; + + if (block_id <= decoded->max_block_id) { report_invalid_record(state, "out-of-order block_id %u at %X/%X", @@ -1295,9 +1716,9 @@ DecodeXLogRecord(XLogReaderState *state, XLogRecord *record, char **errormsg) LSN_FORMAT_ARGS(state->ReadRecPtr)); goto err; } - state->max_block_id = block_id; + decoded->max_block_id = block_id; - blk = &state->blocks[block_id]; + blk = &decoded->blocks[block_id]; blk->in_use = true; blk->apply_image = false; @@ -1440,17 +1861,18 @@ DecodeXLogRecord(XLogReaderState *state, XLogRecord *record, char **errormsg) /* * Ok, we've parsed the fragment headers, and verified that the total * length of the payload in the fragments is equal to the amount of data - * left. Copy the data of each fragment to a separate buffer. - * - * We could just set up pointers into readRecordBuf, but we want to align - * the data for the convenience of the callers. Backup images are not - * copied, however; they don't need alignment. + * left. Copy the data of each fragment to contiguous space after the + * blocks array, inserting alignment padding before the data fragments so + * they can be cast to struct pointers by REDO routines. */ + out = ((char *) decoded) + + offsetof(DecodedXLogRecord, blocks) + + sizeof(decoded->blocks[0]) * (decoded->max_block_id + 1); /* block data first */ - for (block_id = 0; block_id <= state->max_block_id; block_id++) + for (block_id = 0; block_id <= decoded->max_block_id; block_id++) { - DecodedBkpBlock *blk = &state->blocks[block_id]; + DecodedBkpBlock *blk = &decoded->blocks[block_id]; if (!blk->in_use) continue; @@ -1459,58 +1881,37 @@ DecodeXLogRecord(XLogReaderState *state, XLogRecord *record, char **errormsg) if (blk->has_image) { - blk->bkp_image = ptr; + /* no need to align image */ + blk->bkp_image = out; + memcpy(out, ptr, blk->bimg_len); ptr += blk->bimg_len; + out += blk->bimg_len; } if (blk->has_data) { - if (!blk->data || blk->data_len > blk->data_bufsz) - { - if (blk->data) - pfree(blk->data); - - /* - * Force the initial request to be BLCKSZ so that we don't - * waste time with lots of trips through this stanza as a - * result of WAL compression. - */ - blk->data_bufsz = MAXALIGN(Max(blk->data_len, BLCKSZ)); - blk->data = palloc(blk->data_bufsz); - } + out = (char *) MAXALIGN(out); + blk->data = out; memcpy(blk->data, ptr, blk->data_len); ptr += blk->data_len; + out += blk->data_len; } } /* and finally, the main data */ - if (state->main_data_len > 0) + if (decoded->main_data_len > 0) { - if (!state->main_data || state->main_data_len > state->main_data_bufsz) - { - if (state->main_data) - pfree(state->main_data); - - /* - * main_data_bufsz must be MAXALIGN'ed. In many xlog record - * types, we omit trailing struct padding on-disk to save a few - * bytes; but compilers may generate accesses to the xlog struct - * that assume that padding bytes are present. If the palloc - * request is not large enough to include such padding bytes then - * we'll get valgrind complaints due to otherwise-harmless fetches - * of the padding bytes. - * - * In addition, force the initial request to be reasonably large - * so that we don't waste time with lots of trips through this - * stanza. BLCKSZ / 2 seems like a good compromise choice. - */ - state->main_data_bufsz = MAXALIGN(Max(state->main_data_len, - BLCKSZ / 2)); - state->main_data = palloc(state->main_data_bufsz); - } - memcpy(state->main_data, ptr, state->main_data_len); - ptr += state->main_data_len; + out = (char *) MAXALIGN(out); + decoded->main_data = out; + memcpy(decoded->main_data, ptr, decoded->main_data_len); + ptr += decoded->main_data_len; + out += decoded->main_data_len; } + /* Report the actual size we used. */ + decoded->size = MAXALIGN(out - (char *) decoded); + Assert(DecodeXLogRecordRequiredSpace(record->xl_tot_len) >= + decoded->size); + return true; shortdata_err: @@ -1536,10 +1937,11 @@ XLogRecGetBlockTag(XLogReaderState *record, uint8 block_id, { DecodedBkpBlock *bkpb; - if (!record->blocks[block_id].in_use) + if (block_id > record->record->max_block_id || + !record->record->blocks[block_id].in_use) return false; - bkpb = &record->blocks[block_id]; + bkpb = &record->record->blocks[block_id]; if (rnode) *rnode = bkpb->rnode; if (forknum) @@ -1559,10 +1961,11 @@ XLogRecGetBlockData(XLogReaderState *record, uint8 block_id, Size *len) { DecodedBkpBlock *bkpb; - if (!record->blocks[block_id].in_use) + if (block_id > record->record->max_block_id || + !record->record->blocks[block_id].in_use) return NULL; - bkpb = &record->blocks[block_id]; + bkpb = &record->record->blocks[block_id]; if (!bkpb->has_data) { @@ -1590,12 +1993,13 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page) char *ptr; PGAlignedBlock tmp; - if (!record->blocks[block_id].in_use) + if (block_id > record->record->max_block_id || + !record->record->blocks[block_id].in_use) return false; - if (!record->blocks[block_id].has_image) + if (!record->record->blocks[block_id].has_image) return false; - bkpb = &record->blocks[block_id]; + bkpb = &record->record->blocks[block_id]; ptr = bkpb->bkp_image; if (BKPIMAGE_COMPRESSED(bkpb->bimg_info)) diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index f9f212680b..9feea3e6ec 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -2139,7 +2139,7 @@ xlog_block_info(StringInfo buf, XLogReaderState *record) int block_id; /* decode block references */ - for (block_id = 0; block_id <= record->max_block_id; block_id++) + for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) { RelFileNode rnode; ForkNumber forknum; @@ -2271,7 +2271,7 @@ verifyBackupPageConsistency(XLogReaderState *record) Assert((XLogRecGetInfo(record) & XLR_CHECK_CONSISTENCY) != 0); - for (block_id = 0; block_id <= record->max_block_id; block_id++) + for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) { Buffer buf; Page page; diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c index 54d5f20734..511f2f186f 100644 --- a/src/backend/access/transam/xlogutils.c +++ b/src/backend/access/transam/xlogutils.c @@ -370,7 +370,7 @@ XLogReadBufferForRedoExtended(XLogReaderState *record, * going to initialize it. And vice versa. */ zeromode = (mode == RBM_ZERO_AND_LOCK || mode == RBM_ZERO_AND_CLEANUP_LOCK); - willinit = (record->blocks[block_id].flags & BKPBLOCK_WILL_INIT) != 0; + willinit = (XLogRecGetBlock(record, block_id)->flags & BKPBLOCK_WILL_INIT) != 0; if (willinit && !zeromode) elog(PANIC, "block with WILL_INIT flag in WAL record must be zeroed by redo routine"); if (!willinit && zeromode) diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c index 8c00a73cb9..77bc7aea7a 100644 --- a/src/backend/replication/logical/decode.c +++ b/src/backend/replication/logical/decode.c @@ -111,7 +111,7 @@ LogicalDecodingProcessRecord(LogicalDecodingContext *ctx, XLogReaderState *recor { ReorderBufferAssignChild(ctx->reorder, txid, - record->decoded_record->xl_xid, + XLogRecGetXid(record), buf.origptr); } diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index 56df08c64f..7cfa169e9b 100644 --- a/src/bin/pg_rewind/parsexlog.c +++ b/src/bin/pg_rewind/parsexlog.c @@ -432,7 +432,7 @@ extractPageInfo(XLogReaderState *record) RmgrNames[rmid], info); } - for (block_id = 0; block_id <= record->max_block_id; block_id++) + for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) { RelFileNode rnode; ForkNumber forknum; diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c index f128050b4e..fc081adfb8 100644 --- a/src/bin/pg_waldump/pg_waldump.c +++ b/src/bin/pg_waldump/pg_waldump.c @@ -403,14 +403,13 @@ XLogDumpRecordLen(XLogReaderState *record, uint32 *rec_len, uint32 *fpi_len) * Calculate the amount of FPI data in the record. * * XXX: We peek into xlogreader's private decoded backup blocks for the - * bimg_len indicating the length of FPI data. It doesn't seem worth it to - * add an accessor macro for this. + * bimg_len indicating the length of FPI data. */ *fpi_len = 0; - for (block_id = 0; block_id <= record->max_block_id; block_id++) + for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) { if (XLogRecHasBlockImage(record, block_id)) - *fpi_len += record->blocks[block_id].bimg_len; + *fpi_len += XLogRecGetBlock(record, block_id)->bimg_len; } /* @@ -508,7 +507,7 @@ XLogDumpDisplayRecord(XLogDumpConfig *config, XLogReaderState *record) if (!config->bkp_details) { /* print block references (short format) */ - for (block_id = 0; block_id <= record->max_block_id; block_id++) + for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) { if (!XLogRecHasBlockRef(record, block_id)) continue; @@ -539,7 +538,7 @@ XLogDumpDisplayRecord(XLogDumpConfig *config, XLogReaderState *record) { /* print block references (detailed format) */ putchar('\n'); - for (block_id = 0; block_id <= record->max_block_id; block_id++) + for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) { if (!XLogRecHasBlockRef(record, block_id)) continue; @@ -552,7 +551,7 @@ XLogDumpDisplayRecord(XLogDumpConfig *config, XLogReaderState *record) blk); if (XLogRecHasBlockImage(record, block_id)) { - uint8 bimg_info = record->blocks[block_id].bimg_info; + uint8 bimg_info = XLogRecGetBlock(record, block_id)->bimg_info; if (BKPIMAGE_COMPRESSED(bimg_info)) { @@ -571,11 +570,11 @@ XLogDumpDisplayRecord(XLogDumpConfig *config, XLogReaderState *record) "compression saved: %u, method: %s", XLogRecBlockImageApply(record, block_id) ? "" : " for WAL verification", - record->blocks[block_id].hole_offset, - record->blocks[block_id].hole_length, + XLogRecGetBlock(record, block_id)->hole_offset, + XLogRecGetBlock(record, block_id)->hole_length, BLCKSZ - - record->blocks[block_id].hole_length - - record->blocks[block_id].bimg_len, + XLogRecGetBlock(record, block_id)->hole_length - + XLogRecGetBlock(record, block_id)->bimg_len, method); } else @@ -583,8 +582,8 @@ XLogDumpDisplayRecord(XLogDumpConfig *config, XLogReaderState *record) printf(" (FPW%s); hole: offset: %u, length: %u", XLogRecBlockImageApply(record, block_id) ? "" : " for WAL verification", - record->blocks[block_id].hole_offset, - record->blocks[block_id].hole_length); + XLogRecGetBlock(record, block_id)->hole_offset, + XLogRecGetBlock(record, block_id)->hole_length); } } putchar('\n'); diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h index 477f0efe26..d1f364f4e8 100644 --- a/src/include/access/xlogreader.h +++ b/src/include/access/xlogreader.h @@ -144,6 +144,30 @@ typedef struct uint16 data_bufsz; } DecodedBkpBlock; +/* + * The decoded contents of a record. This occupies a contiguous region of + * memory, with main_data and blocks[n].data pointing to memory after the + * members declared here. + */ +typedef struct DecodedXLogRecord +{ + /* Private member used for resource management. */ + size_t size; /* total size of decoded record */ + bool oversized; /* outside the regular decode buffer? */ + struct DecodedXLogRecord *next; /* decoded record queue link */ + + /* Public members. */ + XLogRecPtr lsn; /* location */ + XLogRecPtr next_lsn; /* location of next record */ + XLogRecord header; /* header */ + RepOriginId record_origin; + TransactionId toplevel_xid; /* XID of top-level transaction */ + char *main_data; /* record's main data portion */ + uint32 main_data_len; /* main data portion's length */ + int max_block_id; /* highest block_id in use (-1 if none) */ + DecodedBkpBlock blocks[FLEXIBLE_ARRAY_MEMBER]; +} DecodedXLogRecord; + struct XLogReaderState { /* @@ -171,6 +195,9 @@ struct XLogReaderState * Start and end point of last record read. EndRecPtr is also used as the * position to read next. Calling XLogBeginRead() sets EndRecPtr to the * starting position and ReadRecPtr to invalid. + * + * Start and end point of last record returned by XLogReadRecord(). These + * are also available as record->lsn and record->next_lsn. */ XLogRecPtr ReadRecPtr; /* start of last record read */ XLogRecPtr EndRecPtr; /* end+1 of last record read */ @@ -192,27 +219,43 @@ struct XLogReaderState * Use XLogRecGet* functions to investigate the record; these fields * should not be accessed directly. * ---------------------------------------- + * Start and end point of the last record read and decoded by + * XLogReadRecordInternal(). NextRecPtr is also used as the position to + * decode next. Calling XLogBeginRead() sets NextRecPtr and EndRecPtr to + * the requested starting position. */ - XLogRecord *decoded_record; /* currently decoded record */ + XLogRecPtr DecodeRecPtr; /* start of last record decoded */ + XLogRecPtr NextRecPtr; /* end+1 of last record decoded */ + XLogRecPtr PrevRecPtr; /* start of previous record decoded */ - char *main_data; /* record's main data portion */ - uint32 main_data_len; /* main data portion's length */ - uint32 main_data_bufsz; /* allocated size of the buffer */ - - RepOriginId record_origin; - - TransactionId toplevel_xid; /* XID of top-level transaction */ - - /* information about blocks referenced by the record. */ - DecodedBkpBlock blocks[XLR_MAX_BLOCK_ID + 1]; - - int max_block_id; /* highest block_id in use (-1 if none) */ + /* Last record returned by XLogReadRecord(). */ + DecodedXLogRecord *record; /* ---------------------------------------- * private/internal state * ---------------------------------------- */ + /* + * Buffer for decoded records. This is a circular buffer, though + * individual records can't be split in the middle, so some space is often + * wasted at the end. Oversized records that don't fit in this space are + * allocated separately. + */ + char *decode_buffer; + size_t decode_buffer_size; + bool free_decode_buffer; /* need to free? */ + char *decode_buffer_head; /* data is read from the head */ + char *decode_buffer_tail; /* new data is written at the tail */ + + /* + * Queue of records that have been decoded. This is a linked list that + * usually consists of consecutive records in decode_buffer, but may also + * contain oversized records allocated with palloc(). + */ + DecodedXLogRecord *decode_queue_head; /* oldest decoded record */ + DecodedXLogRecord *decode_queue_tail; /* newest decoded record */ + /* * Buffer for currently read page (XLOG_BLCKSZ bytes, valid up to at least * readLen bytes) @@ -262,8 +305,25 @@ struct XLogReaderState /* Buffer to hold error message */ char *errormsg_buf; + bool errormsg_deferred; + + /* + * Flag to indicate to XLogPageReadCB that it should not block, during + * read ahead. + */ + bool nonblocking; }; +/* + * Check if the XLogNextRecord() has any more queued records or errors. This + * can be used by a read_page callback to decide whether it should block. + */ +static inline bool +XLogReaderHasQueuedRecordOrError(XLogReaderState *state) +{ + return (state->decode_queue_head != NULL) || state->errormsg_deferred; +} + /* Get a new XLogReader */ extern XLogReaderState *XLogReaderAllocate(int wal_segment_size, const char *waldir, @@ -274,16 +334,40 @@ extern XLogReaderRoutine *LocalXLogReaderRoutine(void); /* Free an XLogReader */ extern void XLogReaderFree(XLogReaderState *state); +/* Optionally provide a circular decoding buffer to allow readahead. */ +extern void XLogReaderSetDecodeBuffer(XLogReaderState *state, + void *buffer, + size_t size); + /* Position the XLogReader to given record */ extern void XLogBeginRead(XLogReaderState *state, XLogRecPtr RecPtr); #ifdef FRONTEND extern XLogRecPtr XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr); #endif /* FRONTEND */ +/* Return values from XLogPageReadCB. */ +typedef enum XLogPageReadResult +{ + XLREAD_SUCCESS = 0, /* record is successfully read */ + XLREAD_FAIL = -1, /* failed during reading a record */ + XLREAD_WOULDBLOCK = -2 /* nonblocking mode only, no data */ +} XLogPageReadResult; + /* Read the next XLog record. Returns NULL on end-of-WAL or failure */ -extern struct XLogRecord *XLogReadRecord(XLogReaderState *state, +extern XLogRecord *XLogReadRecord(XLogReaderState *state, + char **errormsg); + +/* Consume the next record or error. */ +extern DecodedXLogRecord *XLogNextRecord(XLogReaderState *state, char **errormsg); +/* Release the previously returned record, if necessary. */ +extern void XLogReleasePreviousRecord(XLogReaderState *state); + +/* Try to read ahead, if there is data and space. */ +extern DecodedXLogRecord *XLogReadAhead(XLogReaderState *state, + bool nonblocking); + /* Validate a page */ extern bool XLogReaderValidatePageHeader(XLogReaderState *state, XLogRecPtr recptr, char *phdr); @@ -307,25 +391,32 @@ extern bool WALRead(XLogReaderState *state, /* Functions for decoding an XLogRecord */ -extern bool DecodeXLogRecord(XLogReaderState *state, XLogRecord *record, +extern size_t DecodeXLogRecordRequiredSpace(size_t xl_tot_len); +extern bool DecodeXLogRecord(XLogReaderState *state, + DecodedXLogRecord *decoded, + XLogRecord *record, + XLogRecPtr lsn, char **errmsg); -#define XLogRecGetTotalLen(decoder) ((decoder)->decoded_record->xl_tot_len) -#define XLogRecGetPrev(decoder) ((decoder)->decoded_record->xl_prev) -#define XLogRecGetInfo(decoder) ((decoder)->decoded_record->xl_info) -#define XLogRecGetRmid(decoder) ((decoder)->decoded_record->xl_rmid) -#define XLogRecGetXid(decoder) ((decoder)->decoded_record->xl_xid) -#define XLogRecGetOrigin(decoder) ((decoder)->record_origin) -#define XLogRecGetTopXid(decoder) ((decoder)->toplevel_xid) -#define XLogRecGetData(decoder) ((decoder)->main_data) -#define XLogRecGetDataLen(decoder) ((decoder)->main_data_len) -#define XLogRecHasAnyBlockRefs(decoder) ((decoder)->max_block_id >= 0) -#define XLogRecHasBlockRef(decoder, block_id) \ - ((decoder)->blocks[block_id].in_use) -#define XLogRecHasBlockImage(decoder, block_id) \ - ((decoder)->blocks[block_id].has_image) -#define XLogRecBlockImageApply(decoder, block_id) \ - ((decoder)->blocks[block_id].apply_image) +#define XLogRecGetTotalLen(decoder) ((decoder)->record->header.xl_tot_len) +#define XLogRecGetPrev(decoder) ((decoder)->record->header.xl_prev) +#define XLogRecGetInfo(decoder) ((decoder)->record->header.xl_info) +#define XLogRecGetRmid(decoder) ((decoder)->record->header.xl_rmid) +#define XLogRecGetXid(decoder) ((decoder)->record->header.xl_xid) +#define XLogRecGetOrigin(decoder) ((decoder)->record->record_origin) +#define XLogRecGetTopXid(decoder) ((decoder)->record->toplevel_xid) +#define XLogRecGetData(decoder) ((decoder)->record->main_data) +#define XLogRecGetDataLen(decoder) ((decoder)->record->main_data_len) +#define XLogRecHasAnyBlockRefs(decoder) ((decoder)->record->max_block_id >= 0) +#define XLogRecMaxBlockId(decoder) ((decoder)->record->max_block_id) +#define XLogRecGetBlock(decoder, i) (&(decoder)->record->blocks[(i)]) +#define XLogRecHasBlockRef(decoder, block_id) \ + (((decoder)->record->max_block_id >= (block_id)) && \ + ((decoder)->record->blocks[block_id].in_use)) +#define XLogRecHasBlockImage(decoder, block_id) \ + ((decoder)->record->blocks[block_id].has_image) +#define XLogRecBlockImageApply(decoder, block_id) \ + ((decoder)->record->blocks[block_id].apply_image) #ifndef FRONTEND extern FullTransactionId XLogRecGetFullXid(XLogReaderState *record); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index eaf3e7a8d4..f57f7e0f53 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -533,6 +533,7 @@ DeadLockState DeallocateStmt DeclareCursorStmt DecodedBkpBlock +DecodedXLogRecord DecodingOutputState DefElem DefElemAction @@ -2939,6 +2940,7 @@ XLogPageHeader XLogPageHeaderData XLogPageReadCB XLogPageReadPrivate +XLogPageReadResult XLogReaderRoutine XLogReaderState XLogRecData -- 2.30.2 [text/x-patch] v22-0002-Prefetch-referenced-data-in-recovery-take-II.patch (70.3K, ../../CA+hUKGKusWunqOd9En+FkT+US_kuDK-MSZyXHCr=OTwRw8Ah1A@mail.gmail.com/3-v22-0002-Prefetch-referenced-data-in-recovery-take-II.patch) download | inline diff: From 05fc2981d5c6d5b1603080c45b11d96562e9f44d Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Tue, 9 Nov 2021 16:43:45 +1300 Subject: [PATCH v22 2/2] Prefetch referenced data in recovery, take II. Introduce a new GUC recovery_prefetch, disabled by default. When enabled, look ahead in the WAL and try to initiate asynchronous reading of referenced data blocks that are not yet cached in our buffer pool. For now, this is done with posix_fadvise(), which has several caveats. Better mechanisms will follow in later work on the I/O subsystem. The GUC maintenance_io_concurrency is used to limit the number of concurrent I/Os we allow ourselves to initiate, based on pessimistic heuristics used to infer that I/Os have begun and completed. The GUC wal_decode_buffer_size limits the maximum distance we are prepared to read ahead in the WAL to find uncached blocks. Reviewed-by: Tomas Vondra <[email protected]> Reviewed-by: Alvaro Herrera <[email protected]> (earlier version) Reviewed-by: Andres Freund <[email protected]> (earlier version) Reviewed-by: Justin Pryzby <[email protected]> (earlier version) Tested-by: Tomas Vondra <[email protected]> (earlier version) Tested-by: Jakub Wartak <[email protected]> (earlier version) Tested-by: Dmitry Dolgov <[email protected]> (earlier version) Tested-by: Sait Talha Nisanci <[email protected]> (earlier version) Discussion: https://postgr.es/m/CA%2BhUKGJ4VJN8ttxScUFM8dOKX0BrBiboo5uz1cq%3DAovOddfHpA%40mail.gmail.com --- doc/src/sgml/config.sgml | 61 ++ doc/src/sgml/monitoring.sgml | 77 +- doc/src/sgml/wal.sgml | 12 + src/backend/access/transam/Makefile | 1 + src/backend/access/transam/xlog.c | 2 + src/backend/access/transam/xlogprefetcher.c | 962 ++++++++++++++++++ src/backend/access/transam/xlogreader.c | 13 + src/backend/access/transam/xlogrecovery.c | 160 ++- src/backend/access/transam/xlogutils.c | 27 +- src/backend/catalog/system_views.sql | 13 + src/backend/storage/freespace/freespace.c | 3 +- src/backend/storage/ipc/ipci.c | 3 + src/backend/utils/misc/guc.c | 39 +- src/backend/utils/misc/postgresql.conf.sample | 5 + src/include/access/xlog.h | 1 + src/include/access/xlogprefetcher.h | 43 + src/include/access/xlogreader.h | 8 + src/include/access/xlogutils.h | 3 +- src/include/catalog/pg_proc.dat | 8 + src/include/utils/guc.h | 4 + src/test/regress/expected/rules.out | 10 + src/tools/pgindent/typedefs.list | 7 + 22 files changed, 1400 insertions(+), 62 deletions(-) create mode 100644 src/backend/access/transam/xlogprefetcher.c create mode 100644 src/include/access/xlogprefetcher.h diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 5612e80453..4244e0a7bb 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -3644,6 +3644,67 @@ include_dir 'conf.d' </variablelist> </sect2> + <sect2 id="runtime-config-wal-recovery"> + + <title>Recovery</title> + + <indexterm> + <primary>configuration</primary> + <secondary>of recovery</secondary> + <tertiary>general settings</tertiary> + </indexterm> + + <para> + This section describes the settings that apply to recovery in general, + affecting crash recovery, streaming replication and archive-based + replication. + </para> + + + <variablelist> + <varlistentry id="guc-recovery-prefetch" xreflabel="recovery_prefetch"> + <term><varname>recovery_prefetch</varname> (<type>boolean</type>) + <indexterm> + <primary><varname>recovery_prefetch</varname> configuration parameter</primary> + </indexterm> + </term> + <listitem> + <para> + Whether to try to prefetch blocks that are referenced in the WAL that + are not yet in the buffer pool, during recovery. Prefetching blocks + that will soon be needed can reduce I/O wait times in some workloads. + See also the <xref linkend="guc-wal-decode-buffer-size"/> and + <xref linkend="guc-maintenance-io-concurrency"/> settings, which limit + prefetching activity. + This setting is disabled by default. + </para> + <para> + This feature currently depends on an effective + <function>posix_fadvise</function> function, which some + operating systems lack. + </para> + </listitem> + </varlistentry> + + <varlistentry id="guc-wal-decode-buffer-size" xreflabel="wal_decode_buffer_size"> + <term><varname>wal_decode_buffer_size</varname> (<type>integer</type>) + <indexterm> + <primary><varname>wal_decode_buffer_size</varname> configuration parameter</primary> + </indexterm> + </term> + <listitem> + <para> + A limit on how far ahead the server can look in the WAL, to find + blocks to prefetch. If this value is specified without units, it is + taken as bytes. + The default is 512kB. + </para> + </listitem> + </varlistentry> + + </variablelist> + </sect2> + <sect2 id="runtime-config-wal-archive-recovery"> <title>Archive Recovery</title> diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 9fb62fec8e..2e3b73f49e 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -328,6 +328,13 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser </entry> </row> + <row> + <entry><structname>pg_stat_prefetch_recovery</structname><indexterm><primary>pg_stat_prefetch_recovery</primary></indexterm></entry> + <entry>Only one row, showing statistics about blocks prefetched during recovery. + See <xref linkend="pg-stat-prefetch-recovery-view"/> for details. + </entry> + </row> + <row> <entry><structname>pg_stat_subscription</structname><indexterm><primary>pg_stat_subscription</primary></indexterm></entry> <entry>At least one row per subscription, showing information about @@ -2958,6 +2965,69 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i copy of the subscribed tables. </para> + <table id="pg-stat-prefetch-recovery-view" xreflabel="pg_stat_prefetch_recovery"> + <title><structname>pg_stat_prefetch_recovery</structname> View</title> + <tgroup cols="3"> + <thead> + <row> + <entry>Column</entry> + <entry>Type</entry> + <entry>Description</entry> + </row> + </thead> + + <tbody> + <row> + <entry><structfield>prefetch</structfield></entry> + <entry><type>bigint</type></entry> + <entry>Number of blocks prefetched because they were not in the buffer pool</entry> + </row> + <row> + <entry><structfield>hit</structfield></entry> + <entry><type>bigint</type></entry> + <entry>Number of blocks not prefetched because they were already in the buffer pool</entry> + </row> + <row> + <entry><structfield>skip_init</structfield></entry> + <entry><type>bigint</type></entry> + <entry>Number of blocks not prefetched because they would be zero-initialized</entry> + </row> + <row> + <entry><structfield>skip_new</structfield></entry> + <entry><type>bigint</type></entry> + <entry>Number of blocks not prefetched because they didn't exist yet</entry> + </row> + <row> + <entry><structfield>skip_fpw</structfield></entry> + <entry><type>bigint</type></entry> + <entry>Number of blocks not prefetched because a full page image was included in the WAL</entry> + </row> + <row> + <entry><structfield>wal_distance</structfield></entry> + <entry><type>integer</type></entry> + <entry>How many bytes ahead the prefetcher is looking</entry> + </row> + <row> + <entry><structfield>block_distance</structfield></entry> + <entry><type>integer</type></entry> + <entry>How many blocks ahead the prefetcher is looking</entry> + </row> + <row> + <entry><structfield>io_depth</structfield></entry> + <entry><type>integer</type></entry> + <entry>How many prefetches have been initiated but are not yet known to have completed</entry> + </row> + </tbody> + </tgroup> + </table> + + <para> + The <structname>pg_stat_prefetch_recovery</structname> view will contain only + one row. It is filled with nulls if recovery is not running or WAL + prefetching is not enabled. See <xref linkend="guc-recovery-prefetch"/> + for more information. + </para> + <table id="pg-stat-subscription" xreflabel="pg_stat_subscription"> <title><structname>pg_stat_subscription</structname> View</title> <tgroup cols="1"> @@ -5177,8 +5247,11 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i all the counters shown in the <structname>pg_stat_bgwriter</structname> view, <literal>archiver</literal> to reset all the counters shown in - the <structname>pg_stat_archiver</structname> view or <literal>wal</literal> - to reset all the counters shown in the <structname>pg_stat_wal</structname> view. + the <structname>pg_stat_archiver</structname> view, + <literal>wal</literal> to reset all the counters shown in the + <structname>pg_stat_wal</structname> view or + <literal>prefetch_recovery</literal> to reset all the counters shown + in the <structname>pg_stat_prefetch_recovery</structname> view. </para> <para> This function is restricted to superusers by default, but other users diff --git a/doc/src/sgml/wal.sgml b/doc/src/sgml/wal.sgml index 2bb27a8468..8566f297d3 100644 --- a/doc/src/sgml/wal.sgml +++ b/doc/src/sgml/wal.sgml @@ -803,6 +803,18 @@ counted as <literal>wal_write</literal> and <literal>wal_sync</literal> in <structname>pg_stat_wal</structname>, respectively. </para> + + <para> + The <xref linkend="guc-recovery-prefetch"/> parameter can + be used to improve I/O performance during recovery by instructing + <productname>PostgreSQL</productname> to initiate reads + of disk blocks that will soon be needed but are not currently in + <productname>PostgreSQL</productname>'s buffer pool. + The <xref linkend="guc-maintenance-io-concurrency"/> and + <xref linkend="guc-wal-decode-buffer-size"/> settings limit prefetching + concurrency and distance, respectively. + By default, prefetching in recovery is disabled. + </para> </sect1> <sect1 id="wal-internals"> diff --git a/src/backend/access/transam/Makefile b/src/backend/access/transam/Makefile index 79314c69ab..8c17c88dfc 100644 --- a/src/backend/access/transam/Makefile +++ b/src/backend/access/transam/Makefile @@ -31,6 +31,7 @@ OBJS = \ xlogarchive.o \ xlogfuncs.o \ xloginsert.o \ + xlogprefetcher.o \ xlogreader.o \ xlogrecovery.o \ xlogutils.o diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 2b4e591736..23ecf0a237 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -59,6 +59,7 @@ #include "access/xlog_internal.h" #include "access/xlogarchive.h" #include "access/xloginsert.h" +#include "access/xlogprefetcher.h" #include "access/xlogreader.h" #include "access/xlogrecovery.h" #include "access/xlogutils.h" @@ -132,6 +133,7 @@ int CommitDelay = 0; /* precommit delay in microseconds */ int CommitSiblings = 5; /* # concurrent xacts needed to sleep */ int wal_retrieve_retry_interval = 5000; int max_slot_wal_keep_size_mb = -1; +int wal_decode_buffer_size = 512 * 1024; bool track_wal_io_timing = false; #ifdef WAL_DEBUG diff --git a/src/backend/access/transam/xlogprefetcher.c b/src/backend/access/transam/xlogprefetcher.c new file mode 100644 index 0000000000..ec24cbf386 --- /dev/null +++ b/src/backend/access/transam/xlogprefetcher.c @@ -0,0 +1,962 @@ +/*------------------------------------------------------------------------- + * + * xlogprefetcher.c + * Prefetching support for recovery. + * + * Portions Copyright (c) 2021, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * + * IDENTIFICATION + * src/backend/access/transam/xlogprefetcher.c + * + * This module provides a drop-in replacement for an XLogReader that tries to + * minimize I/O stalls by looking up future blocks in the buffer cache, and + * initiating I/Os that might complete before the caller eventually needs the + * data. XLogRecBufferForRedo() cooperates uses information stored in the + * decoded record to find buffers efficiently. + * + *------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include "access/xlog.h" +#include "access/xlogprefetcher.h" +#include "access/xlogreader.h" +#include "access/xlogutils.h" +#include "catalog/pg_class.h" +#include "catalog/pg_control.h" +#include "catalog/storage_xlog.h" +#include "commands/dbcommands_xlog.h" +#include "utils/fmgrprotos.h" +#include "utils/timestamp.h" +#include "funcapi.h" +#include "pgstat.h" +#include "miscadmin.h" +#include "port/atomics.h" +#include "storage/bufmgr.h" +#include "storage/shmem.h" +#include "storage/smgr.h" +#include "utils/guc.h" +#include "utils/hsearch.h" + +/* Every time we process this much WAL, we update dynamic values in shm. */ +#define XLOGPREFETCHER_STATS_SHM_DISTANCE BLCKSZ + +/* GUCs */ +bool recovery_prefetch = false; + +static int XLogPrefetchReconfigureCount = 0; + +/* + * Enum used to report whether an IO should be started. + */ +typedef enum +{ + LRQ_NEXT_NO_IO, + LRQ_NEXT_IO, + LRQ_NEXT_AGAIN +} LsnReadQueueNextStatus; + +/* + * Type of callback that can decide which block to prefetch next. For now + * there is only one. + */ +typedef LsnReadQueueNextStatus (*LsnReadQueueNextFun) (uintptr_t lrq_private, + XLogRecPtr *lsn); + +/* + * A simple circular queue of LSNs, using to control the number of + * (potentially) inflight IOs. This stands in for a later more general IO + * control mechanism, which is why it has the apparently unnecessary + * indirection through a function pointer. + */ +typedef struct LsnReadQueue +{ + LsnReadQueueNextFun next; + uintptr_t lrq_private; + uint32 max_inflight; + uint32 inflight; + uint32 completed; + uint32 head; + uint32 tail; + uint32 size; + struct + { + bool io; + XLogRecPtr lsn; + } queue[FLEXIBLE_ARRAY_MEMBER]; +} LsnReadQueue; + +/* + * A prefetcher. This is a mechanism that wraps an XLogReader, prefetching + * blocks that will be soon be referenced, to try to avoid IO stalls. + */ +struct XLogPrefetcher +{ + /* WAL reader and current reading state. */ + XLogReaderState *reader; + DecodedXLogRecord *record; + int next_block_id; + + /* When to publish stats. */ + XLogRecPtr next_stats_shm_lsn; + + /* Book-keeping required to avoid accessing non-existing blocks. */ + HTAB *filter_table; + dlist_head filter_queue; + + /* Book-keeping for readahead barriers. */ + XLogRecPtr no_readahead_until; + + /* IO depth manager. */ + LsnReadQueue *streaming_read; + + XLogRecPtr begin_ptr; + + int reconfigure_count; +}; + +/* + * A temporary filter used to track block ranges that haven't been created + * yet, whole relations that haven't been created yet, and whole relations + * that (we assume) have already been dropped, or will be created by bulk WAL + * operators. + */ +typedef struct XLogPrefetcherFilter +{ + RelFileNode rnode; + XLogRecPtr filter_until_replayed; + BlockNumber filter_from_block; + dlist_node link; +} XLogPrefetcherFilter; + +/* + * Counters exposed in shared memory for pg_stat_prefetch_recovery. + */ +typedef struct XLogPrefetchStats +{ + pg_atomic_uint64 reset_time; /* Time of last reset. */ + pg_atomic_uint64 prefetch; /* Prefetches initiated. */ + pg_atomic_uint64 hit; /* Blocks already in cache. */ + pg_atomic_uint64 skip_init; /* Zero-inited blocks skipped. */ + pg_atomic_uint64 skip_new; /* New/missing blocks filtered. */ + pg_atomic_uint64 skip_fpw; /* FPWs skipped. */ + + /* Reset counters */ + pg_atomic_uint32 reset_request; + uint32 reset_handled; + + /* Dynamic values */ + int wal_distance; /* Number of WAL bytes ahead. */ + int block_distance; /* Number of block references ahead. */ + int io_depth; /* Number of I/Os in progress. */ +} XLogPrefetchStats; + +static inline void XLogPrefetcherAddFilter(XLogPrefetcher *prefetcher, + RelFileNode rnode, + BlockNumber blockno, + XLogRecPtr lsn); +static inline bool XLogPrefetcherIsFiltered(XLogPrefetcher *prefetcher, + RelFileNode rnode, + BlockNumber blockno); +static inline void XLogPrefetcherCompleteFilters(XLogPrefetcher *prefetcher, + XLogRecPtr replaying_lsn); +static LsnReadQueueNextStatus XLogPrefetcherNextBlock(uintptr_t pgsr_private, + XLogRecPtr *lsn); + +static XLogPrefetchStats *SharedStats; + +static inline LsnReadQueue * +lrq_alloc(uint32 max_distance, + uint32 max_inflight, + uintptr_t lrq_private, + LsnReadQueueNextFun next) +{ + LsnReadQueue *lrq; + uint32 size; + + Assert(max_distance >= max_inflight); + + size = max_distance + 1; /* full ring buffer has a gap */ + lrq = palloc(offsetof(LsnReadQueue, queue) + sizeof(lrq->queue[0]) * size); + lrq->lrq_private = lrq_private; + lrq->max_inflight = max_inflight; + lrq->size = size; + lrq->next = next; + lrq->head = 0; + lrq->tail = 0; + lrq->inflight = 0; + lrq->completed = 0; + + return lrq; +} + +static inline void +lrq_free(LsnReadQueue *lrq) +{ + pfree(lrq); +} + +static inline uint32 +lrq_inflight(LsnReadQueue *lrq) +{ + return lrq->inflight; +} + +static inline uint32 +lrq_completed(LsnReadQueue *lrq) +{ + return lrq->completed; +} + +static inline void +lrq_prefetch(LsnReadQueue *lrq) +{ + /* Try to start as many IOs as we can within our limits. */ + while (lrq->inflight < lrq->max_inflight && + lrq->inflight + lrq->completed < lrq->size - 1) + { + Assert(((lrq->head + 1) % lrq->size) != lrq->tail); + switch (lrq->next(lrq->lrq_private, &lrq->queue[lrq->head].lsn)) + { + case LRQ_NEXT_AGAIN: + return; + case LRQ_NEXT_IO: + lrq->queue[lrq->head].io = true; + lrq->inflight++; + break; + case LRQ_NEXT_NO_IO: + lrq->queue[lrq->head].io = false; + lrq->completed++; + break; + } + lrq->head++; + if (lrq->head == lrq->size) + lrq->head = 0; + } +} + +static inline void +lrq_complete_lsn(LsnReadQueue *lrq, XLogRecPtr lsn) +{ + /* + * We know that LSNs before 'lsn' have been replayed, so we can now assume + * that any IOs that were started before then have finished. + */ + while (lrq->tail != lrq->head && + lrq->queue[lrq->tail].lsn < lsn) + { + if (lrq->queue[lrq->tail].io) + lrq->inflight--; + else + lrq->completed--; + lrq->tail++; + if (lrq->tail == lrq->size) + lrq->tail = 0; + } + if (recovery_prefetch) + lrq_prefetch(lrq); +} + +size_t +XLogPrefetchShmemSize(void) +{ + return sizeof(XLogPrefetchStats); +} + +static void +XLogPrefetchResetStats(void) +{ + pg_atomic_write_u64(&SharedStats->reset_time, GetCurrentTimestamp()); + pg_atomic_write_u64(&SharedStats->prefetch, 0); + pg_atomic_write_u64(&SharedStats->hit, 0); + pg_atomic_write_u64(&SharedStats->skip_init, 0); + pg_atomic_write_u64(&SharedStats->skip_new, 0); + pg_atomic_write_u64(&SharedStats->skip_fpw, 0); +} + +void +XLogPrefetchShmemInit(void) +{ + bool found; + + SharedStats = (XLogPrefetchStats *) + ShmemInitStruct("XLogPrefetchStats", + sizeof(XLogPrefetchStats), + &found); + + if (!found) + { + pg_atomic_init_u32(&SharedStats->reset_request, 0); + SharedStats->reset_handled = 0; + + pg_atomic_init_u64(&SharedStats->reset_time, GetCurrentTimestamp()); + pg_atomic_init_u64(&SharedStats->prefetch, 0); + pg_atomic_init_u64(&SharedStats->hit, 0); + pg_atomic_init_u64(&SharedStats->skip_init, 0); + pg_atomic_init_u64(&SharedStats->skip_new, 0); + pg_atomic_init_u64(&SharedStats->skip_fpw, 0); + } +} + +/* + * Called when any GUC is changed that affects prefetching. + */ +void +XLogPrefetchReconfigure(void) +{ + XLogPrefetchReconfigureCount++; +} + +/* + * Called by any backend to request that the stats be reset. + */ +void +XLogPrefetchRequestResetStats(void) +{ + pg_atomic_fetch_add_u32(&SharedStats->reset_request, 1); +} + +/* + * Increment a counter in shared memory. This is equivalent to *counter++ on a + * plain uint64 without any memory barrier or locking, except on platforms + * where readers can't read uint64 without possibly observing a torn value. + */ +static inline void +XLogPrefetchIncrement(pg_atomic_uint64 *counter) +{ + Assert(AmStartupProcess() || !IsUnderPostmaster); + pg_atomic_write_u64(counter, pg_atomic_read_u64(counter) + 1); +} + +/* + * Create a prefetcher that is ready to begin prefetching blocks referenced by + * WAL records. + */ +XLogPrefetcher * +XLogPrefetcherAllocate(XLogReaderState *reader) +{ + XLogPrefetcher *prefetcher; + static HASHCTL hash_table_ctl = { + .keysize = sizeof(RelFileNode), + .entrysize = sizeof(XLogPrefetcherFilter) + }; + + prefetcher = palloc0(sizeof(XLogPrefetcher)); + + prefetcher->reader = reader; + prefetcher->filter_table = hash_create("XLogPrefetcherFilterTable", 1024, + &hash_table_ctl, + HASH_ELEM | HASH_BLOBS); + dlist_init(&prefetcher->filter_queue); + + SharedStats->wal_distance = 0; + SharedStats->block_distance = 0; + SharedStats->io_depth = 0; + + /* First usage will cause streaming_read to be allocated. */ + prefetcher->reconfigure_count = XLogPrefetchReconfigureCount - 1; + + return prefetcher; +} + +/* + * Destroy a prefetcher and release all resources. + */ +void +XLogPrefetcherFree(XLogPrefetcher *prefetcher) +{ + lrq_free(prefetcher->streaming_read); + hash_destroy(prefetcher->filter_table); + pfree(prefetcher); +} + +/* + * Provide access to the reader. + */ +XLogReaderState * +XLogPrefetcherReader(XLogPrefetcher *prefetcher) +{ + return prefetcher->reader; +} + +static void +XLogPrefetcherComputeStats(XLogPrefetcher *prefetcher, XLogRecPtr lsn) +{ + uint32 io_depth; + uint32 completed; + uint32 reset_request; + int64 wal_distance; + + + /* How far ahead of replay are we now? */ + if (prefetcher->record) + wal_distance = prefetcher->record->lsn - prefetcher->reader->record->lsn; + else + wal_distance = 0; + + /* How many IOs are currently in flight and completed? */ + io_depth = lrq_inflight(prefetcher->streaming_read); + completed = lrq_completed(prefetcher->streaming_read); + + /* Update the instantaneous stats visible in pg_stat_prefetch_recovery. */ + SharedStats->io_depth = io_depth; + SharedStats->block_distance = io_depth + completed; + SharedStats->wal_distance = wal_distance; + + /* + * Have we been asked to reset our stats counters? This is checked with + * an unsynchronized memory read, but we'll see it eventually and we'll be + * accessing that cache line anyway. + */ + reset_request = pg_atomic_read_u32(&SharedStats->reset_request); + if (reset_request != SharedStats->reset_handled) + { + XLogPrefetchResetStats(); + SharedStats->reset_handled = reset_request; + } + + prefetcher->next_stats_shm_lsn = lsn + XLOGPREFETCHER_STATS_SHM_DISTANCE; +} + +/* + * A callback that reads ahead in the WAL and tries to initiate one IO. + */ +static LsnReadQueueNextStatus +XLogPrefetcherNextBlock(uintptr_t pgsr_private, XLogRecPtr *lsn) +{ + XLogPrefetcher *prefetcher = (XLogPrefetcher *) pgsr_private; + XLogReaderState *reader = prefetcher->reader; + XLogRecPtr replaying_lsn = reader->ReadRecPtr; + + /* + * We keep track of the record and block we're up to between calls with + * prefetcher->record and prefetcher->next_block_id. + */ + for (;;) + { + DecodedXLogRecord *record; + + /* Try to read a new future record, if we don't already have one. */ + if (prefetcher->record == NULL) + { + bool nonblocking; + + /* + * If there are already records or an error queued up that could + * be replayed, we don't want to block here. Otherwise, it's OK + * to block waiting for more data: presumably the caller has + * nothing else to do. + */ + nonblocking = XLogReaderHasQueuedRecordOrError(reader); + + /* Certain records act as barriers for all readahead. */ + if (nonblocking && replaying_lsn < prefetcher->no_readahead_until) + return LRQ_NEXT_AGAIN; + + record = XLogReadAhead(prefetcher->reader, nonblocking); + if (record == NULL) + { + /* + * We can't read any more, due to an error or lack of data in + * nonblocking mode. + */ + return LRQ_NEXT_AGAIN; + } + + /* + * If prefetching is disabled, we don't need to analyze the record + * or issue any prefetches. We just need to cause one record to + * be decoded. + */ + if (!recovery_prefetch) + { + *lsn = InvalidXLogRecPtr; + return LRQ_NEXT_NO_IO; + } + + /* We have a new record to process. */ + prefetcher->record = record; + prefetcher->next_block_id = 0; + } + else + { + /* Continue to process from last call, or last loop. */ + record = prefetcher->record; + } + + /* + * Check for operations that require us to filter out block ranges, or + * stop readahead completely. + * + * XXX Perhaps this information could be derived automatically if we + * had some standardized header flags and fields for these fields, + * instead of special logic. + * + * XXX Are there other operations that need this treatment? + */ + if (replaying_lsn < record->lsn) + { + uint8 rmid = record->header.xl_rmid; + uint8 record_type = record->header.xl_info & ~XLR_INFO_MASK; + + if (rmid == RM_XLOG_ID) + { + if (record_type == XLOG_CHECKPOINT_SHUTDOWN || + record_type == XLOG_END_OF_RECOVERY) + { + /* + * These records might change the TLI. Avoid potential + * bugs if we were to allow "read TLI" and "replay TLI" to + * differ without more analysis. + */ + prefetcher->no_readahead_until = record->lsn; + } + } + else if (rmid == RM_DBASE_ID) + { + if (record_type == XLOG_DBASE_CREATE) + { + xl_dbase_create_rec *xlrec = (xl_dbase_create_rec *) + record->main_data; + RelFileNode rnode = {InvalidOid, xlrec->db_id, InvalidOid}; + + /* + * Don't try to prefetch anything in this database until + * it has been created, or we might confuse blocks on OID + * wraparound. (We could use XLOG_DBASE_DROP instead, but + * there shouldn't be any reference to blocks in a + * database between DROP and CREATE for the same OID, and + * doing it on CREATE avoids the more expensive + * ENOENT-handling if we didn't treat CREATE as a + * barrier). + */ + XLogPrefetcherAddFilter(prefetcher, rnode, 0, record->lsn); + } + } + else if (rmid == RM_SMGR_ID) + { + if (record_type == XLOG_SMGR_CREATE) + { + xl_smgr_create *xlrec = (xl_smgr_create *) + record->main_data; + + /* + * Don't prefetch anything for this whole relation until + * it has been created, or we might confuse blocks on OID + * wraparound. + */ + XLogPrefetcherAddFilter(prefetcher, xlrec->rnode, 0, + record->lsn); + } + else if (record_type == XLOG_SMGR_TRUNCATE) + { + xl_smgr_truncate *xlrec = (xl_smgr_truncate *) + record->main_data; + + /* + * Don't prefetch anything in the truncated range until + * the truncation has been performed. + */ + XLogPrefetcherAddFilter(prefetcher, xlrec->rnode, + xlrec->blkno, + record->lsn); + } + } + } + + /* Scan the block references, starting where we left off last time. */ + while (prefetcher->next_block_id <= record->max_block_id) + { + int block_id = prefetcher->next_block_id++; + DecodedBkpBlock *block = &record->blocks[block_id]; + SMgrRelation reln; + PrefetchBufferResult result; + + if (!block->in_use) + continue; + + Assert(!BufferIsValid(block->prefetch_buffer));; + + /* + * Record the LSN of this record. When it's replayed, + * LsnReadQueue will consider any IOs submitted for earlier LSNs + * to be finished. + */ + *lsn = record->lsn; + + /* We don't try to prefetch anything but the main fork for now. */ + if (block->forknum != MAIN_FORKNUM) + { + return LRQ_NEXT_NO_IO; + } + + /* + * If there is a full page image attached, we won't be reading the + * page, so don't both trying to prefetch. + */ + if (block->has_image) + { + XLogPrefetchIncrement(&SharedStats->skip_fpw); + return LRQ_NEXT_NO_IO; + } + + /* There is no point in reading a page that will be zeroed. */ + if (block->flags & BKPBLOCK_WILL_INIT) + { + XLogPrefetchIncrement(&SharedStats->skip_init); + return LRQ_NEXT_NO_IO; + } + + /* Should we skip prefetching this block due to a filter? */ + if (XLogPrefetcherIsFiltered(prefetcher, block->rnode, block->blkno)) + { + XLogPrefetchIncrement(&SharedStats->skip_new); + return LRQ_NEXT_NO_IO; + } + + /* + * We could try to have a fast path for repeated references to the + * same relation (with some scheme to handle invalidations + * safely), but for now we'll call smgropen() every time. + */ + reln = smgropen(block->rnode, InvalidBackendId); + + /* + * If the block is past the end of the relation, filter out + * further accesses until this record is replayed. + */ + if (block->blkno >= smgrnblocks(reln, block->forknum)) + { + XLogPrefetcherAddFilter(prefetcher, block->rnode, block->blkno, + record->lsn); + XLogPrefetchIncrement(&SharedStats->skip_new); + return LRQ_NEXT_NO_IO; + } + + /* Try to initiate prefetching. */ + result = PrefetchSharedBuffer(reln, block->forknum, block->blkno); + if (BufferIsValid(result.recent_buffer)) + { + /* Cache hit, nothing to do. */ + XLogPrefetchIncrement(&SharedStats->hit); + block->prefetch_buffer = result.recent_buffer; + return LRQ_NEXT_NO_IO; + } + else if (result.initiated_io) + { + /* Cache miss, I/O (presumably) started. */ + XLogPrefetchIncrement(&SharedStats->prefetch); + block->prefetch_buffer = InvalidBuffer; + return LRQ_NEXT_IO; + } + else + { + /* + * Neither cached nor initiated. The underlying segment file + * doesn't exist. (ENOENT) + * + * It might be missing becaused it was unlinked, we crashed, + * and now we're replaying WAL. Recovery will correct this + * problem or complain if something is wrong. + */ + XLogPrefetcherAddFilter(prefetcher, block->rnode, 0, + record->lsn); + XLogPrefetchIncrement(&SharedStats->skip_new); + return LRQ_NEXT_NO_IO; + } + } + + /* + * Several callsites need to be able to read exactly one record + * without any internal readahead. Examples: xlog.c reading + * checkpoint records with emode set to PANIC, which might otherwise + * cause XLogPageRead() to panic on some future page, and xlog.c + * determining where to start writing WAL next, which depends on the + * contents of the reader's internal buffer after reading one record. + * Therefore, don't even think about prefetching until the first + * record after XLogPrefetcherBeginRead() has been consumed. + */ + if (prefetcher->reader->decode_queue_tail && + prefetcher->reader->decode_queue_tail->lsn == prefetcher->begin_ptr) + return LRQ_NEXT_AGAIN; + + /* Advance to the next record. */ + prefetcher->record = NULL; + } + pg_unreachable(); +} + +/* + * Expose statistics about recovery prefetching. + */ +Datum +pg_stat_get_prefetch_recovery(PG_FUNCTION_ARGS) +{ +#define PG_STAT_GET_PREFETCH_RECOVERY_COLS 10 + ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; + TupleDesc tupdesc; + Tuplestorestate *tupstore; + MemoryContext per_query_ctx; + MemoryContext oldcontext; + Datum values[PG_STAT_GET_PREFETCH_RECOVERY_COLS]; + bool nulls[PG_STAT_GET_PREFETCH_RECOVERY_COLS]; + + if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("set-valued function called in context that cannot accept a set"))); + if (!(rsinfo->allowedModes & SFRM_Materialize)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("materialize mod required, but it is not allowed in this context"))); + + if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) + elog(ERROR, "return type must be a row type"); + + per_query_ctx = rsinfo->econtext->ecxt_per_query_memory; + oldcontext = MemoryContextSwitchTo(per_query_ctx); + + tupstore = tuplestore_begin_heap(true, false, work_mem); + rsinfo->returnMode = SFRM_Materialize; + rsinfo->setResult = tupstore; + rsinfo->setDesc = tupdesc; + + MemoryContextSwitchTo(oldcontext); + + if (pg_atomic_read_u32(&SharedStats->reset_request) != SharedStats->reset_handled) + { + /* There's an unhandled reset request, so just show NULLs */ + for (int i = 0; i < PG_STAT_GET_PREFETCH_RECOVERY_COLS; ++i) + nulls[i] = true; + } + else + { + for (int i = 0; i < PG_STAT_GET_PREFETCH_RECOVERY_COLS; ++i) + nulls[i] = false; + } + + values[0] = TimestampTzGetDatum(pg_atomic_read_u64(&SharedStats->reset_time)); + values[1] = Int64GetDatum(pg_atomic_read_u64(&SharedStats->prefetch)); + values[2] = Int64GetDatum(pg_atomic_read_u64(&SharedStats->hit)); + values[3] = Int64GetDatum(pg_atomic_read_u64(&SharedStats->skip_init)); + values[4] = Int64GetDatum(pg_atomic_read_u64(&SharedStats->skip_new)); + values[5] = Int64GetDatum(pg_atomic_read_u64(&SharedStats->skip_fpw)); + values[6] = Int32GetDatum(SharedStats->wal_distance); + values[7] = Int32GetDatum(SharedStats->block_distance); + values[8] = Int32GetDatum(SharedStats->io_depth); + tuplestore_putvalues(tupstore, tupdesc, values, nulls); + tuplestore_donestoring(tupstore); + + return (Datum) 0; +} + +/* + * Don't prefetch any blocks >= 'blockno' from a given 'rnode', until 'lsn' + * has been replayed. + */ +static inline void +XLogPrefetcherAddFilter(XLogPrefetcher *prefetcher, RelFileNode rnode, + BlockNumber blockno, XLogRecPtr lsn) +{ + XLogPrefetcherFilter *filter; + bool found; + + filter = hash_search(prefetcher->filter_table, &rnode, HASH_ENTER, &found); + if (!found) + { + /* + * Don't allow any prefetching of this block or higher until replayed. + */ + filter->filter_until_replayed = lsn; + filter->filter_from_block = blockno; + dlist_push_head(&prefetcher->filter_queue, &filter->link); + } + else + { + /* + * We were already filtering this rnode. Extend the filter's lifetime + * to cover this WAL record, but leave the lower of the block numbers + * there because we don't want to have to track individual blocks. + */ + filter->filter_until_replayed = lsn; + dlist_delete(&filter->link); + dlist_push_head(&prefetcher->filter_queue, &filter->link); + filter->filter_from_block = Min(filter->filter_from_block, blockno); + } +} + +/* + * Have we replayed any records that caused us to begin filtering a block + * range? That means that relations should have been created, extended or + * dropped as required, so we can stop filtering out accesses to a given + * relfilenode. + */ +static inline void +XLogPrefetcherCompleteFilters(XLogPrefetcher *prefetcher, XLogRecPtr replaying_lsn) +{ + while (unlikely(!dlist_is_empty(&prefetcher->filter_queue))) + { + XLogPrefetcherFilter *filter = dlist_tail_element(XLogPrefetcherFilter, + link, + &prefetcher->filter_queue); + + if (filter->filter_until_replayed >= replaying_lsn) + break; + + dlist_delete(&filter->link); + hash_search(prefetcher->filter_table, filter, HASH_REMOVE, NULL); + } +} + +/* + * Check if a given block should be skipped due to a filter. + */ +static inline bool +XLogPrefetcherIsFiltered(XLogPrefetcher *prefetcher, RelFileNode rnode, + BlockNumber blockno) +{ + /* + * Test for empty queue first, because we expect it to be empty most of + * the time and we can avoid the hash table lookup in that case. + */ + if (unlikely(!dlist_is_empty(&prefetcher->filter_queue))) + { + XLogPrefetcherFilter *filter; + + /* See if the block range is filtered. */ + filter = hash_search(prefetcher->filter_table, &rnode, HASH_FIND, NULL); + if (filter && filter->filter_from_block <= blockno) + return true; + + /* See if the whole database is filtered. */ + rnode.relNode = InvalidOid; + filter = hash_search(prefetcher->filter_table, &rnode, HASH_FIND, NULL); + if (filter && filter->filter_from_block <= blockno) + return true; + } + + return false; +} + +/* + * A wrapper for XLogBeginRead() that also resets the prefetcher. + */ +void +XLogPrefetcherBeginRead(XLogPrefetcher *prefetcher, XLogRecPtr recPtr) +{ + /* This will forget about any in-flight IO. */ + prefetcher->reconfigure_count--; + + /* Book-keeping to avoid readahead on first read. */ + prefetcher->begin_ptr = recPtr; + + prefetcher->no_readahead_until = 0; + + /* This will forget about any queued up records in the decoder. */ + XLogBeginRead(prefetcher->reader, recPtr); +} + +/* + * A wrapper for XLogReadRecord() that provides the same interface, but also + * tries to initiate I/O for blocks referenced in future WAL records. + */ +XLogRecord * +XLogPrefetcherReadRecord(XLogPrefetcher *prefetcher, char **errmsg) +{ + DecodedXLogRecord *record; + + /* + * See if it's time to reset the prefetching machinery, because a relevant + * GUC was changed. + */ + if (unlikely(XLogPrefetchReconfigureCount != prefetcher->reconfigure_count)) + { + if (prefetcher->streaming_read) + lrq_free(prefetcher->streaming_read); + + /* + * Arbitrarily look up to 4 times further ahead than the number of IOs + * we're allowed to run concurrently. + */ + prefetcher->streaming_read = + lrq_alloc(recovery_prefetch ? maintenance_io_concurrency * 4 : 1, + recovery_prefetch ? maintenance_io_concurrency : 1, + (uintptr_t) prefetcher, + XLogPrefetcherNextBlock); + + prefetcher->reconfigure_count = XLogPrefetchReconfigureCount; + } + + /* + * Release last returned record, if there is one. We need to do this so + * that we can check for empty decode queue accurately. + */ + XLogReleasePreviousRecord(prefetcher->reader); + + /* If there's nothing queued yet, then start prefetching. */ + if (!XLogReaderHasQueuedRecordOrError(prefetcher->reader)) + lrq_prefetch(prefetcher->streaming_read); + + /* Read the next record. */ + record = XLogNextRecord(prefetcher->reader, errmsg); + if (!record) + return NULL; + + /* + * The record we just got is the "current" one, for the benefit of the + * XLogRecXXX() macros. + */ + Assert(record == prefetcher->reader->record); + + /* + * Can we drop any prefetch filters yet, given the record we're about to + * return? This assumes that any records with earlier LSNs have been + * replayed, so if we were waiting for a relation to be created or + * extended, it is now OK to access blocks in the covered range. + */ + XLogPrefetcherCompleteFilters(prefetcher, record->lsn); + + /* + * See if it's time to compute some statistics, because enough WAL has + * been processed. + */ + if (unlikely(record->lsn >= prefetcher->next_stats_shm_lsn)) + XLogPrefetcherComputeStats(prefetcher, record->lsn); + + /* + * The caller is about to replay this record, so we can now report that + * all IO initiated because of early WAL must be finished. This may + * trigger more readahead. + */ + lrq_complete_lsn(prefetcher->streaming_read, record->lsn); + + Assert(record == prefetcher->reader->record); + + return &record->header; +} + +bool +check_recovery_prefetch(bool *new_value, void **extra, GucSource source) +{ +#ifndef USE_PREFETCH + if (*new_value) + { + GUC_check_errdetail("recovery_prefetch must be set to off on platforms that lack posix_fadvise()."); + return false; + } +#endif + + return true; +} + +void +assign_recovery_prefetch(bool new_value, void *extra) +{ + /* Reconfigure prefetching, because a setting it depends on changed. */ + recovery_prefetch = new_value; + if (AmStartupProcess()) + XLogPrefetchReconfigure(); +} diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 1a8c651767..94681c3d80 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -1728,6 +1728,8 @@ DecodeXLogRecord(XLogReaderState *state, blk->has_image = ((fork_flags & BKPBLOCK_HAS_IMAGE) != 0); blk->has_data = ((fork_flags & BKPBLOCK_HAS_DATA) != 0); + blk->prefetch_buffer = InvalidBuffer; + COPY_HEADER_FIELD(&blk->data_len, sizeof(uint16)); /* cross-check that the HAS_DATA flag is set iff data_length > 0 */ if (blk->has_data && blk->data_len == 0) @@ -1934,6 +1936,15 @@ err: bool XLogRecGetBlockTag(XLogReaderState *record, uint8 block_id, RelFileNode *rnode, ForkNumber *forknum, BlockNumber *blknum) +{ + return XLogRecGetBlockInfo(record, block_id, rnode, forknum, blknum, NULL); +} + +bool +XLogRecGetBlockInfo(XLogReaderState *record, uint8 block_id, + RelFileNode *rnode, ForkNumber *forknum, + BlockNumber *blknum, + Buffer *prefetch_buffer) { DecodedBkpBlock *bkpb; @@ -1948,6 +1959,8 @@ XLogRecGetBlockTag(XLogReaderState *record, uint8 block_id, *forknum = bkpb->forknum; if (blknum) *blknum = bkpb->blkno; + if (prefetch_buffer) + *prefetch_buffer = bkpb->prefetch_buffer; return true; } diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index 9feea3e6ec..e5e7821c79 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -36,6 +36,7 @@ #include "access/xact.h" #include "access/xlog_internal.h" #include "access/xlogarchive.h" +#include "access/xlogprefetcher.h" #include "access/xlogreader.h" #include "access/xlogrecovery.h" #include "access/xlogutils.h" @@ -183,6 +184,9 @@ static bool doRequestWalReceiverReply; /* XLogReader object used to parse the WAL records */ static XLogReaderState *xlogreader = NULL; +/* XLogPrefetcher object used to consume WAL records with read-ahead */ +static XLogPrefetcher *xlogprefetcher = NULL; + /* Parameters passed down from ReadRecord to the XLogPageRead callback. */ typedef struct XLogPageReadPrivate { @@ -404,18 +408,21 @@ static void recoveryPausesHere(bool endOfRecovery); static bool recoveryApplyDelay(XLogReaderState *record); static void ConfirmRecoveryPaused(void); -static XLogRecord *ReadRecord(XLogReaderState *xlogreader, - int emode, bool fetching_ckpt, TimeLineID replayTLI); +static XLogRecord *ReadRecord(XLogPrefetcher *xlogprefetcher, + int emode, bool fetching_ckpt, + TimeLineID replayTLI); static int XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr, int reqLen, XLogRecPtr targetRecPtr, char *readBuf); -static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, - bool fetching_ckpt, - XLogRecPtr tliRecPtr, - TimeLineID replayTLI, - XLogRecPtr replayLSN); +static XLogPageReadResult WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, + bool randAccess, + bool fetching_ckpt, + XLogRecPtr tliRecPtr, + TimeLineID replayTLI, + XLogRecPtr replayLSN, + bool nonblocking); static int emode_for_corrupt_record(int emode, XLogRecPtr RecPtr); -static XLogRecord *ReadCheckpointRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, +static XLogRecord *ReadCheckpointRecord(XLogPrefetcher *xlogprefetcher, XLogRecPtr RecPtr, int whichChkpt, bool report, TimeLineID replayTLI); static bool rescanLatestTimeLine(TimeLineID replayTLI, XLogRecPtr replayLSN); static int XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli, @@ -561,6 +568,15 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr, errdetail("Failed while allocating a WAL reading processor."))); xlogreader->system_identifier = ControlFile->system_identifier; + /* + * Set the WAL decode buffer size. This limits how far ahead we can read + * in the WAL. + */ + XLogReaderSetDecodeBuffer(xlogreader, NULL, wal_decode_buffer_size); + + /* Create a WAL prefetcher. */ + xlogprefetcher = XLogPrefetcherAllocate(xlogreader); + /* * Allocate two page buffers dedicated to WAL consistency checks. We do * it this way, rather than just making static arrays, for two reasons: @@ -589,7 +605,8 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr, * When a backup_label file is present, we want to roll forward from * the checkpoint it identifies, rather than using pg_control. */ - record = ReadCheckpointRecord(xlogreader, CheckPointLoc, 0, true, CheckPointTLI); + record = ReadCheckpointRecord(xlogprefetcher, CheckPointLoc, 0, true, + CheckPointTLI); if (record != NULL) { memcpy(&checkPoint, XLogRecGetData(xlogreader), sizeof(CheckPoint)); @@ -607,8 +624,8 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr, */ if (checkPoint.redo < CheckPointLoc) { - XLogBeginRead(xlogreader, checkPoint.redo); - if (!ReadRecord(xlogreader, LOG, false, + XLogPrefetcherBeginRead(xlogprefetcher, checkPoint.redo); + if (!ReadRecord(xlogprefetcher, LOG, false, checkPoint.ThisTimeLineID)) ereport(FATAL, (errmsg("could not find redo location referenced by checkpoint record"), @@ -727,7 +744,7 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr, CheckPointTLI = ControlFile->checkPointCopy.ThisTimeLineID; RedoStartLSN = ControlFile->checkPointCopy.redo; RedoStartTLI = ControlFile->checkPointCopy.ThisTimeLineID; - record = ReadCheckpointRecord(xlogreader, CheckPointLoc, 1, true, + record = ReadCheckpointRecord(xlogprefetcher, CheckPointLoc, 1, true, CheckPointTLI); if (record != NULL) { @@ -1403,8 +1420,8 @@ FinishWalRecovery(void) lastRec = XLogRecoveryCtl->lastReplayedReadRecPtr; lastRecTLI = XLogRecoveryCtl->lastReplayedTLI; } - XLogBeginRead(xlogreader, lastRec); - (void) ReadRecord(xlogreader, PANIC, false, lastRecTLI); + XLogPrefetcherBeginRead(xlogprefetcher, lastRec); + (void) ReadRecord(xlogprefetcher, PANIC, false, lastRecTLI); endOfLog = xlogreader->EndRecPtr; /* @@ -1501,6 +1518,8 @@ ShutdownWalRecovery(void) } XLogReaderFree(xlogreader); + XLogPrefetcherFree(xlogprefetcher); + if (ArchiveRecoveryRequested) { /* @@ -1584,15 +1603,15 @@ PerformWalRecovery(void) { /* back up to find the record */ replayTLI = RedoStartTLI; - XLogBeginRead(xlogreader, RedoStartLSN); - record = ReadRecord(xlogreader, PANIC, false, replayTLI); + XLogPrefetcherBeginRead(xlogprefetcher, RedoStartLSN); + record = ReadRecord(xlogprefetcher, PANIC, false, replayTLI); } else { /* just have to read next record after CheckPoint */ Assert(xlogreader->ReadRecPtr == CheckPointLoc); replayTLI = CheckPointTLI; - record = ReadRecord(xlogreader, LOG, false, replayTLI); + record = ReadRecord(xlogprefetcher, LOG, false, replayTLI); } if (record != NULL) @@ -1706,7 +1725,7 @@ PerformWalRecovery(void) } /* Else, try to fetch the next WAL record */ - record = ReadRecord(xlogreader, LOG, false, replayTLI); + record = ReadRecord(xlogprefetcher, LOG, false, replayTLI); } while (record != NULL); /* @@ -1922,6 +1941,9 @@ ApplyWalRecord(XLogReaderState *xlogreader, XLogRecord *record, TimeLineID *repl */ if (AllowCascadeReplication()) WalSndWakeup(); + + /* Reset the prefetcher. */ + XLogPrefetchReconfigure(); } } @@ -2302,7 +2324,8 @@ verifyBackupPageConsistency(XLogReaderState *record) * temporary page. */ buf = XLogReadBufferExtended(rnode, forknum, blkno, - RBM_NORMAL_NO_LOG); + RBM_NORMAL_NO_LOG, + InvalidBuffer); if (!BufferIsValid(buf)) continue; @@ -2914,17 +2937,18 @@ ConfirmRecoveryPaused(void) * Attempt to read the next XLOG record. * * Before first call, the reader needs to be positioned to the first record - * by calling XLogBeginRead(). + * by calling XLogPrefetcherBeginRead(). * * If no valid record is available, returns NULL, or fails if emode is PANIC. * (emode must be either PANIC, LOG). In standby mode, retries until a valid * record is available. */ static XLogRecord * -ReadRecord(XLogReaderState *xlogreader, int emode, +ReadRecord(XLogPrefetcher *xlogprefetcher, int emode, bool fetching_ckpt, TimeLineID replayTLI) { XLogRecord *record; + XLogReaderState *xlogreader = XLogPrefetcherReader(xlogprefetcher); XLogPageReadPrivate *private = (XLogPageReadPrivate *) xlogreader->private_data; /* Pass through parameters to XLogPageRead */ @@ -2940,7 +2964,7 @@ ReadRecord(XLogReaderState *xlogreader, int emode, { char *errormsg; - record = XLogReadRecord(xlogreader, &errormsg); + record = XLogPrefetcherReadRecord(xlogprefetcher, &errormsg); if (record == NULL) { /* @@ -3073,6 +3097,9 @@ ReadRecord(XLogReaderState *xlogreader, int emode, * and call XLogPageRead() again with the same arguments. This lets * XLogPageRead() to try fetching the record from another source, or to * sleep and retry. + * + * While prefetching, xlogreader->nonblocking may be set. In that case, + * return XLREAD_WOULDBLOCK if we'd otherwise have to wait. */ static int XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr, int reqLen, @@ -3122,20 +3149,31 @@ retry: (readSource == XLOG_FROM_STREAM && flushedUpto < targetPagePtr + reqLen)) { - if (!WaitForWALToBecomeAvailable(targetPagePtr + reqLen, - private->randAccess, - private->fetching_ckpt, - targetRecPtr, - private->replayTLI, - xlogreader->EndRecPtr)) + if (readFile >= 0 && + xlogreader->nonblocking && + readSource == XLOG_FROM_STREAM && + flushedUpto < targetPagePtr + reqLen) + return XLREAD_WOULDBLOCK; + + switch (WaitForWALToBecomeAvailable(targetPagePtr + reqLen, + private->randAccess, + private->fetching_ckpt, + targetRecPtr, + private->replayTLI, + xlogreader->EndRecPtr, + xlogreader->nonblocking)) { - if (readFile >= 0) - close(readFile); - readFile = -1; - readLen = 0; - readSource = XLOG_FROM_ANY; - - return -1; + case XLREAD_WOULDBLOCK: + return XLREAD_WOULDBLOCK; + case XLREAD_FAIL: + if (readFile >= 0) + close(readFile); + readFile = -1; + readLen = 0; + readSource = XLOG_FROM_ANY; + return XLREAD_FAIL; + case XLREAD_SUCCESS: + break; } } @@ -3260,7 +3298,7 @@ next_record_is_invalid: if (StandbyMode) goto retry; else - return -1; + return XLREAD_FAIL; } /* @@ -3292,11 +3330,15 @@ next_record_is_invalid: * containing it (if not open already), and returns true. When end of standby * mode is triggered by the user, and there is no more WAL available, returns * false. + * + * If nonblocking is true, then give up immediately if we can't satisfy the + * request, returning XLREAD_WOULDBLOCK instead of waiting. */ -static bool +static XLogPageReadResult WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, bool fetching_ckpt, XLogRecPtr tliRecPtr, - TimeLineID replayTLI, XLogRecPtr replayLSN) + TimeLineID replayTLI, XLogRecPtr replayLSN, + bool nonblocking) { static TimestampTz last_fail_time = 0; TimestampTz now; @@ -3350,6 +3392,14 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, */ if (lastSourceFailed) { + /* + * Don't allow any retry loops to occur during nonblocking + * readahead. Let the caller process everything that has been + * decoded already first. + */ + if (nonblocking) + return XLREAD_WOULDBLOCK; + switch (currentSource) { case XLOG_FROM_ARCHIVE: @@ -3364,7 +3414,7 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, if (StandbyMode && CheckForStandbyTrigger()) { XLogShutdownWalRcv(); - return false; + return XLREAD_FAIL; } /* @@ -3372,7 +3422,7 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, * and pg_wal. */ if (!StandbyMode) - return false; + return XLREAD_FAIL; /* * Move to XLOG_FROM_STREAM state, and set to start a @@ -3516,7 +3566,7 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, currentSource == XLOG_FROM_ARCHIVE ? XLOG_FROM_ANY : currentSource); if (readFile >= 0) - return true; /* success! */ + return XLREAD_SUCCESS; /* success! */ /* * Nope, not found in archive or pg_wal. @@ -3671,11 +3721,15 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, /* just make sure source info is correct... */ readSource = XLOG_FROM_STREAM; XLogReceiptSource = XLOG_FROM_STREAM; - return true; + return XLREAD_SUCCESS; } break; } + /* In nonblocking mode, return rather than sleeping. */ + if (nonblocking) + return XLREAD_WOULDBLOCK; + /* * Data not here yet. Check for trigger, then wait for * walreceiver to wake us up when new WAL arrives. @@ -3683,13 +3737,13 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, if (CheckForStandbyTrigger()) { /* - * Note that we don't "return false" immediately here. - * After being triggered, we still want to replay all - * the WAL that was already streamed. It's in pg_wal - * now, so we just treat this as a failure, and the - * state machine will move on to replay the streamed - * WAL from pg_wal, and then recheck the trigger and - * exit replay. + * Note that we don't return XLREAD_FAIL immediately + * here. After being triggered, we still want to + * replay all the WAL that was already streamed. It's + * in pg_wal now, so we just treat this as a failure, + * and the state machine will move on to replay the + * streamed WAL from pg_wal, and then recheck the + * trigger and exit replay. */ lastSourceFailed = true; break; @@ -3740,7 +3794,7 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, HandleStartupProcInterrupts(); } - return false; /* not reached */ + return XLREAD_FAIL; /* not reached */ } @@ -3785,7 +3839,7 @@ emode_for_corrupt_record(int emode, XLogRecPtr RecPtr) * 1 for "primary", 0 for "other" (backup_label) */ static XLogRecord * -ReadCheckpointRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, +ReadCheckpointRecord(XLogPrefetcher *xlogprefetcher, XLogRecPtr RecPtr, int whichChkpt, bool report, TimeLineID replayTLI) { XLogRecord *record; @@ -3812,8 +3866,8 @@ ReadCheckpointRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, return NULL; } - XLogBeginRead(xlogreader, RecPtr); - record = ReadRecord(xlogreader, LOG, true, replayTLI); + XLogPrefetcherBeginRead(xlogprefetcher, RecPtr); + record = ReadRecord(xlogprefetcher, LOG, true, replayTLI); if (record == NULL) { diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c index 511f2f186f..ea22577b41 100644 --- a/src/backend/access/transam/xlogutils.c +++ b/src/backend/access/transam/xlogutils.c @@ -22,6 +22,7 @@ #include "access/timeline.h" #include "access/xlogrecovery.h" #include "access/xlog_internal.h" +#include "access/xlogprefetcher.h" #include "access/xlogutils.h" #include "miscadmin.h" #include "pgstat.h" @@ -355,11 +356,13 @@ XLogReadBufferForRedoExtended(XLogReaderState *record, RelFileNode rnode; ForkNumber forknum; BlockNumber blkno; + Buffer prefetch_buffer; Page page; bool zeromode; bool willinit; - if (!XLogRecGetBlockTag(record, block_id, &rnode, &forknum, &blkno)) + if (!XLogRecGetBlockInfo(record, block_id, &rnode, &forknum, &blkno, + &prefetch_buffer)) { /* Caller specified a bogus block_id */ elog(PANIC, "failed to locate backup block with ID %d", block_id); @@ -381,7 +384,8 @@ XLogReadBufferForRedoExtended(XLogReaderState *record, { Assert(XLogRecHasBlockImage(record, block_id)); *buf = XLogReadBufferExtended(rnode, forknum, blkno, - get_cleanup_lock ? RBM_ZERO_AND_CLEANUP_LOCK : RBM_ZERO_AND_LOCK); + get_cleanup_lock ? RBM_ZERO_AND_CLEANUP_LOCK : RBM_ZERO_AND_LOCK, + prefetch_buffer); page = BufferGetPage(*buf); if (!RestoreBlockImage(record, block_id, page)) elog(ERROR, "failed to restore block image"); @@ -410,7 +414,7 @@ XLogReadBufferForRedoExtended(XLogReaderState *record, } else { - *buf = XLogReadBufferExtended(rnode, forknum, blkno, mode); + *buf = XLogReadBufferExtended(rnode, forknum, blkno, mode, prefetch_buffer); if (BufferIsValid(*buf)) { if (mode != RBM_ZERO_AND_LOCK && mode != RBM_ZERO_AND_CLEANUP_LOCK) @@ -450,6 +454,10 @@ XLogReadBufferForRedoExtended(XLogReaderState *record, * exist, and we don't check for all-zeroes. Thus, no log entry is made * to imply that the page should be dropped or truncated later. * + * Optionally, recent_buffer can be used to provide a hint about the location + * of the page in the buffer pool; it does not have to be correct, but avoids + * a buffer mapping table probe if it is. + * * NB: A redo function should normally not call this directly. To get a page * to modify, use XLogReadBufferForRedoExtended instead. It is important that * all pages modified by a WAL record are registered in the WAL records, or @@ -457,7 +465,8 @@ XLogReadBufferForRedoExtended(XLogReaderState *record, */ Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum, - BlockNumber blkno, ReadBufferMode mode) + BlockNumber blkno, ReadBufferMode mode, + Buffer recent_buffer) { BlockNumber lastblock; Buffer buffer; @@ -465,6 +474,15 @@ XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum, Assert(blkno != P_NEW); + /* Do we have a clue where the buffer might be already? */ + if (BufferIsValid(recent_buffer) && + mode == RBM_NORMAL && + ReadRecentBuffer(rnode, forknum, blkno, recent_buffer)) + { + buffer = recent_buffer; + goto recent_buffer_fast_path; + } + /* Open the relation at smgr level */ smgr = smgropen(rnode, InvalidBackendId); @@ -523,6 +541,7 @@ XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum, } } +recent_buffer_fast_path: if (mode == RBM_NORMAL) { /* check that page has been initialized */ diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 40b7bca5a9..4608140bb5 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -905,6 +905,19 @@ CREATE VIEW pg_stat_wal_receiver AS FROM pg_stat_get_wal_receiver() s WHERE s.pid IS NOT NULL; +CREATE VIEW pg_stat_prefetch_recovery AS + SELECT + s.stats_reset, + s.prefetch, + s.hit, + s.skip_init, + s.skip_new, + s.skip_fpw, + s.wal_distance, + s.block_distance, + s.io_depth + FROM pg_stat_get_prefetch_recovery() s; + CREATE VIEW pg_stat_subscription AS SELECT su.oid AS subid, diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c index 78c073b7c9..d41ae37090 100644 --- a/src/backend/storage/freespace/freespace.c +++ b/src/backend/storage/freespace/freespace.c @@ -211,7 +211,8 @@ XLogRecordPageWithFreeSpace(RelFileNode rnode, BlockNumber heapBlk, blkno = fsm_logical_to_physical(addr); /* If the page doesn't exist already, extend */ - buf = XLogReadBufferExtended(rnode, FSM_FORKNUM, blkno, RBM_ZERO_ON_ERROR); + buf = XLogReadBufferExtended(rnode, FSM_FORKNUM, blkno, RBM_ZERO_ON_ERROR, + InvalidBuffer); LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE); page = BufferGetPage(buf); diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c index cd4ebe2fc5..17f54b153b 100644 --- a/src/backend/storage/ipc/ipci.c +++ b/src/backend/storage/ipc/ipci.c @@ -22,6 +22,7 @@ #include "access/subtrans.h" #include "access/syncscan.h" #include "access/twophase.h" +#include "access/xlogprefetcher.h" #include "access/xlogrecovery.h" #include "commands/async.h" #include "miscadmin.h" @@ -119,6 +120,7 @@ CalculateShmemSize(int *num_semaphores) size = add_size(size, LockShmemSize()); size = add_size(size, PredicateLockShmemSize()); size = add_size(size, ProcGlobalShmemSize()); + size = add_size(size, XLogPrefetchShmemSize()); size = add_size(size, XLOGShmemSize()); size = add_size(size, XLogRecoveryShmemSize()); size = add_size(size, CLOGShmemSize()); @@ -243,6 +245,7 @@ CreateSharedMemoryAndSemaphores(void) * Set up xlog, clog, and buffers */ XLOGShmemInit(); + XLogPrefetchShmemInit(); XLogRecoveryShmemInit(); CLOGShmemInit(); CommitTsShmemInit(); diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index e7f0a380e6..38f93f204f 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -41,6 +41,7 @@ #include "access/twophase.h" #include "access/xact.h" #include "access/xlog_internal.h" +#include "access/xlogprefetcher.h" #include "access/xlogrecovery.h" #include "catalog/namespace.h" #include "catalog/pg_authid.h" @@ -215,6 +216,7 @@ static bool check_effective_io_concurrency(int *newval, void **extra, GucSource static bool check_maintenance_io_concurrency(int *newval, void **extra, GucSource source); static bool check_huge_page_size(int *newval, void **extra, GucSource source); static bool check_client_connection_check_interval(int *newval, void **extra, GucSource source); +static void assign_maintenance_io_concurrency(int newval, void *extra); static void assign_pgstat_temp_directory(const char *newval, void *extra); static bool check_application_name(char **newval, void **extra, GucSource source); static void assign_application_name(const char *newval, void *extra); @@ -1321,6 +1323,15 @@ static struct config_bool ConfigureNamesBool[] = true, NULL, NULL, NULL }, + { + {"recovery_prefetch", PGC_SIGHUP, WAL_SETTINGS, + gettext_noop("Prefetch referenced blocks during recovery"), + gettext_noop("Read ahead of the current replay position to find uncached blocks.") + }, + &recovery_prefetch, + false, + check_recovery_prefetch, assign_recovery_prefetch, NULL + }, { {"wal_log_hints", PGC_POSTMASTER, WAL_SETTINGS, @@ -2792,6 +2803,17 @@ static struct config_int ConfigureNamesInt[] = NULL, NULL, NULL }, + { + {"wal_decode_buffer_size", PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY, + gettext_noop("Maximum buffer size for reading ahead in the WAL during recovery."), + gettext_noop("This controls the maximum distance we can read ahead in the WAL to prefetch referenced blocks."), + GUC_UNIT_BYTE + }, + &wal_decode_buffer_size, + 512 * 1024, 64 * 1024, INT_MAX, + NULL, NULL, NULL + }, + { {"wal_keep_size", PGC_SIGHUP, REPLICATION_SENDING, gettext_noop("Sets the size of WAL files held for standby servers."), @@ -3115,7 +3137,8 @@ static struct config_int ConfigureNamesInt[] = 0, #endif 0, MAX_IO_CONCURRENCY, - check_maintenance_io_concurrency, NULL, NULL + check_maintenance_io_concurrency, assign_maintenance_io_concurrency, + NULL }, { @@ -12211,6 +12234,20 @@ check_client_connection_check_interval(int *newval, void **extra, GucSource sour return true; } +static void +assign_maintenance_io_concurrency(int newval, void *extra) +{ +#ifdef USE_PREFETCH + /* + * Reconfigure recovery prefetching, because a setting it depends on + * changed. + */ + maintenance_io_concurrency = newval; + if (AmStartupProcess()) + XLogPrefetchReconfigure(); +#endif +} + static void assign_pgstat_temp_directory(const char *newval, void *extra) { diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 4cf5b26a36..0a6c7bd83e 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -241,6 +241,11 @@ #max_wal_size = 1GB #min_wal_size = 80MB +# - Prefetching during recovery - + +#wal_decode_buffer_size = 512kB # lookahead window used for prefetching +#recovery_prefetch = off # prefetch pages referenced in the WAL? + # - Archiving - #archive_mode = off # enables archiving; off, on, or always diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 09f6464331..1df9dd2fbe 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -50,6 +50,7 @@ extern bool *wal_consistency_checking; extern char *wal_consistency_checking_string; extern bool log_checkpoints; extern bool track_wal_io_timing; +extern int wal_decode_buffer_size; extern int CheckPointSegments; diff --git a/src/include/access/xlogprefetcher.h b/src/include/access/xlogprefetcher.h new file mode 100644 index 0000000000..f5bdb920d5 --- /dev/null +++ b/src/include/access/xlogprefetcher.h @@ -0,0 +1,43 @@ +/*------------------------------------------------------------------------- + * + * xlogprefetcher.h + * Declarations for the recovery prefetching module. + * + * Portions Copyright (c) 2021, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * IDENTIFICATION + * src/include/access/xlogprefetcher.h + *------------------------------------------------------------------------- + */ +#ifndef XLOGPREFETCHER_H +#define XLOGPREFETCHER_H + +#include "access/xlogdefs.h" + +/* GUCs */ +extern bool recovery_prefetch; + +struct XLogPrefetcher; +typedef struct XLogPrefetcher XLogPrefetcher; + + +extern void XLogPrefetchReconfigure(void); + +extern size_t XLogPrefetchShmemSize(void); +extern void XLogPrefetchShmemInit(void); + +extern void XLogPrefetchRequestResetStats(void); + +extern XLogPrefetcher *XLogPrefetcherAllocate(XLogReaderState *reader); +extern void XLogPrefetcherFree(XLogPrefetcher *prefetcher); + +extern XLogReaderState *XLogPrefetcherReader(XLogPrefetcher *prefetcher); + +extern void XLogPrefetcherBeginRead(XLogPrefetcher *prefetcher, + XLogRecPtr recPtr); + +extern XLogRecord *XLogPrefetcherReadRecord(XLogPrefetcher *prefetcher, + char **errmsg); + +#endif diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h index d1f364f4e8..8446050225 100644 --- a/src/include/access/xlogreader.h +++ b/src/include/access/xlogreader.h @@ -39,6 +39,7 @@ #endif #include "access/xlogrecord.h" +#include "storage/buf.h" /* WALOpenSegment represents a WAL segment being read. */ typedef struct WALOpenSegment @@ -125,6 +126,9 @@ typedef struct ForkNumber forknum; BlockNumber blkno; + /* Prefetching workspace. */ + Buffer prefetch_buffer; + /* copy of the fork_flags field from the XLogRecordBlockHeader */ uint8 flags; @@ -427,5 +431,9 @@ extern char *XLogRecGetBlockData(XLogReaderState *record, uint8 block_id, Size * extern bool XLogRecGetBlockTag(XLogReaderState *record, uint8 block_id, RelFileNode *rnode, ForkNumber *forknum, BlockNumber *blknum); +extern bool XLogRecGetBlockInfo(XLogReaderState *record, uint8 block_id, + RelFileNode *rnode, ForkNumber *forknum, + BlockNumber *blknum, + Buffer *prefetch_buffer); #endif /* XLOGREADER_H */ diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h index 64708949db..ff40f96e42 100644 --- a/src/include/access/xlogutils.h +++ b/src/include/access/xlogutils.h @@ -84,7 +84,8 @@ extern XLogRedoAction XLogReadBufferForRedoExtended(XLogReaderState *record, Buffer *buf); extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum, - BlockNumber blkno, ReadBufferMode mode); + BlockNumber blkno, ReadBufferMode mode, + Buffer recent_buffer); extern Relation CreateFakeRelcacheEntry(RelFileNode rnode); extern void FreeFakeRelcacheEntry(Relation fakerel); diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index d8e8715ed1..534ad0a5fb 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -6360,6 +6360,14 @@ prorettype => 'text', proargtypes => '', prosrc => 'pg_get_wal_replay_pause_state' }, +{ oid => '9085', descr => 'statistics: information about WAL prefetching', + proname => 'pg_stat_get_prefetch_recovery', prorows => '1', provolatile => 'v', + proretset => 't', prorettype => 'record', proargtypes => '', + proallargtypes => '{timestamptz,int8,int8,int8,int8,int8,int4,int4,int4}', + proargmodes => '{o,o,o,o,o,o,o,o,o}', + proargnames => '{stats_reset,prefetch,hit,skip_init,skip_new,skip_fpw,wal_distance,block_distance,io_depth}', + prosrc => 'pg_stat_get_prefetch_recovery' }, + { oid => '2621', descr => 'reload configuration files', proname => 'pg_reload_conf', provolatile => 'v', prorettype => 'bool', proargtypes => '', prosrc => 'pg_reload_conf' }, diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index ea774968f0..de59b08772 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -450,4 +450,8 @@ extern void assign_search_path(const char *newval, void *extra); extern bool check_wal_buffers(int *newval, void **extra, GucSource source); extern void assign_xlog_sync_method(int new_sync_method, void *extra); +/* in access/transam/xlogprefetcher.c */ +extern bool check_recovery_prefetch(bool *new_value, void **extra, GucSource source); +extern void assign_recovery_prefetch(bool new_value, void *extra); + #endif /* GUC_H */ diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index ac468568a1..8ad54191cd 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1857,6 +1857,16 @@ pg_stat_gssapi| SELECT s.pid, s.gss_enc AS encrypted FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) WHERE (s.client_port IS NOT NULL); +pg_stat_prefetch_recovery| SELECT s.stats_reset, + s.prefetch, + s.hit, + s.skip_init, + s.skip_new, + s.skip_fpw, + s.wal_distance, + s.block_distance, + s.io_depth + FROM pg_stat_get_prefetch_recovery() s(stats_reset, prefetch, hit, skip_init, skip_new, skip_fpw, wal_distance, block_distance, io_depth); pg_stat_progress_analyze| SELECT s.pid, s.datid, d.datname, diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index f57f7e0f53..3a008ef433 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1408,6 +1408,9 @@ LogicalRepWorker LogicalRewriteMappingData LogicalTape LogicalTapeSet +LsnReadQueue +LsnReadQueueNextFun +LsnReadQueueNextStatus LtreeGistOptions LtreeSignature MAGIC @@ -2941,6 +2944,10 @@ XLogPageHeaderData XLogPageReadCB XLogPageReadPrivate XLogPageReadResult +XLogPrefetcher +XLogPrefetcherFilter +XLogPrefetchState +XLogPrefetchStats XLogReaderRoutine XLogReaderState XLogRecData -- 2.30.2 ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: WIP: WAL prefetch (another approach) @ 2022-03-11 06:03 Andres Freund <[email protected]> parent: Thomas Munro <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Andres Freund @ 2022-03-11 06:03 UTC (permalink / raw) To: Thomas Munro <[email protected]>; Julien Rouhaud <[email protected]>; +Cc: Ashutosh Sharma <[email protected]>; Daniel Gustafsson <[email protected]>; Stephen Frost <[email protected]>; Alvaro Herrera <[email protected]>; David Steele <[email protected]>; Dmitry Dolgov <[email protected]>; Jakub Wartak <[email protected]>; Tom Lane <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; pgsql-hackers On March 10, 2022 9:31:13 PM PST, Thomas Munro <[email protected]> wrote: > The other thing I need to change is that I should turn on >recovery_prefetch for platforms that support it (ie Linux and maybe >NetBSD only for now), in the tests. Could a setting of "try" make sense? -- Sent from my Android device with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [nested|flat] 23+ messages in thread
end of thread, other threads:[~2022-03-11 06:03 UTC | newest] Thread overview: 23+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2020-07-02 23:46 [PATCH v3 2/2] Avoid bitmap index scan inconsistent with partition constraint Justin Pryzby <[email protected]> 2022-03-09 06:46 Re: WIP: WAL prefetch (another approach) Julien Rouhaud <[email protected]> 2022-03-11 05:31 ` Re: WIP: WAL prefetch (another approach) Thomas Munro <[email protected]> 2022-03-11 06:03 ` Re: WIP: WAL prefetch (another approach) Andres Freund <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox