Thread: Proposal: remove obsolete hot-standby testing infrastructure

Proposal: remove obsolete hot-standby testing infrastructure

From
Tom Lane
Date:
The attached proposed patch removes some ancient infrastructure for
manually testing hot standby.  I doubt anyone has used this in years,
because AFAICS there is nothing here that's not done better by the
src/test/recovery TAP tests.  (Or if there is, we ought to migrate
it into the TAP tests.)

Thoughts?

            regards, tom lane

diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 724ef566e7..8cf10085d3 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -391,52 +391,6 @@ make check EXTRA_TESTS=numeric_big
 </screen>
    </para>
   </sect2>
-
-  <sect2>
-   <title>Testing Hot Standby</title>
-
-  <para>
-   The source distribution also contains regression tests for the static
-   behavior of Hot Standby. These tests require a running primary server
-   and a running standby server that is accepting new WAL changes from the
-   primary (using either file-based log shipping or streaming replication).
-   Those servers are not automatically created for you, nor is replication
-   setup documented here. Please check the various sections of the
-   documentation devoted to the required commands and related issues.
-  </para>
-
-  <para>
-   To run the Hot Standby tests, first create a database
-   called <literal>regression</literal> on the primary:
-<screen>
-psql -h primary -c "CREATE DATABASE regression"
-</screen>
-   Next, run the preparatory script
-   <filename>src/test/regress/sql/hs_primary_setup.sql</filename>
-   on the primary in the regression database, for example:
-<screen>
-psql -h primary -f src/test/regress/sql/hs_primary_setup.sql regression
-</screen>
-   Allow these changes to propagate to the standby.
-  </para>
-
-  <para>
-   Now arrange for the default database connection to be to the standby
-   server under test (for example, by setting the <envar>PGHOST</envar> and
-   <envar>PGPORT</envar> environment variables).
-   Finally, run <literal>make standbycheck</literal> in the regression directory:
-<screen>
-cd src/test/regress
-make standbycheck
-</screen>
-  </para>
-
-  <para>
-   Some extreme behaviors can also be generated on the primary using the
-   script <filename>src/test/regress/sql/hs_primary_extremes.sql</filename>
-   to allow the behavior of the standby to be tested.
-  </para>
-  </sect2>
   </sect1>

   <sect1 id="regress-evaluation">
diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index 330eca2b83..1ef45b82ec 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -130,9 +130,6 @@ installcheck-parallel: all
 installcheck-tests: all
     $(pg_regress_installcheck) $(REGRESS_OPTS) $(TESTS) $(EXTRA_TESTS)

-standbycheck: all
-    $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/standby_schedule --use-existing
-
 # old interfaces follow...

 runcheck: check
