From: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Subject: Re: SimpleLruTruncate() mutual exclusion
Date: Wed, 31 Jul 2019 23:51:17 -0700
Message-ID: <20190801065117.GA2355684@rfd.leadboat.com> (raw)
In-Reply-To: <29566.1564419497@sss.pgh.pa.us>
References: <20190218073103.GA1434723@rfd.leadboat.com>
<20190628170628.GA1238361@rfd.leadboat.com>
<29566.1564419497@sss.pgh.pa.us>
On Mon, Jul 29, 2019 at 12:58:17PM -0400, Tom Lane wrote:
> > On Sun, Feb 17, 2019 at 11:31:03PM -0800, Noah Misch wrote:
> >>> b. Arrange so only one backend runs vac_truncate_clog() at a time. Other than
> >>> AsyncCtl, every SLRU truncation appears in vac_truncate_clog(), in a
> >>> checkpoint, or in the startup process. Hence, also arrange for only one
> >>> backend to call SimpleLruTruncate(AsyncCtl) at a time.
>
> >> More specifically, restrict vac_update_datfrozenxid() to one backend per
> >> database, and restrict vac_truncate_clog() and asyncQueueAdvanceTail() to one
> >> backend per cluster. This, attached, was rather straightforward.
> I'm pretty sure it was intentional that multiple backends
> could run truncation directory scans concurrently, and I don't really
> want to give that up if possible.
I want to avoid a design that requires painstaking concurrency analysis. Such
analysis is worth it for heap_update(), but vac_truncate_clog() isn't enough
of a hot path. If there's a way to make vac_truncate_clog() easy to analyze
and still somewhat concurrent, fair.
> Also, if I understand the data-loss hazard properly, it's what you
> said in the other thread: the latest_page_number could advance after
> we make our decision about what to truncate, and then maybe we could
> truncate newly-added data. We surely don't want to lock out the
> operations that can advance last_page_number, so how does serializing
> vac_truncate_clog help?
>
> I wonder whether what we need to do is add some state in shared
> memory saying "it is only safe to create pages before here", and
> make SimpleLruZeroPage or its callers check that. The truncation
> logic would advance that value, but only after completing a scan.
> (Oh ..., hmm, maybe the point of serializing truncations is to
> ensure that we know that we can safely advance that?)
vac_truncate_clog() already records "it is only safe to create pages before
here" in ShmemVariableCache->xidWrapLimit, which it updates after any unlinks.
The trouble comes when two vac_truncate_clog() run in parallel and you get a
sequence of events like this:
vac_truncate_clog() instance 1 starts, considers segment ABCD eligible to unlink
vac_truncate_clog() instance 2 starts, considers segment ABCD eligible to unlink
vac_truncate_clog() instance 1 unlinks segment ABCD
vac_truncate_clog() instance 1 calls SetTransactionIdLimit()
vac_truncate_clog() instance 1 finishes
some backend calls SimpleLruZeroPage(), creating segment ABCD
vac_truncate_clog() instance 2 unlinks segment ABCD
Serializing vac_truncate_clog() fixes that.
> Can you post whatever script you used to detect/reproduce the problem
> in the first place?
I've attached it, but it's pretty hackish. Apply the patch on commit 7170268,
make sure your --bindir is in $PATH, copy "conf-test-pg" to your home
directory, and run "make trunc-check". This incorporates xid-burn
acceleration code that Jeff Janes shared with -hackers at some point.
> PS: also, re-reading this code, it's worrisome that we are not checking
> for failure of the unlink calls. I think the idea was that it didn't
> really matter because if we did re-use an existing file we'd just
> re-zero it; hence failing the truncate is an overreaction. But probably
> some comments about that are in order.
That's my understanding. We'll zero any page before reusing it. Failing the
whole vac_truncate_clog() (and therefore not advancing the wrap limit) would
do more harm than the bit of wasted disk space. Still, a LOG or WARNING
message would be fine, I think.
Thanks,
nm
diff --git a/GNUmakefile.in b/GNUmakefile.inindex f4e31a7..284fba1 100644--- a/GNUmakefile.in+++ b/GNUmakefile.in@@ -67,6 +67,20 @@ check check-tests installcheck installcheck-parallel installcheck-tests: CHECKPR
check check-tests installcheck installcheck-parallel installcheck-tests: submake-generated-headers
$(MAKE) -C src/test/regress $@
+TEST_PGDATA_DATA=/var/tmp/test-pg+trunc_check:+ make -j20 install PROFILE=-O0+ $(CC) -g -Wall -I`pg_config --includedir` -L`pg_config --libdir` -Wl,-R`pg_config --libdir` trunc-clog-concurrency.c -lpq -o trunc-clog-concurrency-tester+ pg_ctl -m fast -w stop ||:+ rm -rf $(TEST_PGDATA_DATA)+ initdb -N $(TEST_PGDATA_DATA)+ cp -p ~/conf-test-pg $(TEST_PGDATA_DATA)/postgresql.conf+ echo "default_transaction_isolation = 'read committed'" >>$(TEST_PGDATA_DATA)/postgresql.conf+ pg_ctl -w start+ createdb+ psql -Xc 'alter database template0 allow_connections on'+ env time ./trunc-clog-concurrency-tester+
$(call recurse,check-world,src/test src/pl src/interfaces/ecpg contrib src/bin,check)
$(call recurse,checkprep, src/test src/pl src/interfaces/ecpg contrib src/bin)
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.cindex aa089d8..7202f34 100644--- a/src/backend/access/transam/clog.c+++ b/src/backend/access/transam/clog.c@@ -42,6 +42,7 @@
#include "pgstat.h"
#include "pg_trace.h"
#include "storage/proc.h"
+#include "utils/fmgrprotos.h"
/*
* Defines for CLOG page sizes. A page is the same BLCKSZ as is used
@@ -917,6 +918,22 @@ TruncateCLOG(TransactionId oldestXact, Oid oldestxid_datoid)
if (!SlruScanDirectory(ClogCtl, SlruScanDirCbReportPresence, &cutoffPage))
return; /* nothing to remove */
+#if 0+ /* FIXME Move sleep duration into a GUC? */+ if (LWLockConditionalAcquire(TruncSleepLock, LW_EXCLUSIVE))+ {+ elog(LOG, "TruncSleepLock taken: sleeping (%d for %u)",+ cutoffPage, oldestXact);+ DirectFunctionCall1(pg_sleep, Float8GetDatum(10.0));+ /* TODO increase time, attach debugger and check caller vars */+ LWLockRelease(TruncSleepLock);+ elog(LOG, "TruncSleepLock done: proceeding");+ }+ else+ elog(LOG, "TruncSleepLock unavailable: proceeding (%d for %u)",+ cutoffPage, oldestXact);+#endif+
/*
* Advance oldestClogXid before truncating clog, so concurrent xact status
* lookups can ensure they don't attempt to access truncated-away clog.
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.cindex 3623352..ab28eda 100644--- a/src/backend/access/transam/slru.c+++ b/src/backend/access/transam/slru.c@@ -57,6 +57,7 @@
#include "pgstat.h"
#include "storage/fd.h"
#include "storage/shmem.h"
+#include "utils/fmgrprotos.h"
#include "miscadmin.h"
@@ -1171,11 +1172,6 @@ SimpleLruTruncate(SlruCtl ctl, int cutoffPage)
int slotno;
/*
- * The cutoff point is the start of the segment containing cutoffPage.- */- cutoffPage -= cutoffPage % SLRU_PAGES_PER_SEGMENT;-- /*
* Scan shared memory and remove any pages preceding the cutoff page, to
* ensure we won't rewrite them later. (Since this is normally called in
* or just after a checkpoint, any dirty pages should have been flushed
@@ -1191,6 +1187,21 @@ restart:;
* have already wrapped around, and proceeding with the truncation would
* risk removing the current segment.
*/
+ if (shared->ControlLock == CLogControlLock)+ {+ int test = shared->latest_page_number;+ elog(WARNING, "important safety check: %d latest < %d cutoff?",+ shared->latest_page_number, cutoffPage);+ while (test < 130000)+ {+ if (ctl->PagePrecedes(test, cutoffPage))+ {+ elog(WARNING, "safety check would trip at %d", test);+ break;+ }+ test++;+ }+ }
if (ctl->PagePrecedes(shared->latest_page_number, cutoffPage))
{
LWLockRelease(shared->ControlLock);
@@ -1233,6 +1244,29 @@ restart:;
LWLockRelease(shared->ControlLock);
+#if 1+ if (shared->ControlLock == CLogControlLock)+ {+ /* FIXME Move sleep duration into a GUC? */+ if (LWLockConditionalAcquire(TruncSleepLock, LW_EXCLUSIVE))+ {+ elog(LOG, "TruncSleepLock taken: sleeping (%d)",+ cutoffPage);+ DirectFunctionCall1(pg_sleep, Float8GetDatum(10.0));+ /* TODO increase time, attach debugger and check caller vars */+ LWLockRelease(TruncSleepLock);+ elog(LOG, "TruncSleepLock done: proceeding");+ }+ else+ elog(LOG, "TruncSleepLock unavailable: proceeding (%d)",+ cutoffPage);+ }+#endif++ if (ctl->PagePrecedes(shared->latest_page_number, cutoffPage))+ ereport(LOG,+ (errmsg("too late, but apparent wraparound")));+
/* Now we can remove the old segment(s) */
(void) SlruScanDirectory(ctl, SlruScanDirCbDeleteCutoff, &cutoffPage);
}
@@ -1320,11 +1354,10 @@ restart:
bool
SlruScanDirCbReportPresence(SlruCtl ctl, char *filename, int segpage, void *data)
{
+ int seg_last_page = segpage + SLRU_PAGES_PER_SEGMENT - 1;
int cutoffPage = *(int *) data;
- cutoffPage -= cutoffPage % SLRU_PAGES_PER_SEGMENT;-- if (ctl->PagePrecedes(segpage, cutoffPage))+ if (ctl->PagePrecedes(seg_last_page, cutoffPage))
return true; /* found one; don't iterate any more */
return false; /* keep going */
@@ -1337,9 +1370,10 @@ SlruScanDirCbReportPresence(SlruCtl ctl, char *filename, int segpage, void *data
static bool
SlruScanDirCbDeleteCutoff(SlruCtl ctl, char *filename, int segpage, void *data)
{
+ int seg_last_page = segpage + SLRU_PAGES_PER_SEGMENT - 1;
int cutoffPage = *(int *) data;
- if (ctl->PagePrecedes(segpage, cutoffPage))+ if (ctl->PagePrecedes(seg_last_page, cutoffPage))
SlruInternalDeleteSegment(ctl, filename);
return false; /* keep going */
diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.cindex fe94fda..37db959 100644--- a/src/backend/access/transam/varsup.c+++ b/src/backend/access/transam/varsup.c@@ -33,6 +33,8 @@
/* pointer to "variable cache" in shared memory (set up by shmem.c) */
VariableCache ShmemVariableCache = NULL;
+int JJ_xid=0;+
/*
* Allocate the next XID for a new transaction or subtransaction.
@@ -168,6 +170,11 @@ GetNewTransactionId(bool isSubXact)
*
* Extend pg_subtrans and pg_commit_ts too.
*/
+ {+ int incr;+ for (incr=0; incr <=JJ_xid; incr++)+ {+ xid = ShmemVariableCache->nextXid;
ExtendCLOG(xid);
ExtendCommitTs(xid);
ExtendSUBTRANS(xid);
@@ -180,6 +187,13 @@ GetNewTransactionId(bool isSubXact)
*/
TransactionIdAdvance(ShmemVariableCache->nextXid);
+ /* If JJ_xid opposes xidStopLimit, the latter wins */+ if (TransactionIdFollowsOrEquals(ShmemVariableCache->nextXid,+ ShmemVariableCache->xidStopLimit))+ break;+ }+ }+
/*
* We must store the new XID into the shared ProcArray before releasing
* XidGenLock. This ensures that every active XID older than
@@ -302,9 +316,7 @@ SetTransactionIdLimit(TransactionId oldest_datfrozenxid, Oid oldest_datoid)
* We'll refuse to continue assigning XIDs in interactive mode once we get
* within 1M transactions of data loss. This leaves lots of room for the
* DBA to fool around fixing things in a standalone backend, while not
- * being significant compared to total XID space. (Note that since- * vacuuming requires one transaction per table cleaned, we had better be- * sure there's lots of XIDs left...)+ * being significant compared to total XID space.
*/
xidStopLimit = xidWrapLimit - 1000000;
if (xidStopLimit < FirstNormalTransactionId)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.cindex a707d4d..fa7ab6a 100644--- a/src/backend/postmaster/postmaster.c+++ b/src/backend/postmaster/postmaster.c@@ -5143,7 +5143,7 @@ sigusr1_handler(SIGNAL_ARGS)
* that by launching another iteration as soon as the current one
* completes.
*/
- start_autovac_launcher = true;+ /* start_autovac_launcher = true; */
}
if (CheckPostmasterSignal(PMSIGNAL_START_AUTOVAC_WORKER) &&
diff --git a/src/backend/storage/lmgr/lwlocknames.txt b/src/backend/storage/lmgr/lwlocknames.txtindex db47843..335521c 100644--- a/src/backend/storage/lmgr/lwlocknames.txt+++ b/src/backend/storage/lmgr/lwlocknames.txt@@ -49,3 +49,4 @@ MultiXactTruncationLock 41
OldSnapshotTimeMapLock 42
LogicalRepWorkerLock 43
CLogTruncationLock 44
+TruncSleepLock 45diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.cindex f81e042..4e59a68 100644--- a/src/backend/utils/misc/guc.c+++ b/src/backend/utils/misc/guc.c@@ -116,6 +116,7 @@
/* XXX these should appear in other modules' header files */
extern bool Log_disconnections;
extern int CommitDelay;
+extern int JJ_xid;
extern int CommitSiblings;
extern char *default_tablespace;
extern char *temp_tablespaces;
@@ -2617,6 +2618,15 @@ static struct config_int ConfigureNamesInt[] =
},
{
+ {"JJ_xid", PGC_USERSET, WAL_SETTINGS,+ gettext_noop("Skip this many xid every time we acquire one"),+ NULL+ },+ &JJ_xid,+ 0, 0, 1000000, NULL, NULL+ },++ {
{"commit_siblings", PGC_USERSET, WAL_SETTINGS,
gettext_noop("Sets the minimum concurrent open transactions before performing "
"commit_delay."),
diff --git a/trunc-clog-concurrency.c b/trunc-clog-concurrency.cnew file mode 100644index 0000000..2c35dd9--- /dev/null+++ b/trunc-clog-concurrency.c@@ -0,0 +1,178 @@+#include <libpq-fe.h>++#include <stdbool.h>+#include <stdio.h>+#include <stdlib.h>+#include <string.h>+#include <sys/wait.h>+#include <unistd.h>++static void+report_query_failure(const char *query, PGresult *res)+{+ fprintf(stderr, "query \"%s\" failed unexpectedly: %s",+ query, PQresultErrorMessage(res));+}++static void+safe_query(PGconn *conn, const char *query)+{+ PGresult *res;++ res = PQexec(conn, query);+ if (PQresultStatus(res) != PGRES_TUPLES_OK &&+ PQresultStatus(res) != PGRES_COMMAND_OK)+ {+ report_query_failure(query, res);+ exit(1);+ }+ PQclear(res);+}++static int+is_stop_limit(PGresult *res)+{+ return PQresultStatus(res) == PGRES_FATAL_ERROR+ && strcmp(PQresultErrorField(res, PG_DIAG_SQLSTATE), "54000") == 0;+}++int+main(int argc, char **argv)+{+ bool reached_stop_limit = false;+ PGconn *mutate_conn, *hold1_conn, *hold2_conn, *burn_conn;+ PGresult *res;+ int n_burns = 0, n_inserts = 0;++ if (argc != 1)+ {+ fputs("Usage: trunc-clog-concurrency\n", stderr);+ return 1;+ }++ mutate_conn = PQconnectdb("");+ if (PQstatus(mutate_conn) != CONNECTION_OK)+ {+ fprintf(stderr, "PGconnectdb failed: %s", PQerrorMessage(mutate_conn));+ return 1;+ }++ hold1_conn = PQconnectdb("");+ if (PQstatus(hold1_conn) != CONNECTION_OK)+ {+ fprintf(stderr, "PGconnectdb failed: %s", PQerrorMessage(hold1_conn));+ return 1;+ }++ hold2_conn = PQconnectdb("");+ if (PQstatus(hold2_conn) != CONNECTION_OK)+ {+ fprintf(stderr, "PGconnectdb failed: %s", PQerrorMessage(hold2_conn));+ return 1;+ }++ burn_conn = PQconnectdb("options=--JJ_xid=1000000");+ if (PQstatus(burn_conn) != CONNECTION_OK)+ {+ fprintf(stderr, "PGconnectdb failed: %s", PQerrorMessage(burn_conn));+ return 1;+ }++ /* Start a transaction having an xid. */+ safe_query(mutate_conn, "BEGIN;");+ safe_query(mutate_conn, "DROP TABLE IF EXISTS trunc_clog_concurrency;");+ safe_query(mutate_conn, "CREATE TABLE trunc_clog_concurrency ();");++ /* Burn the entire XID space. */+ while (!reached_stop_limit)+ {+ const char query[] = "SELECT txid_current();";+ res = PQexec(burn_conn, query);+ if (PQresultStatus(res) == PGRES_TUPLES_OK)+ {+ ++n_burns;+ if (n_burns == 2)+ {+ safe_query(hold1_conn, "BEGIN ISOLATION LEVEL READ COMMITTED;");+ safe_query(hold1_conn, "CREATE TABLE trunc_clog_concurrency_hold1 ();");+ }+ if (n_burns == 10)+ {+ safe_query(hold2_conn, "BEGIN ISOLATION LEVEL READ COMMITTED;");+ safe_query(hold2_conn, "CREATE TABLE trunc_clog_concurrency_hold2 ();");+ system("psql -Xc 'select state, backend_xid, backend_xmin, query from pg_stat_activity'");+ system("psql -Xc 'select datname,datallowconn,datfrozenxid from pg_database'");+ }+ /* keep burning */;+ }+ else if (is_stop_limit(res))+ {+ reached_stop_limit = true;+ fprintf(stderr, "reached stop limit: %s",+ PQresultErrorMessage(res));+ }+ else+ {+ reached_stop_limit = true; /* FIXME not really */+ report_query_failure(query, res);+ }+ PQclear(res);+ }++ /* Finish the first transaction. xmin raises from start to start+2M. */+ safe_query(mutate_conn, "COMMIT;");++ /* Raise datfrozenxid of all but template1 to start+2M. No truncation. */+ system("for d in postgres template0 test; do vacuumdb -F $d; done; "+ "echo -n 'DONE1 '; date");+ /* Raise xmin to start+10M */+ safe_query(hold1_conn, "COMMIT;");+ /* Sleep on lock before truncating to start+2M. */+ system("(vacuumdb -F template1; echo -n 'DONE2 '; date) &");+ usleep(4000*1000); /* 4s */++ /* Truncate to start+10M. */+ system("(vacuumdb -aF; echo -n 'DONE3 '; date)");+ system("psql -Xc 'select state, backend_xid, backend_xmin, query from pg_stat_activity'");+ system("psql -Xc 'select datname,datallowconn,datfrozenxid from pg_database'");++ /*+ * We want to burn at least 1M xids (the amount protected by xidStopLimit)+ * but not more than 200M (autovacuum_freeze_max_age default) to avoid a+ * second set of VACUUMs.+ */+ while (n_inserts < 150)+ {+ const char query[] =+ "INSERT INTO trunc_clog_concurrency DEFAULT VALUES";+ res = PQexec(burn_conn, query);+ if (PQresultStatus(res) == PGRES_COMMAND_OK)+ {+ n_inserts++;+ fprintf(stderr, "insert %d ", n_inserts);+ system("date >&2");+ }+ else if (is_stop_limit(res))+ {+ fprintf(stderr, "reached stop limit: %s",+ PQresultErrorMessage(res));+ break;+ }+ else+ {+ report_query_failure(query, res);+ return 1;+ }+ PQclear(res);+ }++ system("psql -Xc 'select state, backend_xid, backend_xmin, query from pg_stat_activity'");+ system("psql -Xc 'select datname,datallowconn,datfrozenxid from pg_database'");++ PQfinish(mutate_conn);+ PQfinish(hold1_conn);+ PQfinish(hold2_conn);+ PQfinish(burn_conn);++ return 0;+}
## postgresql.conf for testing; pgdev_initdb copies it into the data directory
## after each initdb. This is intended to work on 8.4 and later.
max_connections = 40
shared_buffers = 24MB
wal_buffers = 16MB
autovacuum = off
datestyle = 'iso, mdy'
deadlock_timeout = 20ms
default_text_search_config = 'pg_catalog.english'
fsync = off
full_page_writes = off
listen_addresses = ''
default_transaction_isolation = serializable
lc_messages = 'en_US.UTF-8'
lc_monetary = 'en_US.UTF-8'
lc_numeric = 'en_US.UTF-8'
lc_time = 'en_US.UTF-8'
log_directory = '..'
log_filename = 'pgdev.log'
log_line_prefix = '%p %m '
log_min_messages = debug1
log_statement = all
log_connections = on
log_disconnections = on
logging_collector = on
# I'd leave this on, but wal_level=minimal is nicer for speed.
#wal_level = hot_standby
#max_wal_senders = 5
Attachments:
[text/plain] repro-vac_truncate_clog-race-v0.patch (13.9K, ../20190801065117.GA2355684@rfd.leadboat.com/2-repro-vac_truncate_clog-race-v0.patch)
download | inline diff:diff --git a/GNUmakefile.in b/GNUmakefile.inindex f4e31a7..284fba1 100644--- a/GNUmakefile.in+++ b/GNUmakefile.in@@ -67,6 +67,20 @@ check check-tests installcheck installcheck-parallel installcheck-tests: CHECKPR
check check-tests installcheck installcheck-parallel installcheck-tests: submake-generated-headers
$(MAKE) -C src/test/regress $@
+TEST_PGDATA_DATA=/var/tmp/test-pg+trunc_check:+ make -j20 install PROFILE=-O0+ $(CC) -g -Wall -I`pg_config --includedir` -L`pg_config --libdir` -Wl,-R`pg_config --libdir` trunc-clog-concurrency.c -lpq -o trunc-clog-concurrency-tester+ pg_ctl -m fast -w stop ||:+ rm -rf $(TEST_PGDATA_DATA)+ initdb -N $(TEST_PGDATA_DATA)+ cp -p ~/conf-test-pg $(TEST_PGDATA_DATA)/postgresql.conf+ echo "default_transaction_isolation = 'read committed'" >>$(TEST_PGDATA_DATA)/postgresql.conf+ pg_ctl -w start+ createdb+ psql -Xc 'alter database template0 allow_connections on'+ env time ./trunc-clog-concurrency-tester+
$(call recurse,check-world,src/test src/pl src/interfaces/ecpg contrib src/bin,check)
$(call recurse,checkprep, src/test src/pl src/interfaces/ecpg contrib src/bin)
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.cindex aa089d8..7202f34 100644--- a/src/backend/access/transam/clog.c+++ b/src/backend/access/transam/clog.c@@ -42,6 +42,7 @@
#include "pgstat.h"
#include "pg_trace.h"
#include "storage/proc.h"
+#include "utils/fmgrprotos.h"
/*
* Defines for CLOG page sizes. A page is the same BLCKSZ as is used
@@ -917,6 +918,22 @@ TruncateCLOG(TransactionId oldestXact, Oid oldestxid_datoid)
if (!SlruScanDirectory(ClogCtl, SlruScanDirCbReportPresence, &cutoffPage))
return; /* nothing to remove */
+#if 0+ /* FIXME Move sleep duration into a GUC? */+ if (LWLockConditionalAcquire(TruncSleepLock, LW_EXCLUSIVE))+ {+ elog(LOG, "TruncSleepLock taken: sleeping (%d for %u)",+ cutoffPage, oldestXact);+ DirectFunctionCall1(pg_sleep, Float8GetDatum(10.0));+ /* TODO increase time, attach debugger and check caller vars */+ LWLockRelease(TruncSleepLock);+ elog(LOG, "TruncSleepLock done: proceeding");+ }+ else+ elog(LOG, "TruncSleepLock unavailable: proceeding (%d for %u)",+ cutoffPage, oldestXact);+#endif+
/*
* Advance oldestClogXid before truncating clog, so concurrent xact status
* lookups can ensure they don't attempt to access truncated-away clog.
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.cindex 3623352..ab28eda 100644--- a/src/backend/access/transam/slru.c+++ b/src/backend/access/transam/slru.c@@ -57,6 +57,7 @@
#include "pgstat.h"
#include "storage/fd.h"
#include "storage/shmem.h"
+#include "utils/fmgrprotos.h"
#include "miscadmin.h"
@@ -1171,11 +1172,6 @@ SimpleLruTruncate(SlruCtl ctl, int cutoffPage)
int slotno;
/*
- * The cutoff point is the start of the segment containing cutoffPage.- */- cutoffPage -= cutoffPage % SLRU_PAGES_PER_SEGMENT;-- /*
* Scan shared memory and remove any pages preceding the cutoff page, to
* ensure we won't rewrite them later. (Since this is normally called in
* or just after a checkpoint, any dirty pages should have been flushed
@@ -1191,6 +1187,21 @@ restart:;
* have already wrapped around, and proceeding with the truncation would
* risk removing the current segment.
*/
+ if (shared->ControlLock == CLogControlLock)+ {+ int test = shared->latest_page_number;+ elog(WARNING, "important safety check: %d latest < %d cutoff?",+ shared->latest_page_number, cutoffPage);+ while (test < 130000)+ {+ if (ctl->PagePrecedes(test, cutoffPage))+ {+ elog(WARNING, "safety check would trip at %d", test);+ break;+ }+ test++;+ }+ }
if (ctl->PagePrecedes(shared->latest_page_number, cutoffPage))
{
LWLockRelease(shared->ControlLock);
@@ -1233,6 +1244,29 @@ restart:;
LWLockRelease(shared->ControlLock);
+#if 1+ if (shared->ControlLock == CLogControlLock)+ {+ /* FIXME Move sleep duration into a GUC? */+ if (LWLockConditionalAcquire(TruncSleepLock, LW_EXCLUSIVE))+ {+ elog(LOG, "TruncSleepLock taken: sleeping (%d)",+ cutoffPage);+ DirectFunctionCall1(pg_sleep, Float8GetDatum(10.0));+ /* TODO increase time, attach debugger and check caller vars */+ LWLockRelease(TruncSleepLock);+ elog(LOG, "TruncSleepLock done: proceeding");+ }+ else+ elog(LOG, "TruncSleepLock unavailable: proceeding (%d)",+ cutoffPage);+ }+#endif++ if (ctl->PagePrecedes(shared->latest_page_number, cutoffPage))+ ereport(LOG,+ (errmsg("too late, but apparent wraparound")));+
/* Now we can remove the old segment(s) */
(void) SlruScanDirectory(ctl, SlruScanDirCbDeleteCutoff, &cutoffPage);
}
@@ -1320,11 +1354,10 @@ restart:
bool
SlruScanDirCbReportPresence(SlruCtl ctl, char *filename, int segpage, void *data)
{
+ int seg_last_page = segpage + SLRU_PAGES_PER_SEGMENT - 1;
int cutoffPage = *(int *) data;
- cutoffPage -= cutoffPage % SLRU_PAGES_PER_SEGMENT;-- if (ctl->PagePrecedes(segpage, cutoffPage))+ if (ctl->PagePrecedes(seg_last_page, cutoffPage))
return true; /* found one; don't iterate any more */
return false; /* keep going */
@@ -1337,9 +1370,10 @@ SlruScanDirCbReportPresence(SlruCtl ctl, char *filename, int segpage, void *data
static bool
SlruScanDirCbDeleteCutoff(SlruCtl ctl, char *filename, int segpage, void *data)
{
+ int seg_last_page = segpage + SLRU_PAGES_PER_SEGMENT - 1;
int cutoffPage = *(int *) data;
- if (ctl->PagePrecedes(segpage, cutoffPage))+ if (ctl->PagePrecedes(seg_last_page, cutoffPage))
SlruInternalDeleteSegment(ctl, filename);
return false; /* keep going */
diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.cindex fe94fda..37db959 100644--- a/src/backend/access/transam/varsup.c+++ b/src/backend/access/transam/varsup.c@@ -33,6 +33,8 @@
/* pointer to "variable cache" in shared memory (set up by shmem.c) */
VariableCache ShmemVariableCache = NULL;
+int JJ_xid=0;+
/*
* Allocate the next XID for a new transaction or subtransaction.
@@ -168,6 +170,11 @@ GetNewTransactionId(bool isSubXact)
*
* Extend pg_subtrans and pg_commit_ts too.
*/
+ {+ int incr;+ for (incr=0; incr <=JJ_xid; incr++)+ {+ xid = ShmemVariableCache->nextXid;
ExtendCLOG(xid);
ExtendCommitTs(xid);
ExtendSUBTRANS(xid);
@@ -180,6 +187,13 @@ GetNewTransactionId(bool isSubXact)
*/
TransactionIdAdvance(ShmemVariableCache->nextXid);
+ /* If JJ_xid opposes xidStopLimit, the latter wins */+ if (TransactionIdFollowsOrEquals(ShmemVariableCache->nextXid,+ ShmemVariableCache->xidStopLimit))+ break;+ }+ }+
/*
* We must store the new XID into the shared ProcArray before releasing
* XidGenLock. This ensures that every active XID older than
@@ -302,9 +316,7 @@ SetTransactionIdLimit(TransactionId oldest_datfrozenxid, Oid oldest_datoid)
* We'll refuse to continue assigning XIDs in interactive mode once we get
* within 1M transactions of data loss. This leaves lots of room for the
* DBA to fool around fixing things in a standalone backend, while not
- * being significant compared to total XID space. (Note that since- * vacuuming requires one transaction per table cleaned, we had better be- * sure there's lots of XIDs left...)+ * being significant compared to total XID space.
*/
xidStopLimit = xidWrapLimit - 1000000;
if (xidStopLimit < FirstNormalTransactionId)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.cindex a707d4d..fa7ab6a 100644--- a/src/backend/postmaster/postmaster.c+++ b/src/backend/postmaster/postmaster.c@@ -5143,7 +5143,7 @@ sigusr1_handler(SIGNAL_ARGS)
* that by launching another iteration as soon as the current one
* completes.
*/
- start_autovac_launcher = true;+ /* start_autovac_launcher = true; */
}
if (CheckPostmasterSignal(PMSIGNAL_START_AUTOVAC_WORKER) &&
diff --git a/src/backend/storage/lmgr/lwlocknames.txt b/src/backend/storage/lmgr/lwlocknames.txtindex db47843..335521c 100644--- a/src/backend/storage/lmgr/lwlocknames.txt+++ b/src/backend/storage/lmgr/lwlocknames.txt@@ -49,3 +49,4 @@ MultiXactTruncationLock 41
OldSnapshotTimeMapLock 42
LogicalRepWorkerLock 43
CLogTruncationLock 44
+TruncSleepLock 45diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.cindex f81e042..4e59a68 100644--- a/src/backend/utils/misc/guc.c+++ b/src/backend/utils/misc/guc.c@@ -116,6 +116,7 @@
/* XXX these should appear in other modules' header files */
extern bool Log_disconnections;
extern int CommitDelay;
+extern int JJ_xid;
extern int CommitSiblings;
extern char *default_tablespace;
extern char *temp_tablespaces;
@@ -2617,6 +2618,15 @@ static struct config_int ConfigureNamesInt[] =
},
{
+ {"JJ_xid", PGC_USERSET, WAL_SETTINGS,+ gettext_noop("Skip this many xid every time we acquire one"),+ NULL+ },+ &JJ_xid,+ 0, 0, 1000000, NULL, NULL+ },++ {
{"commit_siblings", PGC_USERSET, WAL_SETTINGS,
gettext_noop("Sets the minimum concurrent open transactions before performing "
"commit_delay."),
diff --git a/trunc-clog-concurrency.c b/trunc-clog-concurrency.cnew file mode 100644index 0000000..2c35dd9--- /dev/null+++ b/trunc-clog-concurrency.c@@ -0,0 +1,178 @@+#include <libpq-fe.h>++#include <stdbool.h>+#include <stdio.h>+#include <stdlib.h>+#include <string.h>+#include <sys/wait.h>+#include <unistd.h>++static void+report_query_failure(const char *query, PGresult *res)+{+ fprintf(stderr, "query \"%s\" failed unexpectedly: %s",+ query, PQresultErrorMessage(res));+}++static void+safe_query(PGconn *conn, const char *query)+{+ PGresult *res;++ res = PQexec(conn, query);+ if (PQresultStatus(res) != PGRES_TUPLES_OK &&+ PQresultStatus(res) != PGRES_COMMAND_OK)+ {+ report_query_failure(query, res);+ exit(1);+ }+ PQclear(res);+}++static int+is_stop_limit(PGresult *res)+{+ return PQresultStatus(res) == PGRES_FATAL_ERROR+ && strcmp(PQresultErrorField(res, PG_DIAG_SQLSTATE), "54000") == 0;+}++int+main(int argc, char **argv)+{+ bool reached_stop_limit = false;+ PGconn *mutate_conn, *hold1_conn, *hold2_conn, *burn_conn;+ PGresult *res;+ int n_burns = 0, n_inserts = 0;++ if (argc != 1)+ {+ fputs("Usage: trunc-clog-concurrency\n", stderr);+ return 1;+ }++ mutate_conn = PQconnectdb("");+ if (PQstatus(mutate_conn) != CONNECTION_OK)+ {+ fprintf(stderr, "PGconnectdb failed: %s", PQerrorMessage(mutate_conn));+ return 1;+ }++ hold1_conn = PQconnectdb("");+ if (PQstatus(hold1_conn) != CONNECTION_OK)+ {+ fprintf(stderr, "PGconnectdb failed: %s", PQerrorMessage(hold1_conn));+ return 1;+ }++ hold2_conn = PQconnectdb("");+ if (PQstatus(hold2_conn) != CONNECTION_OK)+ {+ fprintf(stderr, "PGconnectdb failed: %s", PQerrorMessage(hold2_conn));+ return 1;+ }++ burn_conn = PQconnectdb("options=--JJ_xid=1000000");+ if (PQstatus(burn_conn) != CONNECTION_OK)+ {+ fprintf(stderr, "PGconnectdb failed: %s", PQerrorMessage(burn_conn));+ return 1;+ }++ /* Start a transaction having an xid. */+ safe_query(mutate_conn, "BEGIN;");+ safe_query(mutate_conn, "DROP TABLE IF EXISTS trunc_clog_concurrency;");+ safe_query(mutate_conn, "CREATE TABLE trunc_clog_concurrency ();");++ /* Burn the entire XID space. */+ while (!reached_stop_limit)+ {+ const char query[] = "SELECT txid_current();";+ res = PQexec(burn_conn, query);+ if (PQresultStatus(res) == PGRES_TUPLES_OK)+ {+ ++n_burns;+ if (n_burns == 2)+ {+ safe_query(hold1_conn, "BEGIN ISOLATION LEVEL READ COMMITTED;");+ safe_query(hold1_conn, "CREATE TABLE trunc_clog_concurrency_hold1 ();");+ }+ if (n_burns == 10)+ {+ safe_query(hold2_conn, "BEGIN ISOLATION LEVEL READ COMMITTED;");+ safe_query(hold2_conn, "CREATE TABLE trunc_clog_concurrency_hold2 ();");+ system("psql -Xc 'select state, backend_xid, backend_xmin, query from pg_stat_activity'");+ system("psql -Xc 'select datname,datallowconn,datfrozenxid from pg_database'");+ }+ /* keep burning */;+ }+ else if (is_stop_limit(res))+ {+ reached_stop_limit = true;+ fprintf(stderr, "reached stop limit: %s",+ PQresultErrorMessage(res));+ }+ else+ {+ reached_stop_limit = true; /* FIXME not really */+ report_query_failure(query, res);+ }+ PQclear(res);+ }++ /* Finish the first transaction. xmin raises from start to start+2M. */+ safe_query(mutate_conn, "COMMIT;");++ /* Raise datfrozenxid of all but template1 to start+2M. No truncation. */+ system("for d in postgres template0 test; do vacuumdb -F $d; done; "+ "echo -n 'DONE1 '; date");+ /* Raise xmin to start+10M */+ safe_query(hold1_conn, "COMMIT;");+ /* Sleep on lock before truncating to start+2M. */+ system("(vacuumdb -F template1; echo -n 'DONE2 '; date) &");+ usleep(4000*1000); /* 4s */++ /* Truncate to start+10M. */+ system("(vacuumdb -aF; echo -n 'DONE3 '; date)");+ system("psql -Xc 'select state, backend_xid, backend_xmin, query from pg_stat_activity'");+ system("psql -Xc 'select datname,datallowconn,datfrozenxid from pg_database'");++ /*+ * We want to burn at least 1M xids (the amount protected by xidStopLimit)+ * but not more than 200M (autovacuum_freeze_max_age default) to avoid a+ * second set of VACUUMs.+ */+ while (n_inserts < 150)+ {+ const char query[] =+ "INSERT INTO trunc_clog_concurrency DEFAULT VALUES";+ res = PQexec(burn_conn, query);+ if (PQresultStatus(res) == PGRES_COMMAND_OK)+ {+ n_inserts++;+ fprintf(stderr, "insert %d ", n_inserts);+ system("date >&2");+ }+ else if (is_stop_limit(res))+ {+ fprintf(stderr, "reached stop limit: %s",+ PQresultErrorMessage(res));+ break;+ }+ else+ {+ report_query_failure(query, res);+ return 1;+ }+ PQclear(res);+ }++ system("psql -Xc 'select state, backend_xid, backend_xmin, query from pg_stat_activity'");+ system("psql -Xc 'select datname,datallowconn,datfrozenxid from pg_database'");++ PQfinish(mutate_conn);+ PQfinish(hold1_conn);+ PQfinish(hold2_conn);+ PQfinish(burn_conn);++ return 0;+}
[text/plain] conf-test-pg (831B, ../20190801065117.GA2355684@rfd.leadboat.com/3-conf-test-pg)
download | inline:
## postgresql.conf for testing; pgdev_initdb copies it into the data directory
## after each initdb. This is intended to work on 8.4 and later.
max_connections = 40
shared_buffers = 24MB
wal_buffers = 16MB
autovacuum = off
datestyle = 'iso, mdy'
deadlock_timeout = 20ms
default_text_search_config = 'pg_catalog.english'
fsync = off
full_page_writes = off
listen_addresses = ''
default_transaction_isolation = serializable
lc_messages = 'en_US.UTF-8'
lc_monetary = 'en_US.UTF-8'
lc_numeric = 'en_US.UTF-8'
lc_time = 'en_US.UTF-8'
log_directory = '..'
log_filename = 'pgdev.log'
log_line_prefix = '%p %m '
log_min_messages = debug1
log_statement = all
log_connections = on
log_disconnections = on
logging_collector = on
# I'd leave this on, but wal_level=minimal is nicer for speed.
#wal_level = hot_standby
#max_wal_senders = 5
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: noah@leadboat.com, tgl@sss.pgh.pa.us
Subject: Re: SimpleLruTruncate() mutual exclusion
In-Reply-To: <20190801065117.GA2355684@rfd.leadboat.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