From 6d30c7bf0bf25618e159c937aeb5d3080d5ca825 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 1 Nov 2020 15:46:03 -0600 Subject: [PATCH 1/9] Refactor CIC to rely on REINDEX CONCURRENTLY --- src/backend/commands/indexcmds.c | 187 +-------------------- src/test/regress/expected/create_index.out | 10 +- src/test/regress/sql/create_index.sql | 4 +- 3 files changed, 16 insertions(+), 185 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 75552c64ed..a3ba24947e 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -526,12 +526,8 @@ DefineIndex(Oid relationId, bits16 constr_flags; int numberOfAttributes; int numberOfKeyAttributes; - TransactionId limitXmin; ObjectAddress address; - LockRelId heaprelid; - LOCKTAG heaplocktag; LOCKMODE lockmode; - Snapshot snapshot; int save_nestlevel = -1; int i; @@ -1109,6 +1105,11 @@ DefineIndex(Oid relationId, if (pd->nparts != 0) flags |= INDEX_CREATE_INVALID; } + else if (concurrent) + { + /* If concurrent, initially build indexes as "invalid" */ + flags |= INDEX_CREATE_INVALID; + } if (stmt->deferrable) constr_flags |= INDEX_CONSTR_CREATE_DEFERRABLE; @@ -1387,11 +1388,9 @@ DefineIndex(Oid relationId, return address; } + table_close(rel, NoLock); if (!concurrent) { - /* Close the heap and we're done, in the non-concurrent case */ - table_close(rel, NoLock); - /* If this is the top-level index, we're done. */ if (!OidIsValid(parentIndexId)) pgstat_progress_end_command(); @@ -1399,185 +1398,13 @@ DefineIndex(Oid relationId, return address; } - /* save lockrelid and locktag for below, then close rel */ - heaprelid = rel->rd_lockInfo.lockRelId; - SET_LOCKTAG_RELATION(heaplocktag, heaprelid.dbId, heaprelid.relId); - table_close(rel, NoLock); - - /* - * For a concurrent build, it's important to make the catalog entries - * visible to other transactions before we start to build the index. That - * will prevent them from making incompatible HOT updates. The new index - * will be marked not indisready and not indisvalid, so that no one else - * tries to either insert into it or use it for queries. - * - * We must commit our current transaction so that the index becomes - * visible; then start another. Note that all the data structures we just - * built are lost in the commit. The only data we keep past here are the - * relation IDs. - * - * Before committing, get a session-level lock on the table, to ensure - * that neither it nor the index can be dropped before we finish. This - * cannot block, even if someone else is waiting for access, because we - * already have the same lock within our transaction. - * - * Note: we don't currently bother with a session lock on the index, - * because there are no operations that could change its state while we - * hold lock on the parent table. This might need to change later. - */ - LockRelationIdForSession(&heaprelid, ShareUpdateExclusiveLock); - - PopActiveSnapshot(); - CommitTransactionCommand(); - StartTransactionCommand(); - /* * The index is now visible, so we can report the OID. */ pgstat_progress_update_param(PROGRESS_CREATEIDX_INDEX_OID, indexRelationId); - /* - * Phase 2 of concurrent index build (see comments for validate_index() - * for an overview of how this works) - * - * Now we must wait until no running transaction could have the table open - * with the old list of indexes. Use ShareLock to consider running - * transactions that hold locks that permit writing to the table. Note we - * do not need to worry about xacts that open the table for writing after - * this point; they will see the new index when they open it. - * - * Note: the reason we use actual lock acquisition here, rather than just - * checking the ProcArray and sleeping, is that deadlock is possible if - * one of the transactions in question is blocked trying to acquire an - * exclusive lock on our table. The lock code will detect deadlock and - * error out properly. - */ - pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE, - PROGRESS_CREATEIDX_PHASE_WAIT_1); - WaitForLockers(heaplocktag, ShareLock, true); - - /* - * At this moment we are sure that there are no transactions with the - * table open for write that don't have this new index in their list of - * indexes. We have waited out all the existing transactions and any new - * transaction will have the new index in its list, but the index is still - * marked as "not-ready-for-inserts". The index is consulted while - * deciding HOT-safety though. This arrangement ensures that no new HOT - * chains can be created where the new tuple and the old tuple in the - * chain have different index keys. - * - * We now take a new snapshot, and build the index using all tuples that - * are visible in this snapshot. We can be sure that any HOT updates to - * these tuples will be compatible with the index, since any updates made - * by transactions that didn't know about the index are now committed or - * rolled back. Thus, each visible tuple is either the end of its - * HOT-chain or the extension of the chain is HOT-safe for this index. - */ - - /* Set ActiveSnapshot since functions in the indexes may need it */ - PushActiveSnapshot(GetTransactionSnapshot()); - - /* Perform concurrent build of index */ - index_concurrently_build(relationId, indexRelationId); - - /* we can do away with our snapshot */ - PopActiveSnapshot(); - - /* - * Commit this transaction to make the indisready update visible. - */ - CommitTransactionCommand(); - StartTransactionCommand(); - - /* - * Phase 3 of concurrent index build - * - * We once again wait until no transaction can have the table open with - * the index marked as read-only for updates. - */ - pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE, - PROGRESS_CREATEIDX_PHASE_WAIT_2); - WaitForLockers(heaplocktag, ShareLock, true); - - /* - * Now take the "reference snapshot" that will be used by validate_index() - * to filter candidate tuples. Beware! There might still be snapshots in - * use that treat some transaction as in-progress that our reference - * snapshot treats as committed. If such a recently-committed transaction - * deleted tuples in the table, we will not include them in the index; yet - * those transactions which see the deleting one as still-in-progress will - * expect such tuples to be there once we mark the index as valid. - * - * We solve this by waiting for all endangered transactions to exit before - * we mark the index as valid. - * - * We also set ActiveSnapshot to this snap, since functions in indexes may - * need a snapshot. - */ - snapshot = RegisterSnapshot(GetTransactionSnapshot()); - PushActiveSnapshot(snapshot); - - /* - * Scan the index and the heap, insert any missing index entries. - */ - validate_index(relationId, indexRelationId, snapshot); - - /* - * Drop the reference snapshot. We must do this before waiting out other - * snapshot holders, else we will deadlock against other processes also - * doing CREATE INDEX CONCURRENTLY, which would see our snapshot as one - * they must wait for. But first, save the snapshot's xmin to use as - * limitXmin for GetCurrentVirtualXIDs(). - */ - limitXmin = snapshot->xmin; - - PopActiveSnapshot(); - UnregisterSnapshot(snapshot); - - /* - * The snapshot subsystem could still contain registered snapshots that - * are holding back our process's advertised xmin; in particular, if - * default_transaction_isolation = serializable, there is a transaction - * snapshot that is still active. The CatalogSnapshot is likewise a - * hazard. To ensure no deadlocks, we must commit and start yet another - * transaction, and do our wait before any snapshot has been taken in it. - */ - CommitTransactionCommand(); - StartTransactionCommand(); - - /* We should now definitely not be advertising any xmin. */ - Assert(MyProc->xmin == InvalidTransactionId); - - /* - * The index is now valid in the sense that it contains all currently - * interesting tuples. But since it might not contain tuples deleted just - * before the reference snap was taken, we have to wait out any - * transactions that might have older snapshots. - */ - pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE, - PROGRESS_CREATEIDX_PHASE_WAIT_3); - WaitForOlderSnapshots(limitXmin, true); - - /* - * Index can now be marked valid -- update its pg_index entry - */ - index_set_state_flags(indexRelationId, INDEX_CREATE_SET_VALID); - - /* - * The pg_index update will cause backends (including this one) to update - * relcache entries for the index itself, but we should also send a - * relcache inval on the parent table to force replanning of cached plans. - * Otherwise existing sessions might fail to use the new index where it - * would be useful. (Note that our earlier commits did not create reasons - * to replan; so relcache flush on the index itself was sufficient.) - */ - CacheInvalidateRelcacheByRelid(heaprelid.relId); - - /* - * Last thing to do is release the session-level lock on the parent table. - */ - UnlockRelationIdForSession(&heaprelid, ShareUpdateExclusiveLock); + ReindexRelationConcurrently(indexRelationId, REINDEXOPT_CONCURRENTLY); pgstat_progress_end_command(); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index 012c1eb067..47f2236853 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1381,12 +1381,13 @@ ERROR: duplicate key value violates unique constraint "concur_index2" DETAIL: Key (f1)=(b) already exists. -- check if constraint is enforced properly at build time CREATE UNIQUE INDEX CONCURRENTLY concur_index3 ON concur_heap(f2); -ERROR: could not create unique index "concur_index3" +ERROR: could not create unique index "concur_index3_ccnew" DETAIL: Key (f2)=(b) is duplicated. -- test that expression indexes and partial indexes work concurrently CREATE INDEX CONCURRENTLY concur_index4 on concur_heap(f2) WHERE f1='a'; CREATE INDEX CONCURRENTLY concur_index5 on concur_heap(f2) WHERE f1='x'; -- here we also check that you can default the index name +DROP INDEX CONCURRENTLY IF EXISTS "concur_index3_ccnew"; CREATE INDEX CONCURRENTLY on concur_heap((f2||f1)); -- You can't do a concurrent index build in a transaction BEGIN; @@ -1467,7 +1468,7 @@ DROP INDEX CONCURRENTLY "concur_index2"; -- works DROP INDEX CONCURRENTLY IF EXISTS "concur_index2"; -- notice NOTICE: index "concur_index2" does not exist, skipping -- failures -DROP INDEX CONCURRENTLY "concur_index2", "concur_index3"; +DROP INDEX CONCURRENTLY "concur_index2", "concur_index3_ccnew"; ERROR: DROP INDEX CONCURRENTLY does not support dropping multiple objects BEGIN; DROP INDEX CONCURRENTLY "concur_index5"; @@ -2495,13 +2496,14 @@ CREATE TABLE concur_reindex_tab4 (c1 int); INSERT INTO concur_reindex_tab4 VALUES (1), (1), (2); -- This trick creates an invalid index. CREATE UNIQUE INDEX CONCURRENTLY concur_reindex_ind5 ON concur_reindex_tab4 (c1); -ERROR: could not create unique index "concur_reindex_ind5" +ERROR: could not create unique index "concur_reindex_ind5_ccnew" DETAIL: Key (c1)=(1) is duplicated. -- Reindexing concurrently this index fails with the same failure. -- The extra index created is itself invalid, and can be dropped. REINDEX INDEX CONCURRENTLY concur_reindex_ind5; -ERROR: could not create unique index "concur_reindex_ind5_ccnew" +ERROR: could not create unique index "concur_reindex_ind5_ccnew1" DETAIL: Key (c1)=(1) is duplicated. +DROP INDEX concur_reindex_ind5_ccnew1; \d concur_reindex_tab4 Table "public.concur_reindex_tab4" Column | Type | Collation | Nullable | Default diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index 4e45b18613..9ccb6201ab 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -480,6 +480,7 @@ CREATE UNIQUE INDEX CONCURRENTLY concur_index3 ON concur_heap(f2); CREATE INDEX CONCURRENTLY concur_index4 on concur_heap(f2) WHERE f1='a'; CREATE INDEX CONCURRENTLY concur_index5 on concur_heap(f2) WHERE f1='x'; -- here we also check that you can default the index name +DROP INDEX CONCURRENTLY IF EXISTS "concur_index3_ccnew"; CREATE INDEX CONCURRENTLY on concur_heap((f2||f1)); -- You can't do a concurrent index build in a transaction @@ -533,7 +534,7 @@ DROP INDEX CONCURRENTLY "concur_index2"; -- works DROP INDEX CONCURRENTLY IF EXISTS "concur_index2"; -- notice -- failures -DROP INDEX CONCURRENTLY "concur_index2", "concur_index3"; +DROP INDEX CONCURRENTLY "concur_index2", "concur_index3_ccnew"; BEGIN; DROP INDEX CONCURRENTLY "concur_index5"; ROLLBACK; @@ -1053,6 +1054,7 @@ CREATE UNIQUE INDEX CONCURRENTLY concur_reindex_ind5 ON concur_reindex_tab4 (c1) -- Reindexing concurrently this index fails with the same failure. -- The extra index created is itself invalid, and can be dropped. REINDEX INDEX CONCURRENTLY concur_reindex_ind5; +DROP INDEX concur_reindex_ind5_ccnew1; \d concur_reindex_tab4 DROP INDEX concur_reindex_ind5_ccnew; -- This makes the previous failure go away, so the index can become valid. -- 2.17.0