diff --git a/src/test/regress/expected/hs_standby_allowed.out b/src/test/regress/expected/hs_standby_allowed.out
deleted file mode 100644
index 00b8faf9eb..0000000000
--- a/src/test/regress/expected/hs_standby_allowed.out
+++ /dev/null
@@ -1,218 +0,0 @@
---
--- Hot Standby tests
---
--- hs_standby_allowed.sql
---
--- SELECT
-select count(*) as should_be_1 from hs1;
- should_be_1
--------------
-           1
-(1 row)
-
-select count(*) as should_be_2 from hs2;
- should_be_2
--------------
-           2
-(1 row)
-
-select count(*) as should_be_3 from hs3;
- should_be_3
--------------
-           3
-(1 row)
-
-COPY hs1 TO '/tmp/copy_test';
-\! cat /tmp/copy_test
-1
--- Access sequence directly
-select is_called from hsseq;
- is_called
------------
- f
-(1 row)
-
--- Transactions
-begin;
-select count(*)  as should_be_1 from hs1;
- should_be_1
--------------
-           1
-(1 row)
-
-end;
-begin transaction read only;
-select count(*)  as should_be_1 from hs1;
- should_be_1
--------------
-           1
-(1 row)
-
-end;
-begin transaction isolation level repeatable read;
-select count(*) as should_be_1 from hs1;
- should_be_1
--------------
-           1
-(1 row)
-
-select count(*) as should_be_1 from hs1;
- should_be_1
--------------
-           1
-(1 row)
-
-select count(*) as should_be_1 from hs1;
- should_be_1
--------------
-           1
-(1 row)
-
-commit;
-begin;
-select count(*) as should_be_1 from hs1;
- should_be_1
--------------
-           1
-(1 row)
-
-commit;
-begin;
-select count(*) as should_be_1 from hs1;
- should_be_1
--------------
-           1
-(1 row)
-
-abort;
-start transaction;
-select count(*) as should_be_1 from hs1;
- should_be_1
--------------
-           1
-(1 row)
-
-commit;
-begin;
-select count(*) as should_be_1 from hs1;
- should_be_1
--------------
-           1
-(1 row)
-
-rollback;
-begin;
-select count(*) as should_be_1 from hs1;
- should_be_1
--------------
-           1
-(1 row)
-
-savepoint s;
-select count(*) as should_be_2 from hs2;
- should_be_2
--------------
-           2
-(1 row)
-
-commit;
-begin;
-select count(*) as should_be_1 from hs1;
- should_be_1
--------------
-           1
-(1 row)
-
-savepoint s;
-select count(*) as should_be_2 from hs2;
- should_be_2
--------------
-           2
-(1 row)
-
-release savepoint s;
-select count(*) as should_be_2 from hs2;
- should_be_2
--------------
-           2
-(1 row)
-
-savepoint s;
-select count(*) as should_be_3 from hs3;
- should_be_3
--------------
-           3
-(1 row)
-
-rollback to savepoint s;
-select count(*) as should_be_2 from hs2;
- should_be_2
--------------
-           2
-(1 row)
-
-commit;
--- SET parameters
--- has no effect on read only transactions, but we can still set it
-set synchronous_commit = on;
-show synchronous_commit;
- synchronous_commit
---------------------
- on
-(1 row)
-
-reset synchronous_commit;
-discard temp;
-discard all;
--- CURSOR commands
-BEGIN;
-DECLARE hsc CURSOR FOR select * from hs3;
-FETCH next from hsc;
- col1
-------
-  113
-(1 row)
-
-fetch first from hsc;
- col1
-------
-  113
-(1 row)
-
-fetch last from hsc;
- col1
-------
-  115
-(1 row)
-
-fetch 1 from hsc;
- col1
-------
-(0 rows)
-
-CLOSE hsc;
-COMMIT;
--- Prepared plans
-PREPARE hsp AS select count(*) from hs1;
-PREPARE hsp_noexec (integer) AS insert into hs1 values ($1);
-EXECUTE hsp;
- count
--------
-     1
-(1 row)
-
-DEALLOCATE hsp;
--- LOCK
-BEGIN;
-LOCK hs1 IN ACCESS SHARE MODE;
-LOCK hs1 IN ROW SHARE MODE;
-LOCK hs1 IN ROW EXCLUSIVE MODE;
-COMMIT;
--- UNLISTEN
-UNLISTEN a;
-UNLISTEN *;
--- LOAD
--- should work, easier if there is no test for that...
--- ALLOWED COMMANDS
-CHECKPOINT;
-discard all;
diff --git a/src/test/regress/expected/hs_standby_check.out b/src/test/regress/expected/hs_standby_check.out
deleted file mode 100644
index df885ea9e0..0000000000
--- a/src/test/regress/expected/hs_standby_check.out
+++ /dev/null
@@ -1,20 +0,0 @@
---
--- Hot Standby tests
---
--- hs_standby_check.sql
---
---
--- If the query below returns false then all other tests will fail after it.
---
-select case pg_is_in_recovery() when false then
-    'These tests are intended only for execution on a standby server that is reading ' ||
-    'WAL from a server upon which the regression database is already created and into ' ||
-    'which src/test/regress/sql/hs_primary_setup.sql has been run'
-else
-    'Tests are running on a standby server during recovery'
-end;
-                         case
--------------------------------------------------------
- Tests are running on a standby server during recovery
-(1 row)
-
diff --git a/src/test/regress/expected/hs_standby_disallowed.out b/src/test/regress/expected/hs_standby_disallowed.out
deleted file mode 100644
index 8d3cafa5ce..0000000000
--- a/src/test/regress/expected/hs_standby_disallowed.out
+++ /dev/null
@@ -1,133 +0,0 @@
---
--- Hot Standby tests
---
--- hs_standby_disallowed.sql
---
-SET transaction_read_only = off;
-ERROR:  cannot set transaction read-write mode during recovery
-begin transaction read write;
-ERROR:  cannot set transaction read-write mode during recovery
-commit;
-WARNING:  there is no transaction in progress
--- SELECT
-select * from hs1 FOR SHARE;
-ERROR:  cannot execute SELECT FOR SHARE in a read-only transaction
-select * from hs1 FOR UPDATE;
-ERROR:  cannot execute SELECT FOR UPDATE in a read-only transaction
--- DML
-BEGIN;
-insert into hs1 values (37);
-ERROR:  cannot execute INSERT in a read-only transaction
-ROLLBACK;
-BEGIN;
-delete from hs1 where col1 = 1;
-ERROR:  cannot execute DELETE in a read-only transaction
-ROLLBACK;
-BEGIN;
-update hs1 set col1 = NULL where col1 > 0;
-ERROR:  cannot execute UPDATE in a read-only transaction
-ROLLBACK;
-BEGIN;
-truncate hs3;
-ERROR:  cannot execute TRUNCATE TABLE in a read-only transaction
-ROLLBACK;
--- DDL
-create temporary table hstemp1 (col1 integer);
-ERROR:  cannot execute CREATE TABLE in a read-only transaction
-BEGIN;
-drop table hs2;
-ERROR:  cannot execute DROP TABLE in a read-only transaction
-ROLLBACK;
-BEGIN;
-create table hs4 (col1 integer);
-ERROR:  cannot execute CREATE TABLE in a read-only transaction
-ROLLBACK;
--- Sequences
-SELECT nextval('hsseq');
-ERROR:  cannot execute nextval() in a read-only transaction
--- Two-phase commit transaction stuff
-BEGIN;
-SELECT count(*) FROM hs1;
- count
--------
-     1
-(1 row)
-
-PREPARE TRANSACTION 'foobar';
-ERROR:  cannot execute PREPARE TRANSACTION during recovery
-ROLLBACK;
-BEGIN;
-SELECT count(*) FROM hs1;
- count
--------
-     1
-(1 row)
-
-COMMIT PREPARED 'foobar';
-ERROR:  cannot execute COMMIT PREPARED during recovery
-ROLLBACK;
-BEGIN;
-SELECT count(*) FROM hs1;
- count
--------
-     1
-(1 row)
-
-PREPARE TRANSACTION 'foobar';
-ERROR:  cannot execute PREPARE TRANSACTION during recovery
-ROLLBACK PREPARED 'foobar';
-ERROR:  current transaction is aborted, commands ignored until end of transaction block
-ROLLBACK;
-BEGIN;
-SELECT count(*) FROM hs1;
- count
--------
-     1
-(1 row)
-
-ROLLBACK PREPARED 'foobar';
-ERROR:  cannot execute ROLLBACK PREPARED during recovery
-ROLLBACK;
--- Locks
-BEGIN;
-LOCK hs1;
-ERROR:  cannot execute LOCK TABLE during recovery
-COMMIT;
-BEGIN;
-LOCK hs1 IN SHARE UPDATE EXCLUSIVE MODE;
-ERROR:  cannot execute LOCK TABLE during recovery
-COMMIT;
-BEGIN;
-LOCK hs1 IN SHARE MODE;
-ERROR:  cannot execute LOCK TABLE during recovery
-COMMIT;
-BEGIN;
-LOCK hs1 IN SHARE ROW EXCLUSIVE MODE;
-ERROR:  cannot execute LOCK TABLE during recovery
-COMMIT;
-BEGIN;
-LOCK hs1 IN EXCLUSIVE MODE;
-ERROR:  cannot execute LOCK TABLE during recovery
-COMMIT;
-BEGIN;
-LOCK hs1 IN ACCESS EXCLUSIVE MODE;
-ERROR:  cannot execute LOCK TABLE during recovery
-COMMIT;
--- Listen
-listen a;
-ERROR:  cannot execute LISTEN during recovery
-notify a;
-ERROR:  cannot execute NOTIFY during recovery
--- disallowed commands
-ANALYZE hs1;
-ERROR:  cannot execute ANALYZE during recovery
-VACUUM hs2;
-ERROR:  cannot execute VACUUM during recovery
-CLUSTER hs2 using hs1_pkey;
-ERROR:  cannot execute CLUSTER during recovery
-REINDEX TABLE hs2;
-ERROR:  cannot execute REINDEX during recovery
-REVOKE SELECT ON hs1 FROM PUBLIC;
-ERROR:  cannot execute REVOKE in a read-only transaction
-GRANT SELECT ON hs1 TO PUBLIC;
-ERROR:  cannot execute GRANT in a read-only transaction
diff --git a/src/test/regress/expected/hs_standby_functions.out b/src/test/regress/expected/hs_standby_functions.out
deleted file mode 100644
index ce846b758b..0000000000
--- a/src/test/regress/expected/hs_standby_functions.out
+++ /dev/null
@@ -1,40 +0,0 @@
---
--- Hot Standby tests
---
--- hs_standby_functions.sql
---
--- should fail
-select pg_current_xact_id();
-ERROR:  cannot execute pg_current_xact_id() during recovery
-select length(pg_current_snapshot()::text) >= 4;
- ?column?
-----------
- t
-(1 row)
-
-select pg_start_backup('should fail');
-ERROR:  recovery is in progress
-HINT:  WAL control functions cannot be executed during recovery.
-select pg_switch_wal();
-ERROR:  recovery is in progress
-HINT:  WAL control functions cannot be executed during recovery.
-select pg_stop_backup();
-ERROR:  recovery is in progress
-HINT:  WAL control functions cannot be executed during recovery.
--- should return no rows
-select * from pg_prepared_xacts;
- transaction | gid | prepared | owner | database
--------------+-----+----------+-------+----------
-(0 rows)
-
--- just the startup process
-select locktype, virtualxid, virtualtransaction, mode, granted
-from pg_locks where virtualxid = '1/1';
-  locktype  | virtualxid | virtualtransaction |     mode      | granted
-------------+------------+--------------------+---------------+---------
- virtualxid | 1/1        | 1/0                | ExclusiveLock | t
-(1 row)
-
--- suicide is painless
-select pg_cancel_backend(pg_backend_pid());
-ERROR:  canceling statement due to user request
diff --git a/src/test/regress/sql/hs_primary_extremes.sql b/src/test/regress/sql/hs_primary_extremes.sql
deleted file mode 100644
index 2051e2e5cf..0000000000
--- a/src/test/regress/sql/hs_primary_extremes.sql
+++ /dev/null
@@ -1,73 +0,0 @@
---
--- Hot Standby tests
---
--- hs_primary_extremes.sql
---
-
-drop table if exists hs_extreme;
-create table hs_extreme (col1 integer);
-
-CREATE OR REPLACE FUNCTION hs_subxids (n integer)
-RETURNS void
-LANGUAGE plpgsql
-AS $$
-    BEGIN
-      IF n <= 0 THEN RETURN; END IF;
-      INSERT INTO hs_extreme VALUES (n);
-      PERFORM hs_subxids(n - 1);
-      RETURN;
-    EXCEPTION WHEN raise_exception THEN NULL; END;
-$$;
-
-BEGIN;
-SELECT hs_subxids(257);
-ROLLBACK;
-BEGIN;
-SELECT hs_subxids(257);
-COMMIT;
-
-set client_min_messages = 'warning';
-
-CREATE OR REPLACE FUNCTION hs_locks_create (n integer)
-RETURNS void
-LANGUAGE plpgsql
-AS $$
-    BEGIN
-      IF n <= 0 THEN
-        CHECKPOINT;
-        RETURN;
-      END IF;
-      EXECUTE 'CREATE TABLE hs_locks_' || n::text || ' ()';
-      PERFORM hs_locks_create(n - 1);
-      RETURN;
-    EXCEPTION WHEN raise_exception THEN NULL; END;
-$$;
-
-CREATE OR REPLACE FUNCTION hs_locks_drop (n integer)
-RETURNS void
-LANGUAGE plpgsql
-AS $$
-    BEGIN
-      IF n <= 0 THEN
-        CHECKPOINT;
-        RETURN;
-      END IF;
-      EXECUTE 'DROP TABLE IF EXISTS hs_locks_' || n::text;
-      PERFORM hs_locks_drop(n - 1);
-      RETURN;
-    EXCEPTION WHEN raise_exception THEN NULL; END;
-$$;
-
-BEGIN;
-SELECT hs_locks_drop(257);
-SELECT hs_locks_create(257);
-SELECT count(*) > 257 FROM pg_locks;
-ROLLBACK;
-BEGIN;
-SELECT hs_locks_drop(257);
-SELECT hs_locks_create(257);
-SELECT count(*) > 257 FROM pg_locks;
-COMMIT;
-SELECT hs_locks_drop(257);
-
-SELECT pg_switch_wal();
diff --git a/src/test/regress/sql/hs_primary_setup.sql b/src/test/regress/sql/hs_primary_setup.sql
deleted file mode 100644
index eeb4421307..0000000000
--- a/src/test/regress/sql/hs_primary_setup.sql
+++ /dev/null
@@ -1,25 +0,0 @@
---
--- Hot Standby tests
---
--- hs_primary_setup.sql
---
-
-drop table if exists hs1;
-create table hs1 (col1 integer primary key);
-insert into hs1 values (1);
-
-drop table if exists hs2;
-create table hs2 (col1 integer primary key);
-insert into hs2 values (12);
-insert into hs2 values (13);
-
-drop table if exists hs3;
-create table hs3 (col1 integer primary key);
-insert into hs3 values (113);
-insert into hs3 values (114);
-insert into hs3 values (115);
-
-DROP sequence if exists hsseq;
-create sequence hsseq;
-
-SELECT pg_switch_wal();
diff --git a/src/test/regress/sql/hs_standby_allowed.sql b/src/test/regress/sql/hs_standby_allowed.sql
deleted file mode 100644
index 6debddc5e9..0000000000
--- a/src/test/regress/sql/hs_standby_allowed.sql
+++ /dev/null
@@ -1,125 +0,0 @@
---
--- Hot Standby tests
---
--- hs_standby_allowed.sql
---
-
--- SELECT
-
-select count(*) as should_be_1 from hs1;
-
-select count(*) as should_be_2 from hs2;
-
-select count(*) as should_be_3 from hs3;
-
-COPY hs1 TO '/tmp/copy_test';
-\! cat /tmp/copy_test
-
--- Access sequence directly
-select is_called from hsseq;
-
--- Transactions
-
-begin;
-select count(*)  as should_be_1 from hs1;
-end;
-
-begin transaction read only;
-select count(*)  as should_be_1 from hs1;
-end;
-
-begin transaction isolation level repeatable read;
-select count(*) as should_be_1 from hs1;
-select count(*) as should_be_1 from hs1;
-select count(*) as should_be_1 from hs1;
-commit;
-
-begin;
-select count(*) as should_be_1 from hs1;
-commit;
-
-begin;
-select count(*) as should_be_1 from hs1;
-abort;
-
-start transaction;
-select count(*) as should_be_1 from hs1;
-commit;
-
-begin;
-select count(*) as should_be_1 from hs1;
-rollback;
-
-begin;
-select count(*) as should_be_1 from hs1;
-savepoint s;
-select count(*) as should_be_2 from hs2;
-commit;
-
-begin;
-select count(*) as should_be_1 from hs1;
-savepoint s;
-select count(*) as should_be_2 from hs2;
-release savepoint s;
-select count(*) as should_be_2 from hs2;
-savepoint s;
-select count(*) as should_be_3 from hs3;
-rollback to savepoint s;
-select count(*) as should_be_2 from hs2;
-commit;
-
--- SET parameters
-
--- has no effect on read only transactions, but we can still set it
-set synchronous_commit = on;
-show synchronous_commit;
-reset synchronous_commit;
-
-discard temp;
-discard all;
-
--- CURSOR commands
-
-BEGIN;
-
-DECLARE hsc CURSOR FOR select * from hs3;
-
-FETCH next from hsc;
-fetch first from hsc;
-fetch last from hsc;
-fetch 1 from hsc;
-
-CLOSE hsc;
-
-COMMIT;
-
--- Prepared plans
-
-PREPARE hsp AS select count(*) from hs1;
-PREPARE hsp_noexec (integer) AS insert into hs1 values ($1);
-
-EXECUTE hsp;
-
-DEALLOCATE hsp;
-
--- LOCK
-
-BEGIN;
-LOCK hs1 IN ACCESS SHARE MODE;
-LOCK hs1 IN ROW SHARE MODE;
-LOCK hs1 IN ROW EXCLUSIVE MODE;
-COMMIT;
-
--- UNLISTEN
-UNLISTEN a;
-UNLISTEN *;
-
--- LOAD
--- should work, easier if there is no test for that...
-
-
--- ALLOWED COMMANDS
-
-CHECKPOINT;
-
-discard all;
diff --git a/src/test/regress/sql/hs_standby_check.sql b/src/test/regress/sql/hs_standby_check.sql
deleted file mode 100644
index 3fe8a02720..0000000000
--- a/src/test/regress/sql/hs_standby_check.sql
+++ /dev/null
@@ -1,16 +0,0 @@
---
--- Hot Standby tests
---
--- hs_standby_check.sql
---
-
---
--- If the query below returns false then all other tests will fail after it.
---
-select case pg_is_in_recovery() when false then
-    'These tests are intended only for execution on a standby server that is reading ' ||
-    'WAL from a server upon which the regression database is already created and into ' ||
-    'which src/test/regress/sql/hs_primary_setup.sql has been run'
-else
-    'Tests are running on a standby server during recovery'
-end;
diff --git a/src/test/regress/sql/hs_standby_disallowed.sql b/src/test/regress/sql/hs_standby_disallowed.sql
deleted file mode 100644
index a470600eec..0000000000
--- a/src/test/regress/sql/hs_standby_disallowed.sql
+++ /dev/null
@@ -1,103 +0,0 @@
---
--- Hot Standby tests
---
--- hs_standby_disallowed.sql
---
-
-SET transaction_read_only = off;
-
-begin transaction read write;
-commit;
-
--- SELECT
-
-select * from hs1 FOR SHARE;
-select * from hs1 FOR UPDATE;
-
--- DML
-BEGIN;
-insert into hs1 values (37);
-ROLLBACK;
-BEGIN;
-delete from hs1 where col1 = 1;
-ROLLBACK;
-BEGIN;
-update hs1 set col1 = NULL where col1 > 0;
-ROLLBACK;
-BEGIN;
-truncate hs3;
-ROLLBACK;
-
--- DDL
-
-create temporary table hstemp1 (col1 integer);
-BEGIN;
-drop table hs2;
-ROLLBACK;
-BEGIN;
-create table hs4 (col1 integer);
-ROLLBACK;
-
--- Sequences
-
-SELECT nextval('hsseq');
-
--- Two-phase commit transaction stuff
-
-BEGIN;
-SELECT count(*) FROM hs1;
-PREPARE TRANSACTION 'foobar';
-ROLLBACK;
-BEGIN;
-SELECT count(*) FROM hs1;
-COMMIT PREPARED 'foobar';
-ROLLBACK;
-
-BEGIN;
-SELECT count(*) FROM hs1;
-PREPARE TRANSACTION 'foobar';
-ROLLBACK PREPARED 'foobar';
-ROLLBACK;
-
-BEGIN;
-SELECT count(*) FROM hs1;
-ROLLBACK PREPARED 'foobar';
-ROLLBACK;
-
-
--- Locks
-BEGIN;
-LOCK hs1;
-COMMIT;
-BEGIN;
-LOCK hs1 IN SHARE UPDATE EXCLUSIVE MODE;
-COMMIT;
-BEGIN;
-LOCK hs1 IN SHARE MODE;
-COMMIT;
-BEGIN;
-LOCK hs1 IN SHARE ROW EXCLUSIVE MODE;
-COMMIT;
-BEGIN;
-LOCK hs1 IN EXCLUSIVE MODE;
-COMMIT;
-BEGIN;
-LOCK hs1 IN ACCESS EXCLUSIVE MODE;
-COMMIT;
-
--- Listen
-listen a;
-notify a;
-
--- disallowed commands
-
-ANALYZE hs1;
-
-VACUUM hs2;
-
-CLUSTER hs2 using hs1_pkey;
-
-REINDEX TABLE hs2;
-
-REVOKE SELECT ON hs1 FROM PUBLIC;
-GRANT SELECT ON hs1 TO PUBLIC;
diff --git a/src/test/regress/sql/hs_standby_functions.sql b/src/test/regress/sql/hs_standby_functions.sql
deleted file mode 100644
index b57f67ff8b..0000000000
--- a/src/test/regress/sql/hs_standby_functions.sql
+++ /dev/null
@@ -1,24 +0,0 @@
---
--- Hot Standby tests
---
--- hs_standby_functions.sql
---
-
--- should fail
-select pg_current_xact_id();
-
-select length(pg_current_snapshot()::text) >= 4;
-
-select pg_start_backup('should fail');
-select pg_switch_wal();
-select pg_stop_backup();
-
--- should return no rows
-select * from pg_prepared_xacts;
-
--- just the startup process
-select locktype, virtualxid, virtualtransaction, mode, granted
-from pg_locks where virtualxid = '1/1';
-
--- suicide is painless
-select pg_cancel_backend(pg_backend_pid());
diff --git a/src/test/regress/standby_schedule b/src/test/regress/standby_schedule
deleted file mode 100644
index 3ddb961c88..0000000000
--- a/src/test/regress/standby_schedule
+++ /dev/null
@@ -1,21 +0,0 @@
-# src/test/regress/standby_schedule
-#
-# Test schedule for Hot Standby
-#
-# First test checks we are on a standby server.
-# Subsequent tests rely upon a setup script having already
-# been executed in the appropriate database on the primary server
-# which is feeding WAL files to target standby.
-#
-# psql -f src/test/regress/sql/hs_primary_setup.sql regression
-#
-test: hs_standby_check
-#
-# These tests will pass on both primary and standby servers
-#
-test: hs_standby_allowed
-#
-# These tests will fail on a non-standby server
-#
-test: hs_standby_disallowed
-test: hs_standby_functions

