From: Mark Dilger <mark.dilger@enterprisedb.com>
To: amul sul <sulamul@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: [Patch] ALTER SYSTEM READ ONLY
Date: Tue, 14 Sep 2021 16:04:01 -0700
Message-ID: <9BA3BA57-6B7B-45CB-B8D9-6B5EB0104FFA@enterprisedb.com> (raw)
In-Reply-To: <CAAJ_b97KZzdJsffwRK7w0XU5HnXkcgKgTR69t8cOZztsyXjkQw@mail.gmail.com>
References: <CAAJ_b97KZzdJsffwRK7w0XU5HnXkcgKgTR69t8cOZztsyXjkQw@mail.gmail.com>
> On Jun 16, 2020, at 6:55 AM, amul sul <sulamul@gmail.com> wrote:
>
> (2) if the session is idle, we also need the top-level abort
> record to be written immediately, but can't send an error to the client until the next
> command is issued without losing wire protocol synchronization. For now, we just use
> FATAL to kill the session; maybe this can be improved in the future.
Andres,
I'd like to have a patch that tests the impact of a vacuum running for xid wraparound purposes, blocked on a pinned page held by the cursor, when another session disables WAL. It would be very interesting to test how the vacuum handles that specific change. I have not figured out the cleanest way to do this, though, as we don't as a project yet have a standard way of setting up xid exhaustion in a regression test, do we? The closest I saw to it was your work in [1], but that doesn't seem to have made much headway recently, and is designed for the TAP testing infrastructure, which isn't useable from inside an isolation test. Do you have a suggestion how best to continue developing out the test infrastructure?
Amul,
The most obvious way to test how your ALTER SYSTEM READ ONLY feature interacts with concurrent sessions is using the isolation tester in src/test/isolation/, but as it stands now, the first permutation that gets a FATAL causes the test to abort and all subsequent permutations to not run. Attached patch v34-0009 fixes that.
Attached patch v34-0010 adds a test of cursors opened FOR UPDATE interacting with a system that is set read-only by a different session. The expected output is worth reviewing to see how this plays out. I don't see anything in there which is obviously wrong, but some of it is a bit clunky. For example, by the time the client sees an error "FATAL: WAL is now prohibited", the system may already have switched back to read-write. Also, it is a bit strange to get one of these errors on an attempted ROLLBACK. Once again, not wrong as such, but clunky.
[1] https://www.postgresql.org/message-id/flat/CAP4vRV5gEHFLB7NwOE6_dyHAeVfkvqF8Z_g5GaCQZNgBAE0Frw%40mai...
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:download all (2 files, .tar.gz)v34-0009-Make-isolationtester-handle-closed-sessions.patch (application/octet-stream, 2.0K) download
From b7c21e95c9a48208f1aec377366b26e40cf9ecb4 Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Tue, 14 Sep 2021 15:18:52 -0700
Subject: [PATCH v34 09/10] Make isolationtester handle closed sessions.
The recent implementation of ALTER SYSTEM READ ONLY should be
testable from src/test/isolation. For that, the isolation tester
needs to reconnect after getting kicked out rather than aborting.
---
src/test/isolation/isolationtester.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.cindex 88594a3cb5..87cf88cbec 100644--- a/src/test/isolation/isolationtester.c+++ b/src/test/isolation/isolationtester.c@@ -85,6 +85,24 @@ disconnect_atexit(void)
PQfinish(conns[i].conn);
}
+static void+restore_connection(PGconn *conn)+{+ if (PQstatus(conn) != CONNECTION_BAD)+ return;++ fprintf(stderr,+ _("The connection to the server was lost. Attempting reset: "));+ PQreset(conn);+ if (PQstatus(conn) == CONNECTION_BAD)+ {+ fprintf(stderr, _("Failed.\n"));+ exit(1);+ }+ else+ fprintf(stderr, _("Succeeded.\n"));+}+
int
main(int argc, char **argv)
{
@@ -890,7 +908,7 @@ try_complete_step(TestSpec *testspec, PermutationStep *pstep, int flags)
{
fprintf(stderr, "PQconsumeInput failed: %s\n",
PQerrorMessage(conn));
- exit(1);+ restore_connection(conn);
}
if (!PQisBusy(conn))
break;
@@ -964,7 +982,7 @@ try_complete_step(TestSpec *testspec, PermutationStep *pstep, int flags)
{
fprintf(stderr, "PQconsumeInput failed: %s\n",
PQerrorMessage(conn));
- exit(1);+ restore_connection(conn);
}
}
@@ -1013,6 +1031,8 @@ try_complete_step(TestSpec *testspec, PermutationStep *pstep, int flags)
printf("%s: %s\n", sev, msg);
else
printf("%s\n", PQresultErrorMessage(res));
++ restore_connection(conn);
}
break;
default:
--
2.21.1 (Apple Git-122.3)
=
v34-0010-Test-ALTER-SYSTEM-READ-ONLY-against-cursors.patch (application/octet-stream, 6.7K) download
From ce3054a97c78325b06e67219fe0be3535b7be272 Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Tue, 14 Sep 2021 15:28:09 -0700
Subject: [PATCH v34 10/10] Test ALTER SYSTEM READ ONLY against cursors.
Add an isolation test which checks the impact of setting the system
read-only upon cursors that use a FOR UPDATE query.
---
.../expected/cursor-prohibit-wal.out | 157 ++++++++++++++++++
src/test/isolation/isolation_schedule | 1 +
.../isolation/specs/cursor-prohibit-wal.spec | 38 +++++
3 files changed, 196 insertions(+)
create mode 100644 src/test/isolation/expected/cursor-prohibit-wal.out
create mode 100644 src/test/isolation/specs/cursor-prohibit-wal.spec
diff --git a/src/test/isolation/expected/cursor-prohibit-wal.out b/src/test/isolation/expected/cursor-prohibit-wal.outnew file mode 100644index 0000000000..6b479143a6--- /dev/null+++ b/src/test/isolation/expected/cursor-prohibit-wal.out@@ -0,0 +1,157 @@+Parsed test spec with 2 sessions++starting permutation: s1a s1b s1c s2a s2b s1d+step s1a: DECLARE rw_cur CURSOR FOR SELECT a FROM tbl WHERE a = 5000 FOR UPDATE;+step s1b: FETCH FORWARD ALL FROM rw_cur;+ a+----+5000+(1 row)++step s1c: CLOSE ALL;+step s2a: SELECT pg_prohibit_wal(true);+pg_prohibit_wal+---------------+ +(1 row)++step s2b: SELECT pg_prohibit_wal(false);+pg_prohibit_wal+---------------+ +(1 row)++PQconsumeInput failed: FATAL: WAL is now prohibited+HINT: Sessions with open write transactions must be terminated.+server closed the connection unexpectedly+ This probably means the server terminated abnormally+ before or while processing the request.++The connection to the server was lost. Attempting reset: Succeeded.+step s1d: ROLLBACK;++starting permutation: s1a s1b s2a s1c s2b s1d+step s1a: DECLARE rw_cur CURSOR FOR SELECT a FROM tbl WHERE a = 5000 FOR UPDATE;+step s1b: FETCH FORWARD ALL FROM rw_cur;+ a+----+5000+(1 row)++step s2a: SELECT pg_prohibit_wal(true);+pg_prohibit_wal+---------------+ +(1 row)++step s1c: CLOSE ALL;+FATAL: WAL is now prohibited+FATAL: WAL is now prohibited+HINT: Sessions with open write transactions must be terminated.+server closed the connection unexpectedly+ This probably means the server terminated abnormally+ before or while processing the request.++The connection to the server was lost. Attempting reset: Succeeded.+step s2b: SELECT pg_prohibit_wal(false);+pg_prohibit_wal+---------------+ +(1 row)++s1: WARNING: there is no transaction in progress+step s1d: ROLLBACK;++starting permutation: s1a s1b s2a s2b s1c s1d+step s1a: DECLARE rw_cur CURSOR FOR SELECT a FROM tbl WHERE a = 5000 FOR UPDATE;+step s1b: FETCH FORWARD ALL FROM rw_cur;+ a+----+5000+(1 row)++step s2a: SELECT pg_prohibit_wal(true);+pg_prohibit_wal+---------------+ +(1 row)++step s2b: SELECT pg_prohibit_wal(false);+pg_prohibit_wal+---------------+ +(1 row)++PQconsumeInput failed: FATAL: WAL is now prohibited+HINT: Sessions with open write transactions must be terminated.+server closed the connection unexpectedly+ This probably means the server terminated abnormally+ before or while processing the request.++The connection to the server was lost. Attempting reset: Succeeded.+step s1c: CLOSE ALL;+s1: WARNING: there is no transaction in progress+step s1d: ROLLBACK;++starting permutation: s1a s2a s1b s1c s2b s1d+step s1a: DECLARE rw_cur CURSOR FOR SELECT a FROM tbl WHERE a = 5000 FOR UPDATE;+step s2a: SELECT pg_prohibit_wal(true);+pg_prohibit_wal+---------------+ +(1 row)++step s1b: FETCH FORWARD ALL FROM rw_cur;+ERROR: WAL is now prohibited+step s1c: CLOSE ALL;+ERROR: current transaction is aborted, commands ignored until end of transaction block+step s2b: SELECT pg_prohibit_wal(false);+pg_prohibit_wal+---------------+ +(1 row)++step s1d: ROLLBACK;++starting permutation: s1a s2a s1b s2b s1c s1d+step s1a: DECLARE rw_cur CURSOR FOR SELECT a FROM tbl WHERE a = 5000 FOR UPDATE;+step s2a: SELECT pg_prohibit_wal(true);+pg_prohibit_wal+---------------+ +(1 row)++step s1b: FETCH FORWARD ALL FROM rw_cur;+ERROR: WAL is now prohibited+step s2b: SELECT pg_prohibit_wal(false);+pg_prohibit_wal+---------------+ +(1 row)++step s1c: CLOSE ALL;+ERROR: current transaction is aborted, commands ignored until end of transaction block+step s1d: ROLLBACK;++starting permutation: s1a s2a s2b s1b s1c s1d+step s1a: DECLARE rw_cur CURSOR FOR SELECT a FROM tbl WHERE a = 5000 FOR UPDATE;+step s2a: SELECT pg_prohibit_wal(true);+pg_prohibit_wal+---------------+ +(1 row)++step s2b: SELECT pg_prohibit_wal(false);+pg_prohibit_wal+---------------+ +(1 row)++step s1b: FETCH FORWARD ALL FROM rw_cur;+ a+----+5000+(1 row)++step s1c: CLOSE ALL;+step s1d: ROLLBACK;diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_scheduleindex f4c01006fc..fb79e010b0 100644--- a/src/test/isolation/isolation_schedule+++ b/src/test/isolation/isolation_schedule@@ -96,3 +96,4 @@ test: plpgsql-toast
test: truncate-conflict
test: serializable-parallel
test: serializable-parallel-2
+test: cursor-prohibit-waldiff --git a/src/test/isolation/specs/cursor-prohibit-wal.spec b/src/test/isolation/specs/cursor-prohibit-wal.specnew file mode 100644index 0000000000..8f699a4c6e--- /dev/null+++ b/src/test/isolation/specs/cursor-prohibit-wal.spec@@ -0,0 +1,38 @@+# Test for behavior of a cursor over an index scan when the server is read-only+# and the cursor moves over index pages that refer to dead tuples. The key+# point is that the server is read-only, but the scan of the index page will+# want to hint the tuples as dead.+# +# The transaction operating the cursor is intentionally read-only in its+# behavior. If it were to do anything that generated a transaction ID, we+# would expect it to be terminated when the server goes read-only.+#++setup+{+ CREATE TABLE tbl (a integer);+ INSERT INTO tbl SELECT * FROM generate_series(1,10000);+}++teardown+{+ DROP TABLE tbl;+}++session s1+setup { BEGIN; }+step s1a { DECLARE rw_cur CURSOR FOR SELECT a FROM tbl WHERE a = 5000 FOR UPDATE; }+step s1b { FETCH FORWARD ALL FROM rw_cur; }+step s1c { CLOSE ALL; }+step s1d { ROLLBACK; }++session s2+step s2a { SELECT pg_prohibit_wal(true); }+step s2b { SELECT pg_prohibit_wal(false); }++permutation s1a s1b s1c s2a s2b s1d+permutation s1a s1b s2a s1c s2b s1d+permutation s1a s1b s2a s2b s1c s1d+permutation s1a s2a s1b s1c s2b s1d+permutation s1a s2a s1b s2b s1c s1d+permutation s1a s2a s2b s1b s1c s1d--
2.21.1 (Apple Git-122.3)
=
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: mark.dilger@enterprisedb.com, sulamul@gmail.com, andres@anarazel.de
Subject: Re: [Patch] ALTER SYSTEM READ ONLY
In-Reply-To: <9BA3BA57-6B7B-45CB-B8D9-6B5EB0104FFA@enterprisedb.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox