Fix ALTER SEQUENCE locking

In 1753b1b027035029c2a2a1649065762fafbf63f3, the pg_sequence system
catalog was introduced.  This made sequence metadata changes
transactional, while the actual sequence values are still behaving
nontransactionally.  This requires some refinement in how ALTER
SEQUENCE, which operates on both, locks the sequence and the catalog.

The main problems were:

- Concurrent ALTER SEQUENCE causes "tuple concurrently updated" error,
  caused by updates to pg_sequence catalog.

- Sequence WAL writes and catalog updates are not protected by same
  lock, which could lead to inconsistent recovery order.

- nextval() disregarding uncommitted ALTER SEQUENCE changes.

To fix, nextval() and friends now lock the sequence using
RowExclusiveLock instead of AccessShareLock.  ALTER SEQUENCE locks the
sequence using ShareRowExclusiveLock.  This means that nextval() and
ALTER SEQUENCE block each other, and ALTER SEQUENCE on the same sequence
blocks itself.  (This was already the case previously for the OWNER TO,
RENAME, and SET SCHEMA variants.)  Also, rearrange some code so that the
entire AlterSequence is protected by the lock on the sequence.

As an exception, use reduced locking for ALTER SEQUENCE ... RESTART.
Since that is basically a setval(), it does not require the full locking
of other ALTER SEQUENCE actions.  So check whether we are only running a
RESTART and run with less locking if so.

Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
Reported-by: Jason Petersen <jason@citusdata.com>
Reported-by: Andres Freund <andres@anarazel.de>

3 files changed, 125 insertions, 0 deletions

diff --git a/src/test/isolation/expected/sequence-ddl.out b/src/test/isolation/expected/sequence-ddl.out
new file mode 100644
index 0000000000..6b7119738f
--- /dev/null
+++ b/src/test/isolation/expected/sequence-ddl.out
@@ -0,0 +1,85 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1alter s1commit s2nv
+step s1alter: ALTER SEQUENCE seq1 MAXVALUE 10;
+step s1commit: COMMIT;
+step s2nv: SELECT nextval('seq1') FROM generate_series(1, 15);
+ERROR:  nextval: reached maximum value of sequence "seq1" (10)
+
+starting permutation: s1alter s2nv s1commit
+step s1alter: ALTER SEQUENCE seq1 MAXVALUE 10;
+step s2nv: SELECT nextval('seq1') FROM generate_series(1, 15); <waiting ...>
+step s1commit: COMMIT;
+step s2nv: <... completed>
+error in steps s1commit s2nv: ERROR:  nextval: reached maximum value of sequence "seq1" (10)
+
+starting permutation: s2begin s2nv s1alter2 s2commit s1commit
+step s2begin: BEGIN;
+step s2nv: SELECT nextval('seq1') FROM generate_series(1, 15);
+nextval        
+
+1              
+2              
+3              
+4              
+5              
+6              
+7              
+8              
+9              
+10             
+11             
+12             
+13             
+14             
+15             
+step s1alter2: ALTER SEQUENCE seq1 MAXVALUE 20; <waiting ...>
+step s2commit: COMMIT;
+step s1alter2: <... completed>
+step s1commit: COMMIT;
+
+starting permutation: s1restart s2nv s1commit
+step s1restart: ALTER SEQUENCE seq1 RESTART WITH 5;
+step s2nv: SELECT nextval('seq1') FROM generate_series(1, 15);
+nextval        
+
+5              
+6              
+7              
+8              
+9              
+10             
+11             
+12             
+13             
+14             
+15             
+16             
+17             
+18             
+19             
+step s1commit: COMMIT;
+
+starting permutation: s2begin s2nv s1restart s2commit s1commit
+step s2begin: BEGIN;
+step s2nv: SELECT nextval('seq1') FROM generate_series(1, 15);
+nextval        
+
+1              
+2              
+3              
+4              
+5              
+6              
+7              
+8              
+9              
+10             
+11             
+12             
+13             
+14             
+15             
+step s1restart: ALTER SEQUENCE seq1 RESTART WITH 5;
+step s2commit: COMMIT;
+step s1commit: COMMIT;
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index fc1918dedc..32c965b2a0 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -58,6 +58,7 @@ test: alter-table-1
 test: alter-table-2
 test: alter-table-3
 test: create-trigger
+test: sequence-ddl
 test: async-notify
 test: vacuum-reltuples
 test: timeouts
diff --git a/src/test/isolation/specs/sequence-ddl.spec b/src/test/isolation/specs/sequence-ddl.spec
new file mode 100644
index 0000000000..42ee3b0615
--- /dev/null
+++ b/src/test/isolation/specs/sequence-ddl.spec
@@ -0,0 +1,39 @@
+# Test sequence usage and concurrent sequence DDL
+
+setup
+{
+    CREATE SEQUENCE seq1;
+}
+
+teardown
+{
+    DROP SEQUENCE seq1;
+}
+
+session "s1"
+setup           { BEGIN; }
+step "s1alter"  { ALTER SEQUENCE seq1 MAXVALUE 10; }
+step "s1alter2" { ALTER SEQUENCE seq1 MAXVALUE 20; }
+step "s1restart" { ALTER SEQUENCE seq1 RESTART WITH 5; }
+step "s1commit" { COMMIT; }
+
+session "s2"
+step "s2begin"  { BEGIN; }
+step "s2nv"     { SELECT nextval('seq1') FROM generate_series(1, 15); }
+step "s2commit" { COMMIT; }
+
+permutation "s1alter" "s1commit" "s2nv"
+
+# Prior to PG10, the s2nv would see the uncommitted s1alter change,
+# but now it waits.
+permutation "s1alter" "s2nv" "s1commit"
+
+# nextval doesn't release lock until transaction end, so s1alter2 has
+# to wait for s2commit.
+permutation "s2begin" "s2nv" "s1alter2" "s2commit" "s1commit"
+
+# RESTART is nontransactional, so s2nv sees it right away
+permutation "s1restart" "s2nv" "s1commit"
+
+# RESTART does not wait
+permutation "s2begin" "s2nv" "s1restart" "s2commit" "s1commit"