Re: Proposal: remove obsolete hot-standby testing infrastructure

From
Alexander Lakhin
Date:
Hello Tom,
04.01.2022 00:50, Tom Lane wrote:
> The attached proposed patch removes some ancient infrastructure for
> manually testing hot standby.  I doubt anyone has used this in years,
> because AFAICS there is nothing here that's not done better by the
> src/test/recovery TAP tests.  (Or if there is, we ought to migrate
> it into the TAP tests.)
>
> Thoughts?
It's hardly that important, but we (Postgres Pro) run this test
regularly to check for primary-standby compatibility. It's useful when
checking binary packages from different minor versions. For example, we
setup postgresql-14.0 and postgresql-14.1 aside (renaming one
installation' directory and changing it's port) and perform the test.
What've found with it was e.g. incompatibility due to linkage of
different libicu versions (that was PgPro-only issue). I don't remember
whether we found something related to PostgreSQL itself, but we
definitely use this test and I'm not sure how to replace it in our setup
with a TAP test. On the other hand, testing binaries is not accustomed
in the community yet, so when such testing will be adopted, probably a
brand new set of tests should emerge.

Best regards,
Alexander



Re: Proposal: remove obsolete hot-standby testing infrastructure

From
Tom Lane
Date:
Alexander Lakhin <exclusion@gmail.com> writes:
> 04.01.2022 00:50, Tom Lane wrote:
>> The attached proposed patch removes some ancient infrastructure for
>> manually testing hot standby.  I doubt anyone has used this in years,
>> because AFAICS there is nothing here that's not done better by the
>> src/test/recovery TAP tests.  (Or if there is, we ought to migrate
>> it into the TAP tests.)

