From fe72e8bb482afb2dfbf651d55b73435a362ca243 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Fri, 1 Oct 2021 14:32:47 +0300 Subject: [PATCH] Discard index deletion requests made to columnarAM (#5331) A write operation might trigger index deletion if index already had dead entries for the key we are about to insert. There are two ways of index deletion: a) simple deletion b) bottom-up deletion (>= pg14) Since columnar_index_fetch_tuple never sets all_dead to true, columnarAM doesn't ever expect to receive simple deletion requests (columnar_index_delete_tuples) as we don't mark any index entries as dead. However, since columnarAM doesn't delete any dead entries via simple deletion, postgres might ask for a more comprehensive deletion (i.e.: bottom-up) at some point when pg >= 14. So with this commit, we start gracefully ignoring bottom-up deletion requests made to columnar_index_delete_tuples. Given that users can anyway "VACUUM FULL" their columnar tables, we don't see any problem in ignoring deletion requests. --- src/backend/columnar/columnar_tableam.c | 49 +++++++++- .../regress/expected/columnar_indexes.out | 85 ++++++++++++++++++ src/test/regress/sql/columnar_indexes.sql | 89 +++++++++++++++++++ 3 files changed, 220 insertions(+), 3 deletions(-) diff --git a/src/backend/columnar/columnar_tableam.c b/src/backend/columnar/columnar_tableam.c index 1e767458f..98578612a 100644 --- a/src/backend/columnar/columnar_tableam.c +++ b/src/backend/columnar/columnar_tableam.c @@ -476,8 +476,11 @@ columnar_index_fetch_tuple(struct IndexFetchTableData *sscan, *call_again = false; /* - * No dead tuples are possible in columnar, set it to false if it's - * passed to be non-NULL. + * Initialize all_dead to false if passed to be non-NULL. + * + * XXX: For aborted writes, we should set all_dead to true but this would + * require implementing columnar_index_delete_tuples for simple deletion + * of dead tuples (TM_IndexDeleteOp.bottomup = false). */ if (all_dead) { @@ -655,7 +658,47 @@ static TransactionId columnar_index_delete_tuples(Relation rel, TM_IndexDeleteOp *delstate) { - elog(ERROR, "columnar_index_delete_tuples not implemented"); + CheckCitusVersion(ERROR); + + /* + * XXX: We didn't bother implementing index_delete_tuple for neither of + * simple deletion and bottom-up deletion cases. There is no particular + * reason for that, just to keep things simple. + * + * See the rest of this function to see how we deal with + * index_delete_tuples requests made to columnarAM. + */ + + if (delstate->bottomup) + { + /* + * Ignore any bottom-up deletion requests. + * + * Currently only caller in postgres that does bottom-up deletion is + * _bt_bottomupdel_pass, which in turn calls _bt_delitems_delete_check. + * And this function is okay with ndeltids being set to 0 by tableAM + * for bottom-up deletion. + */ + delstate->ndeltids = 0; + return InvalidTransactionId; + } + else + { + /* + * TableAM is not expected to set ndeltids to 0 for simple deletion + * case, so here we cannot do the same trick that we do for + * bottom-up deletion. + * See the assertion around table_index_delete_tuples call in pg + * function index_compute_xid_horizon_for_tuples. + * + * For this reason, to avoid receiving simple deletion requests for + * columnar tables (bottomup = false), columnar_index_fetch_tuple + * doesn't ever set all_dead to true in order to prevent triggering + * simple deletion of index tuples. But let's throw an error to be on + * the safe side. + */ + elog(ERROR, "columnar_index_delete_tuples not implemented for simple deletion"); + } } diff --git a/src/test/regress/expected/columnar_indexes.out b/src/test/regress/expected/columnar_indexes.out index 949e8c3a0..fa9d8d3ac 100644 --- a/src/test/regress/expected/columnar_indexes.out +++ b/src/test/regress/expected/columnar_indexes.out @@ -704,5 +704,90 @@ begin; insert into uniq select generate_series(1,100); ERROR: cannot read from index when there is unflushed data in upper transactions rollback; +-- Show that we nicely ignore index deletion requests made to columnarAM. +-- +-- An INSERT command might trigger index deletion if index already had dead +-- entries for the key we are about to insert. +-- There are two ways of index deletion: +-- a) simple deletion +-- b) bottom-up deletion (>= pg14) +-- +-- Since columnar_index_fetch_tuple never sets all_dead to true, columnarAM +-- doesn't expect to receive simple deletion as we don't mark any index +-- entries as dead. +-- Otherwise, columnarAM would throw an error for all of below six test cases. +-- +-- However, since columnarAM doesn't delete any dead entries via simple +-- deletion, postgres might ask for a more comprehensive deletion (bottom-up) +-- at some point when pg >= 14. +-- For this reason, all following six test cases would certainly trigger +-- bottom-up deletion. Show that we gracefully ignore such requests. +CREATE TABLE index_tuple_delete (a int UNIQUE) USING COLUMNAR; +ALTER TABLE index_tuple_delete SET (autovacuum_enabled = false); +BEGIN; + -- i) rollback before flushing + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; +ROLLBACK; +-- index deletion test-1 +BEGIN; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; +ROLLBACK; +COPY index_tuple_delete FROM PROGRAM 'seq 10000'; +TRUNCATE index_tuple_delete; +BEGIN; + -- ii) rollback after flushing + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; + SELECT SUM(a) > 0 FROM index_tuple_delete; + ?column? +--------------------------------------------------------------------- + t +(1 row) + +ROLLBACK; +-- index deletion test-2 +BEGIN; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; +ROLLBACK; +COPY index_tuple_delete FROM PROGRAM 'seq 10000'; +TRUNCATE index_tuple_delete; +BEGIN; + -- iii) rollback before flushing, use savepoint + SAVEPOINT sp1; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; + ROLLBACK TO sp1; + -- index deletion test-3 + SAVEPOINT sp2; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; + ROLLBACK TO sp2; + COPY index_tuple_delete FROM PROGRAM 'seq 10000'; +ROLLBACK; +-- index deletion test-4 +BEGIN; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; +ROLLBACK; +COPY index_tuple_delete FROM PROGRAM 'seq 10000'; +TRUNCATE index_tuple_delete; +BEGIN; + -- iv) rollback after flushing, use savepoint + SAVEPOINT sp1; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; + SELECT SUM(a) > 0 FROM index_tuple_delete; + ?column? +--------------------------------------------------------------------- + t +(1 row) + + ROLLBACK TO sp1; + -- index deletion test-5 + SAVEPOINT sp2; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; + ROLLBACK TO sp2; + COPY index_tuple_delete FROM PROGRAM 'seq 10000'; +ROLLBACK; +-- index deletion test-6 +BEGIN; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; +ROLLBACK; +COPY index_tuple_delete FROM PROGRAM 'seq 10000'; SET client_min_messages TO WARNING; DROP SCHEMA columnar_indexes CASCADE; diff --git a/src/test/regress/sql/columnar_indexes.sql b/src/test/regress/sql/columnar_indexes.sql index ce20db589..8fec947b2 100644 --- a/src/test/regress/sql/columnar_indexes.sql +++ b/src/test/regress/sql/columnar_indexes.sql @@ -546,5 +546,94 @@ begin; insert into uniq select generate_series(1,100); rollback; +-- Show that we nicely ignore index deletion requests made to columnarAM. +-- +-- An INSERT command might trigger index deletion if index already had dead +-- entries for the key we are about to insert. +-- There are two ways of index deletion: +-- a) simple deletion +-- b) bottom-up deletion (>= pg14) +-- +-- Since columnar_index_fetch_tuple never sets all_dead to true, columnarAM +-- doesn't expect to receive simple deletion as we don't mark any index +-- entries as dead. +-- Otherwise, columnarAM would throw an error for all of below six test cases. +-- +-- However, since columnarAM doesn't delete any dead entries via simple +-- deletion, postgres might ask for a more comprehensive deletion (bottom-up) +-- at some point when pg >= 14. +-- For this reason, all following six test cases would certainly trigger +-- bottom-up deletion. Show that we gracefully ignore such requests. +CREATE TABLE index_tuple_delete (a int UNIQUE) USING COLUMNAR; +ALTER TABLE index_tuple_delete SET (autovacuum_enabled = false); + +BEGIN; + -- i) rollback before flushing + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; +ROLLBACK; + +-- index deletion test-1 +BEGIN; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; +ROLLBACK; +COPY index_tuple_delete FROM PROGRAM 'seq 10000'; + +TRUNCATE index_tuple_delete; + +BEGIN; + -- ii) rollback after flushing + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; + SELECT SUM(a) > 0 FROM index_tuple_delete; +ROLLBACK; + +-- index deletion test-2 +BEGIN; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; +ROLLBACK; +COPY index_tuple_delete FROM PROGRAM 'seq 10000'; + +TRUNCATE index_tuple_delete; + +BEGIN; + -- iii) rollback before flushing, use savepoint + SAVEPOINT sp1; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; + ROLLBACK TO sp1; + + -- index deletion test-3 + SAVEPOINT sp2; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; + ROLLBACK TO sp2; + COPY index_tuple_delete FROM PROGRAM 'seq 10000'; +ROLLBACK; + +-- index deletion test-4 +BEGIN; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; +ROLLBACK; +COPY index_tuple_delete FROM PROGRAM 'seq 10000'; + +TRUNCATE index_tuple_delete; + +BEGIN; + -- iv) rollback after flushing, use savepoint + SAVEPOINT sp1; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; + SELECT SUM(a) > 0 FROM index_tuple_delete; + ROLLBACK TO sp1; + + -- index deletion test-5 + SAVEPOINT sp2; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; + ROLLBACK TO sp2; + COPY index_tuple_delete FROM PROGRAM 'seq 10000'; +ROLLBACK; + +-- index deletion test-6 +BEGIN; + INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i; +ROLLBACK; +COPY index_tuple_delete FROM PROGRAM 'seq 10000'; + SET client_min_messages TO WARNING; DROP SCHEMA columnar_indexes CASCADE;