> It's hardly that important, but we (Postgres Pro) run this test
> regularly to check for primary-standby compatibility. It's useful when
> checking binary packages from different minor versions. For example, we
> setup postgresql-14.0 and postgresql-14.1 aside (renaming one
> installation' directory and changing it's port) and perform the test.
> What've found with it was e.g. incompatibility due to linkage of
> different libicu versions (that was PgPro-only issue). I don't remember
> whether we found something related to PostgreSQL itself, but we
> definitely use this test and I'm not sure how to replace it in our setup
> with a TAP test. On the other hand, testing binaries is not accustomed
> in the community yet, so when such testing will be adopted, probably a
> brand new set of tests should emerge.

Oh, interesting.  I definitely concur that testing compatibility of
different builds or minor versions is an important use-case.  And
I concede that making src/test/recovery do it would be tricky and
a bit out-of-scope.  But having said that, the hs_standby_* scripts
seem like a poor fit for the job too.  AFAICS they don't really
test any user data type except integer (so I'm surprised that they
located an ICU incompatibility for you); and they spend a lot of
effort on stuff that I doubt is relevant because it *is* covered
by the TAP tests.

If I were trying to test that topic using available spare parts,
what I'd do is run the regular regression tests on the primary
and see if the standby could track it.  Maybe pg_dump from both
servers afterwards and see if the results match, a la the pg_upgrade
test.  Bonus points for a script that could run some other pg_regress
suite such as one of the contrib modules, because then you could
check compatibility of those too.

I'm happy to keep the hs_standby_* scripts if there's a live use-case
for them; but I don't see what they're doing for you that wouldn't be
done better by other pg_regress suites.

            regards, tom lane



Re: Proposal: remove obsolete hot-standby testing infrastructure

From
Alexander Lakhin
Date:
04.01.2022 18:33, Tom Lane wrote:
> Alexander Lakhin <exclusion@gmail.com> writes:
>> It's hardly that important, but we (Postgres Pro) run this test
>> regularly to check for primary-standby compatibility. It's useful when
>> checking binary packages from different minor versions. For example, we
>> setup postgresql-14.0 and postgresql-14.1 aside (renaming one
>> installation' directory and changing it's port) and perform the test.
>> What've found with it was e.g. incompatibility due to linkage of
>> different libicu versions (that was PgPro-only issue). I don't remember
>> whether we found something related to PostgreSQL itself, but we
>> definitely use this test and I'm not sure how to replace it in our setup
>> with a TAP test. On the other hand, testing binaries is not accustomed
>> in the community yet, so when such testing will be adopted, probably a
>> brand new set of tests should emerge.
> Oh, interesting.  I definitely concur that testing compatibility of
> different builds or minor versions is an important use-case.  And
> I concede that making src/test/recovery do it would be tricky and
> a bit out-of-scope.  But having said that, the hs_standby_* scripts
> seem like a poor fit for the job too.  AFAICS they don't really
> test any user data type except integer (so I'm surprised that they
> located an ICU incompatibility for you); and they spend a lot of
> effort on stuff that I doubt is relevant because it *is* covered
> by the TAP tests.
An ICU incompatibility was detected due to our invention [1] "default
collation" that is checked upon connection (before any query processing):
--- C:/tmp/.../src/test/regress/expected/hs_standby_check.out   
2021-10-14 04:07:38.000000000 +0200
+++ C:/tmp/.../src/test/regress/results/hs_standby_check.out   
2021-10-14 06:06:12.004043500 +0200
@@ -1,3 +1,6 @@
+WARNING:  collation "default" has version mismatch

+DETAIL:  The collation in the database was created using version
153.64, but the operating system provides version 153.14.

+HINT:  Check all objects affected by this collation and run ALTER
COLLATION pg_catalog."default" REFRESH VERSION

 --
 -- Hot Standby tests
 --

I admit that we decided to use this test mainly because it exists and
described in the documentation, not because it seemed very useful. It's
usage increased test coverage without a doubt, as it requires a rather
non-trivial setup (similar setups performed by TAP tests, but not with
pre-packaged binaries).
> If I were trying to test that topic using available spare parts,
> what I'd do is run the regular regression tests on the primary
> and see if the standby could track it.  Maybe pg_dump from both
> servers afterwards and see if the results match, a la the pg_upgrade
> test.  Bonus points for a script that could run some other pg_regress
> suite such as one of the contrib modules, because then you could
> check compatibility of those too.
Thanks for the idea! We certainly will implement something like that
when we start testing packages for v15. We've already learned to compare
dumps before/after minor upgrade, so we could reuse that logic for this
test too.
> I'm happy to keep the hs_standby_* scripts if there's a live use-case
> for them; but I don't see what they're doing for you that wouldn't be
> done better by other pg_regress suites.
Yes, I will not miss the test in case you will remove it. I just wanted
to mention that we use(d) it in our testing more or less successfully.

[1]
https://www.postgresql.org/message-id/37A534BE-CBF7-467C-B096-0AAD25091A9F%40yandex-team.ru

Best regards,
Alexander



Re: Proposal: remove obsolete hot-standby testing infrastructure

From
Peter Eisentraut
Date:
On 03.01.22 22:50, Tom Lane wrote:
> The attached proposed patch removes some ancient infrastructure for
> manually testing hot standby.  I doubt anyone has used this in years,
> because AFAICS there is nothing here that's not done better by the
> src/test/recovery TAP tests.  (Or if there is, we ought to migrate
> it into the TAP tests.)

I looked into this some time ago and concluded that this test contains a 
significant amount of testing that isn't obviously done anywhere else. 
I don't have the notes anymore, and surely some things have progressed 
since, but I wouldn't just throw the old test suite away without 
actually checking.



Re: Proposal: remove obsolete hot-standby testing infrastructure

From
Tom Lane
Date:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> On 03.01.22 22:50, Tom Lane wrote:
>> The attached proposed patch removes some ancient infrastructure for
>> manually testing hot standby.

> I looked into this some time ago and concluded that this test contains a 
> significant amount of testing that isn't obviously done anywhere else. 
> I don't have the notes anymore, and surely some things have progressed 
> since, but I wouldn't just throw the old test suite away without 
> actually checking.

Fair enough ... so I looked, and there's not much at all that
I'm worried about.

hs_standby_allowed:
This is basically checking that the standby can see data from
the primary, which we surely have covered.  Although it does
also cover propagation of nextval, which AFAICS is not tested
in src/test/recovery, so perhaps that's worth troubling over.

There are also some checks that particular commands are allowed
on the standby, which seem to me to be not too helpful;
see also comments on the next file.

hs_standby_disallowed:
Inverse of the above: check that some commands are disallowed.
We check some of these in 001_stream_rep.pl, and given the current
code structure in utility.c (ClassifyUtilityCommandAsReadOnly etc),
I do not see much point in adding more test cases of the same sort.
The only likely new bug in that area would be misclassification of
some new command, and no amount of testing of existing cases will
catch that.

There are also tests that particular functions are disallowed, which
isn't something that goes through ClassifyUtilityCommandAsReadOnly.
Nonetheless, adding more test cases here wouldn't help catch future
oversights of that type, so I remain unexcited.

hs_standby_functions:
Mostly also checking that things are disallowed.  There's also
a test of pg_cancel_backend, which is cute but probably suffers
from timing instability (i.e., delayed arrival of the signal
might change the output).  Moreover, pg_cancel_backend is already
covered in the isolation tests, and I see no reason to think
it'd operate differently on a standby.

hs_standby_check:
Checks pg_is_in_recovery(), which is checked far more thoroughly
by pg_ctl/t/003_promote.pl.

hs_primary_extremes:
Checks that we can cope with deep subtransaction nesting.
Maybe this is worth preserving, but I sort of doubt it ---
the standby doesn't even see the nesting does it?
Also checks that the standby can cope with 257 exclusive
locks at once, which corresponds to no interesting limit
that I know of.


So basically, I'd be inclined to add a couple of tests of
sequence-update propagation to src/test/recovery and
call it good.

            regards, tom lane