agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feed[PATCH v17 3/3] Fix replay of create database records on standby
18+ messages / 4 participants
[nested] [flat]
* [PATCH v17 3/3] Fix replay of create database records on standby
@ 2020-01-09 20:54 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 18+ messages in thread
From: Alvaro Herrera @ 2020-01-09 20:54 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch adds mechanism similar to invalid page hash table, to track
missing directories during crash recovery. If all the missing
directory references are matched with corresponding drop records at
the end of crash recovery, the standby can safely enter archive
recovery.
Bug identified by Paul Guo.
Authored by Paul Guo, Kyotaro Horiguchi and Asim R P.
---
src/backend/access/transam/xlogrecovery.c | 6 +
src/backend/access/transam/xlogutils.c | 145 ++++++++++++++++++++++
src/backend/commands/dbcommands.c | 56 +++++++++
src/backend/commands/tablespace.c | 6 +
src/include/access/xlogutils.h | 4 +
5 files changed, 217 insertions(+)
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index f9f212680b..97fed1e04d 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -2043,6 +2043,12 @@ CheckRecoveryConsistency(void)
*/
XLogCheckInvalidPages();
+ /*
+ * Check if the XLOG sequence contained any unresolved references to
+ * missing directories.
+ */
+ XLogCheckMissingDirs();
+
reachedConsistency = true;
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 54d5f20734..3f8f7dadac 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -79,6 +79,151 @@ typedef struct xl_invalid_page
static HTAB *invalid_page_tab = NULL;
+/*
+ * If a create database WAL record is being replayed more than once during
+ * crash recovery on a standby, it is possible that either the tablespace
+ * directory or the template database directory is missing. This happens when
+ * the directories are removed by replay of subsequent drop records. Note
+ * that this problem happens only on standby and not on master. On master, a
+ * checkpoint is created at the end of create database operation. On standby,
+ * however, such a strategy (creating restart points during replay) is not
+ * viable because it will slow down WAL replay.
+ *
+ * The alternative is to track references to each missing directory
+ * encountered when performing crash recovery in the following hash table.
+ * Similar to invalid page table above, the expectation is that each missing
+ * directory entry should be matched with a drop database or drop tablespace
+ * WAL record by the end of crash recovery.
+ */
+typedef struct xl_missing_dir_key
+{
+ Oid spcNode;
+ Oid dbNode;
+} xl_missing_dir_key;
+
+typedef struct xl_missing_dir
+{
+ xl_missing_dir_key key;
+ char path[MAXPGPATH];
+} xl_missing_dir;
+
+static HTAB *missing_dir_tab = NULL;
+
+void
+XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path)
+{
+ xl_missing_dir_key key;
+ bool found;
+ xl_missing_dir *entry;
+
+ /*
+ * Database OID may be invalid but tablespace OID must be valid. If
+ * dbNode is InvalidOid, we are logging a missing tablespace directory,
+ * otherwise we are logging a missing database directory.
+ */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ {
+ /* create hash table when first needed */
+ HASHCTL ctl;
+
+ memset(&ctl, 0, sizeof(ctl));
+ ctl.keysize = sizeof(xl_missing_dir_key);
+ ctl.entrysize = sizeof(xl_missing_dir);
+
+ missing_dir_tab = hash_create("XLOG missing directory table",
+ 100,
+ &ctl,
+ HASH_ELEM | HASH_BLOBS);
+ }
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ entry = hash_search(missing_dir_tab, &key, HASH_ENTER, &found);
+
+ if (found)
+ {
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "missing directory %s (tablespace %d) already exists: %s",
+ path, spcNode, entry->path);
+ else
+ elog(DEBUG2, "missing directory %s (tablespace %d database %d) already exists: %s",
+ path, spcNode, dbNode, entry->path);
+ }
+ else
+ {
+ strlcpy(entry->path, path, sizeof(entry->path));
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "logged missing dir %s (tablespace %d)",
+ path, spcNode);
+ else
+ elog(DEBUG2, "logged missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ }
+}
+
+void
+XLogForgetMissingDir(Oid spcNode, Oid dbNode)
+{
+ xl_missing_dir_key key;
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ /* Database OID may be invalid but tablespace OID must be valid. */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ return;
+
+ if (hash_search(missing_dir_tab, &key, HASH_REMOVE, NULL) != NULL)
+ {
+ if (dbNode == InvalidOid)
+ {
+ elog(DEBUG2, "forgot missing dir (tablespace %d)", spcNode);
+ }
+ else
+ {
+ char *path = GetDatabasePath(dbNode, spcNode);
+
+ elog(DEBUG2, "forgot missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ pfree(path);
+ }
+ }
+}
+
+/*
+ * This is called at the end of crash recovery, before entering archive
+ * recovery on a standby. PANIC if the hash table is not empty.
+ */
+void
+XLogCheckMissingDirs(void)
+{
+ HASH_SEQ_STATUS status;
+ xl_missing_dir *hentry;
+ bool foundone = false;
+
+ if (missing_dir_tab == NULL)
+ return; /* nothing to do */
+
+ hash_seq_init(&status, missing_dir_tab);
+
+ while ((hentry = (xl_missing_dir *) hash_seq_search(&status)) != NULL)
+ {
+ elog(WARNING, "missing directory \"%s\" tablespace %d database %d",
+ hentry->path, hentry->key.spcNode, hentry->key.dbNode);
+ foundone = true;
+ }
+
+ if (foundone)
+ elog(PANIC, "WAL contains references to missing directories");
+
+ hash_destroy(missing_dir_tab);
+ missing_dir_tab = NULL;
+}
/* Report a reference to an invalid page */
static void
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index c37e3c9a9a..8994e9da99 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -30,6 +30,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -2382,7 +2383,9 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_rec *xlrec = (xl_dbase_create_rec *) XLogRecGetData(record);
char *src_path;
char *dst_path;
+ char *parent_path;
struct stat st;
+ bool skip = false;
src_path = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
dst_path = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
@@ -2400,6 +2403,55 @@ dbase_redo(XLogReaderState *record)
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
}
+ else if (!reachedConsistency)
+ {
+ /*
+ * It is possible that drop tablespace record appearing later in
+ * the WAL as already been replayed. That means we are replaying
+ * the create database record second time, as part of crash
+ * recovery. In that case, the tablespace directory has already
+ * been removed and the create database operation cannot be
+ * replayed. We should skip the replay but remember the missing
+ * tablespace directory, to be matched with a drop tablespace
+ * record later.
+ */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ if (!(stat(parent_path, &st) == 0 && S_ISDIR(st.st_mode)))
+ {
+ XLogReportMissingDir(xlrec->tablespace_id, InvalidOid, parent_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Target tablespace \"%s\" not found. We "
+ "expect to encounter a WAL record that "
+ "removes this directory before reaching "
+ "consistent state.", parent_path)));
+ }
+ pfree(parent_path);
+ }
+
+ /*
+ * Source directory may be missing. E.g. the template database used
+ * for creating this database may have been dropped, due to reasons
+ * noted above. Moving a database from one tablespace may also be a
+ * partner in the crime.
+ */
+ if (!(stat(src_path, &st) == 0 && S_ISDIR(st.st_mode)) &&
+ !reachedConsistency)
+ {
+ XLogReportMissingDir(xlrec->src_tablespace_id, xlrec->src_db_id, src_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Source database \"%s\" not found. We expect "
+ "to encounter a WAL record that removes this "
+ "directory before reaching consistent state.",
+ src_path)));
+ }
+
+ if (skip)
+ return;
/*
* Force dirty buffers out to disk, to ensure source database is
@@ -2462,6 +2514,10 @@ dbase_redo(XLogReaderState *record)
ereport(WARNING,
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
+
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->tablespace_ids[i], xlrec->db_id);
+
pfree(dst_path);
}
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 40514ab550..62ee0ca978 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -57,6 +57,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -1574,6 +1575,11 @@ tblspc_redo(XLogReaderState *record)
{
xl_tblspc_drop_rec *xlrec = (xl_tblspc_drop_rec *) XLogRecGetData(record);
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->ts_id, InvalidOid);
+
+ XLogFlush(record->EndRecPtr);
+
/*
* If we issued a WAL record for a drop tablespace it implies that
* there were no files in it at all when the DROP was done. That means
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 64708949db..5d9c20cae7 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -65,6 +65,10 @@ extern void XLogDropDatabase(Oid dbid);
extern void XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
BlockNumber nblocks);
+extern void XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path);
+extern void XLogForgetMissingDir(Oid spcNode, Oid dbNode);
+extern void XLogCheckMissingDirs(void);
+
/* Result codes for XLogReadBufferForRedo[Extended] */
typedef enum
{
--
2.27.0
----Next_Part(Wed_Mar__2_19_31_24_2022_161)----
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v18 3/3] Fix replay of create database records on standby
@ 2020-01-09 20:54 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 18+ messages in thread
From: Alvaro Herrera @ 2020-01-09 20:54 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch adds mechanism similar to invalid page hash table, to track
missing directories during crash recovery. If all the missing
directory references are matched with corresponding drop records at
the end of crash recovery, the standby can safely enter archive
recovery.
Bug identified by Paul Guo.
Authored by Paul Guo, Kyotaro Horiguchi and Asim R P.
---
src/backend/access/transam/xlogrecovery.c | 6 +
src/backend/access/transam/xlogutils.c | 145 ++++++++++++++++++++++
src/backend/commands/dbcommands.c | 56 +++++++++
src/backend/commands/tablespace.c | 6 +
src/include/access/xlogutils.h | 4 +
5 files changed, 217 insertions(+)
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index f9f212680b..97fed1e04d 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -2043,6 +2043,12 @@ CheckRecoveryConsistency(void)
*/
XLogCheckInvalidPages();
+ /*
+ * Check if the XLOG sequence contained any unresolved references to
+ * missing directories.
+ */
+ XLogCheckMissingDirs();
+
reachedConsistency = true;
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 54d5f20734..3f8f7dadac 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -79,6 +79,151 @@ typedef struct xl_invalid_page
static HTAB *invalid_page_tab = NULL;
+/*
+ * If a create database WAL record is being replayed more than once during
+ * crash recovery on a standby, it is possible that either the tablespace
+ * directory or the template database directory is missing. This happens when
+ * the directories are removed by replay of subsequent drop records. Note
+ * that this problem happens only on standby and not on master. On master, a
+ * checkpoint is created at the end of create database operation. On standby,
+ * however, such a strategy (creating restart points during replay) is not
+ * viable because it will slow down WAL replay.
+ *
+ * The alternative is to track references to each missing directory
+ * encountered when performing crash recovery in the following hash table.
+ * Similar to invalid page table above, the expectation is that each missing
+ * directory entry should be matched with a drop database or drop tablespace
+ * WAL record by the end of crash recovery.
+ */
+typedef struct xl_missing_dir_key
+{
+ Oid spcNode;
+ Oid dbNode;
+} xl_missing_dir_key;
+
+typedef struct xl_missing_dir
+{
+ xl_missing_dir_key key;
+ char path[MAXPGPATH];
+} xl_missing_dir;
+
+static HTAB *missing_dir_tab = NULL;
+
+void
+XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path)
+{
+ xl_missing_dir_key key;
+ bool found;
+ xl_missing_dir *entry;
+
+ /*
+ * Database OID may be invalid but tablespace OID must be valid. If
+ * dbNode is InvalidOid, we are logging a missing tablespace directory,
+ * otherwise we are logging a missing database directory.
+ */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ {
+ /* create hash table when first needed */
+ HASHCTL ctl;
+
+ memset(&ctl, 0, sizeof(ctl));
+ ctl.keysize = sizeof(xl_missing_dir_key);
+ ctl.entrysize = sizeof(xl_missing_dir);
+
+ missing_dir_tab = hash_create("XLOG missing directory table",
+ 100,
+ &ctl,
+ HASH_ELEM | HASH_BLOBS);
+ }
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ entry = hash_search(missing_dir_tab, &key, HASH_ENTER, &found);
+
+ if (found)
+ {
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "missing directory %s (tablespace %d) already exists: %s",
+ path, spcNode, entry->path);
+ else
+ elog(DEBUG2, "missing directory %s (tablespace %d database %d) already exists: %s",
+ path, spcNode, dbNode, entry->path);
+ }
+ else
+ {
+ strlcpy(entry->path, path, sizeof(entry->path));
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "logged missing dir %s (tablespace %d)",
+ path, spcNode);
+ else
+ elog(DEBUG2, "logged missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ }
+}
+
+void
+XLogForgetMissingDir(Oid spcNode, Oid dbNode)
+{
+ xl_missing_dir_key key;
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ /* Database OID may be invalid but tablespace OID must be valid. */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ return;
+
+ if (hash_search(missing_dir_tab, &key, HASH_REMOVE, NULL) != NULL)
+ {
+ if (dbNode == InvalidOid)
+ {
+ elog(DEBUG2, "forgot missing dir (tablespace %d)", spcNode);
+ }
+ else
+ {
+ char *path = GetDatabasePath(dbNode, spcNode);
+
+ elog(DEBUG2, "forgot missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ pfree(path);
+ }
+ }
+}
+
+/*
+ * This is called at the end of crash recovery, before entering archive
+ * recovery on a standby. PANIC if the hash table is not empty.
+ */
+void
+XLogCheckMissingDirs(void)
+{
+ HASH_SEQ_STATUS status;
+ xl_missing_dir *hentry;
+ bool foundone = false;
+
+ if (missing_dir_tab == NULL)
+ return; /* nothing to do */
+
+ hash_seq_init(&status, missing_dir_tab);
+
+ while ((hentry = (xl_missing_dir *) hash_seq_search(&status)) != NULL)
+ {
+ elog(WARNING, "missing directory \"%s\" tablespace %d database %d",
+ hentry->path, hentry->key.spcNode, hentry->key.dbNode);
+ foundone = true;
+ }
+
+ if (foundone)
+ elog(PANIC, "WAL contains references to missing directories");
+
+ hash_destroy(missing_dir_tab);
+ missing_dir_tab = NULL;
+}
/* Report a reference to an invalid page */
static void
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index c37e3c9a9a..8994e9da99 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -30,6 +30,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -2382,7 +2383,9 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_rec *xlrec = (xl_dbase_create_rec *) XLogRecGetData(record);
char *src_path;
char *dst_path;
+ char *parent_path;
struct stat st;
+ bool skip = false;
src_path = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
dst_path = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
@@ -2400,6 +2403,55 @@ dbase_redo(XLogReaderState *record)
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
}
+ else if (!reachedConsistency)
+ {
+ /*
+ * It is possible that drop tablespace record appearing later in
+ * the WAL as already been replayed. That means we are replaying
+ * the create database record second time, as part of crash
+ * recovery. In that case, the tablespace directory has already
+ * been removed and the create database operation cannot be
+ * replayed. We should skip the replay but remember the missing
+ * tablespace directory, to be matched with a drop tablespace
+ * record later.
+ */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ if (!(stat(parent_path, &st) == 0 && S_ISDIR(st.st_mode)))
+ {
+ XLogReportMissingDir(xlrec->tablespace_id, InvalidOid, parent_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Target tablespace \"%s\" not found. We "
+ "expect to encounter a WAL record that "
+ "removes this directory before reaching "
+ "consistent state.", parent_path)));
+ }
+ pfree(parent_path);
+ }
+
+ /*
+ * Source directory may be missing. E.g. the template database used
+ * for creating this database may have been dropped, due to reasons
+ * noted above. Moving a database from one tablespace may also be a
+ * partner in the crime.
+ */
+ if (!(stat(src_path, &st) == 0 && S_ISDIR(st.st_mode)) &&
+ !reachedConsistency)
+ {
+ XLogReportMissingDir(xlrec->src_tablespace_id, xlrec->src_db_id, src_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Source database \"%s\" not found. We expect "
+ "to encounter a WAL record that removes this "
+ "directory before reaching consistent state.",
+ src_path)));
+ }
+
+ if (skip)
+ return;
/*
* Force dirty buffers out to disk, to ensure source database is
@@ -2462,6 +2514,10 @@ dbase_redo(XLogReaderState *record)
ereport(WARNING,
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
+
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->tablespace_ids[i], xlrec->db_id);
+
pfree(dst_path);
}
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 40514ab550..62ee0ca978 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -57,6 +57,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -1574,6 +1575,11 @@ tblspc_redo(XLogReaderState *record)
{
xl_tblspc_drop_rec *xlrec = (xl_tblspc_drop_rec *) XLogRecGetData(record);
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->ts_id, InvalidOid);
+
+ XLogFlush(record->EndRecPtr);
+
/*
* If we issued a WAL record for a drop tablespace it implies that
* there were no files in it at all when the DROP was done. That means
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 64708949db..5d9c20cae7 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -65,6 +65,10 @@ extern void XLogDropDatabase(Oid dbid);
extern void XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
BlockNumber nblocks);
+extern void XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path);
+extern void XLogForgetMissingDir(Oid spcNode, Oid dbNode);
+extern void XLogCheckMissingDirs(void);
+
/* Result codes for XLogReadBufferForRedo[Extended] */
typedef enum
{
--
2.27.0
----Next_Part(Fri_Mar__4_09_10_48_2022_359)----
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v8 3/3] Fix replay of create database records on standby
@ 2020-01-09 20:54 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 18+ messages in thread
From: Alvaro Herrera @ 2020-01-09 20:54 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch adds mechanism similar to invalid page hash table, to track
missing directories during crash recovery. If all the missing
directory references are matched with corresponding drop records at
the end of crash recovery, the standby can safely enter archive
recovery.
Bug identified by Paul.
Authored by Paul, Kyotaro and Asim R P.
---
src/backend/access/rmgrdesc/dbasedesc.c | 16 ++-
src/backend/access/transam/xlog.c | 6 ++
src/backend/access/transam/xlogutils.c | 130 ++++++++++++++++++++++++
src/backend/commands/dbcommands.c | 54 ++++++++++
src/backend/commands/tablespace.c | 3 +
src/include/access/xlogutils.h | 4 +
src/include/commands/dbcommands.h | 2 +
7 files changed, 210 insertions(+), 5 deletions(-)
diff --git a/src/backend/access/rmgrdesc/dbasedesc.c b/src/backend/access/rmgrdesc/dbasedesc.c
index 73d2a4ca34..f7117873d7 100644
--- a/src/backend/access/rmgrdesc/dbasedesc.c
+++ b/src/backend/access/rmgrdesc/dbasedesc.c
@@ -23,14 +23,17 @@ dbase_desc(StringInfo buf, XLogReaderState *record)
{
char *rec = XLogRecGetData(record);
uint8 info = XLogRecGetInfo(record) & ~XLR_INFO_MASK;
+ char *dbpath1, *dbpath2;
if (info == XLOG_DBASE_CREATE)
{
xl_dbase_create_rec *xlrec = (xl_dbase_create_rec *) rec;
- appendStringInfo(buf, "copy dir %u/%u to %u/%u",
- xlrec->src_tablespace_id, xlrec->src_db_id,
- xlrec->tablespace_id, xlrec->db_id);
+ dbpath1 = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
+ dbpath2 = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
+ appendStringInfo(buf, "copy dir %s to %s", dbpath1, dbpath2);
+ pfree(dbpath2);
+ pfree(dbpath1);
}
else if (info == XLOG_DBASE_DROP)
{
@@ -39,8 +42,11 @@ dbase_desc(StringInfo buf, XLogReaderState *record)
appendStringInfo(buf, "dir");
for (i = 0; i < xlrec->ntablespaces; i++)
- appendStringInfo(buf, " %u/%u",
- xlrec->tablespace_ids[i], xlrec->db_id);
+ {
+ dbpath1 = GetDatabasePath(xlrec->db_id, xlrec->tablespace_ids[i]);
+ appendStringInfo(buf, "%s", dbpath1);
+ pfree(dbpath1);
+ }
}
}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 7f4f784c0e..d97e48f369 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7890,6 +7890,12 @@ CheckRecoveryConsistency(void)
*/
XLogCheckInvalidPages();
+ /*
+ * Check if the XLOG sequence contained any unresolved references to
+ * missing directories.
+ */
+ XLogCheckMissingDirs();
+
reachedConsistency = true;
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index b55c383370..6c2dd5aba1 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -56,6 +56,136 @@ typedef struct xl_invalid_page
static HTAB *invalid_page_tab = NULL;
+/*
+ * If a create database WAL record is being replayed more than once during
+ * crash recovery on a standby, it is possible that either the tablespace
+ * directory or the template database directory is missing. This happens when
+ * the directories are removed by replay of subsequent drop records. Note
+ * that this problem happens only on standby and not on master. On master, a
+ * checkpoint is created at the end of create database operation. On standby,
+ * however, such a strategy (creating restart points during replay) is not
+ * viable because it will slow down WAL replay.
+ *
+ * The alternative is to track references to each missing directory
+ * encountered when performing crash recovery in the following hash table.
+ * Similar to invalid page table above, the expectation is that each missing
+ * directory entry should be matched with a drop database or drop tablespace
+ * WAL record by the end of crash recovery.
+ */
+typedef struct xl_missing_dir_key
+{
+ Oid spcNode;
+ Oid dbNode;
+} xl_missing_dir_key;
+
+typedef struct xl_missing_dir
+{
+ xl_missing_dir_key key;
+ char path[MAXPGPATH];
+} xl_missing_dir;
+
+static HTAB *missing_dir_tab = NULL;
+
+void
+XLogLogMissingDir(Oid spcNode, Oid dbNode, char *path)
+{
+ xl_missing_dir_key key;
+ bool found;
+ xl_missing_dir *entry;
+
+ /*
+ * Database OID may be invalid but tablespace OID must be valid. If
+ * dbNode is InvalidOid, we are logging a missing tablespace directory,
+ * otherwise we are logging a missing database directory.
+ */
+ Assert(OidIsValid(spcNode));
+
+ if (reachedConsistency)
+ elog(PANIC, "cannot find directory %s tablespace %d database %d",
+ path, spcNode, dbNode);
+
+ if (missing_dir_tab == NULL)
+ {
+ /* create hash table when first needed */
+ HASHCTL ctl;
+
+ memset(&ctl, 0, sizeof(ctl));
+ ctl.keysize = sizeof(xl_missing_dir_key);
+ ctl.entrysize = sizeof(xl_missing_dir);
+
+ missing_dir_tab = hash_create("XLOG missing directory table",
+ 100,
+ &ctl,
+ HASH_ELEM | HASH_BLOBS);
+ }
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ entry = hash_search(missing_dir_tab, &key, HASH_ENTER, &found);
+
+ if (found)
+ elog(DEBUG2, "missing directory %s tablespace %d database %d already exists: %s",
+ path, spcNode, dbNode, entry->path);
+ else
+ {
+ strlcpy(entry->path, path, sizeof(entry->path));
+ elog(DEBUG2, "logged missing dir %s tablespace %d database %d",
+ path, spcNode, dbNode);
+ }
+}
+
+void
+XLogForgetMissingDir(Oid spcNode, Oid dbNode, char *path)
+{
+ xl_missing_dir_key key;
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ /* Database OID may be invalid but tablespace OID must be valid. */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ return;
+
+ if (hash_search(missing_dir_tab, &key, HASH_REMOVE, NULL) == NULL)
+ elog(DEBUG2, "dir %s tablespace %d database %d is not missing",
+ path, spcNode, dbNode);
+ else
+ elog(DEBUG2, "forgot missing dir %s for tablespace %d database %d",
+ path, spcNode, dbNode);
+}
+
+/*
+ * This is called at the end of crash recovery, before entering archive
+ * recovery on a standby. PANIC if the hash table is not empty.
+ */
+void
+XLogCheckMissingDirs(void)
+{
+ HASH_SEQ_STATUS status;
+ xl_missing_dir *hentry;
+ bool foundone = false;
+
+ if (missing_dir_tab == NULL)
+ return; /* nothing to do */
+
+ hash_seq_init(&status, missing_dir_tab);
+
+ while ((hentry = (xl_missing_dir *) hash_seq_search(&status)) != NULL)
+ {
+ elog(WARNING, "missing directory \"%s\" tablespace %d database %d",
+ hentry->path, hentry->key.spcNode, hentry->key.dbNode);
+ foundone = true;
+ }
+
+ if (foundone)
+ elog(PANIC, "WAL contains references to missing directories");
+
+ hash_destroy(missing_dir_tab);
+ missing_dir_tab = NULL;
+}
/* Report a reference to an invalid page */
static void
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 367c30adb0..6d6668e4f8 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -46,6 +46,7 @@
#include "commands/defrem.h"
#include "commands/seclabel.h"
#include "commands/tablespace.h"
+#include "common/file_perm.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "pgstat.h"
@@ -2185,7 +2186,9 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_rec *xlrec = (xl_dbase_create_rec *) XLogRecGetData(record);
char *src_path;
char *dst_path;
+ char *parent_path;
struct stat st;
+ bool skip = false;
src_path = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
dst_path = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
@@ -2203,6 +2206,54 @@ dbase_redo(XLogReaderState *record)
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
}
+ else
+ {
+ /*
+ * It is possible that drop tablespace record appearing later in
+ * the WAL as already been replayed. That means we are replaying
+ * the create database record second time, as part of crash
+ * recovery. In that case, the tablespace directory has already
+ * been removed and the create database operation cannot be
+ * replayed. We should skip the replay but remember the missing
+ * tablespace directory, to be matched with a drop tablespace
+ * record later.
+ */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ if (!(stat(parent_path, &st) == 0 && S_ISDIR(st.st_mode)))
+ {
+ XLogLogMissingDir(xlrec->tablespace_id, InvalidOid, dst_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Target tablespace \"%s\" not found. We "
+ "expect to encounter a WAL record that "
+ "removes this directory before reaching "
+ "consistent state.", parent_path)));
+ }
+ pfree(parent_path);
+ }
+
+ /*
+ * Source directory may be missing. E.g. the template database used
+ * for creating this database may have been dropped, due to reasons
+ * noted above. Moving a database from one tablespace may also be a
+ * partner in the crime.
+ */
+ if (!(stat(src_path, &st) == 0 && S_ISDIR(st.st_mode)))
+ {
+ XLogLogMissingDir(xlrec->src_tablespace_id, xlrec->src_db_id, src_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Source database \"%s\" not found. We expect "
+ "to encounter a WAL record that removes this "
+ "directory before reaching consistent state.",
+ src_path)));
+ }
+
+ if (skip)
+ return;
/*
* Force dirty buffers out to disk, to ensure source database is
@@ -2260,6 +2311,9 @@ dbase_redo(XLogReaderState *record)
ereport(WARNING,
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
+
+ XLogForgetMissingDir(xlrec->tablespace_ids[i], xlrec->db_id, dst_path);
+
pfree(dst_path);
}
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 051478057f..33407dceeb 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -58,6 +58,7 @@
#include "access/xact.h"
#include "access/xlog.h"
#include "access/xloginsert.h"
+#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
#include "catalog/indexing.h"
@@ -1516,6 +1517,8 @@ tblspc_redo(XLogReaderState *record)
{
xl_tblspc_drop_rec *xlrec = (xl_tblspc_drop_rec *) XLogRecGetData(record);
+ XLogForgetMissingDir(xlrec->ts_id, InvalidOid, "");
+
/*
* If we issued a WAL record for a drop tablespace it implies that
* there were no files in it at all when the DROP was done. That means
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 5181a077d9..4106735006 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -23,6 +23,10 @@ extern void XLogDropDatabase(Oid dbid);
extern void XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
BlockNumber nblocks);
+extern void XLogLogMissingDir(Oid spcNode, Oid dbNode, char *path);
+extern void XLogForgetMissingDir(Oid spcNode, Oid dbNode, char *path);
+extern void XLogCheckMissingDirs(void);
+
/* Result codes for XLogReadBufferForRedo[Extended] */
typedef enum
{
diff --git a/src/include/commands/dbcommands.h b/src/include/commands/dbcommands.h
index f8f6d5ffd0..b71b400e70 100644
--- a/src/include/commands/dbcommands.h
+++ b/src/include/commands/dbcommands.h
@@ -19,6 +19,8 @@
#include "lib/stringinfo.h"
#include "nodes/parsenodes.h"
+extern void CheckMissingDirs4DbaseRedo(void);
+
extern Oid createdb(ParseState *pstate, const CreatedbStmt *stmt);
extern void dropdb(const char *dbname, bool missing_ok, bool force);
extern void DropDatabase(ParseState *pstate, DropdbStmt *stmt);
--
2.20.1
--RnlQjJ0d97Da+TV1--
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v16 3/3] Fix replay of create database records on standby
@ 2020-01-09 20:54 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 18+ messages in thread
From: Alvaro Herrera @ 2020-01-09 20:54 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch adds mechanism similar to invalid page hash table, to track
missing directories during crash recovery. If all the missing
directory references are matched with corresponding drop records at
the end of crash recovery, the standby can safely enter archive
recovery.
Bug identified by Paul Guo.
Authored by Paul Guo, Kyotaro Horiguchi and Asim R P.
---
src/backend/access/transam/xlogrecovery.c | 6 +
src/backend/access/transam/xlogutils.c | 145 ++++++++++++++++++++++
src/backend/commands/dbcommands.c | 56 +++++++++
src/backend/commands/tablespace.c | 6 +
src/include/access/xlogutils.h | 4 +
5 files changed, 217 insertions(+)
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index f9f212680b..97fed1e04d 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -2043,6 +2043,12 @@ CheckRecoveryConsistency(void)
*/
XLogCheckInvalidPages();
+ /*
+ * Check if the XLOG sequence contained any unresolved references to
+ * missing directories.
+ */
+ XLogCheckMissingDirs();
+
reachedConsistency = true;
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 54d5f20734..3f8f7dadac 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -79,6 +79,151 @@ typedef struct xl_invalid_page
static HTAB *invalid_page_tab = NULL;
+/*
+ * If a create database WAL record is being replayed more than once during
+ * crash recovery on a standby, it is possible that either the tablespace
+ * directory or the template database directory is missing. This happens when
+ * the directories are removed by replay of subsequent drop records. Note
+ * that this problem happens only on standby and not on master. On master, a
+ * checkpoint is created at the end of create database operation. On standby,
+ * however, such a strategy (creating restart points during replay) is not
+ * viable because it will slow down WAL replay.
+ *
+ * The alternative is to track references to each missing directory
+ * encountered when performing crash recovery in the following hash table.
+ * Similar to invalid page table above, the expectation is that each missing
+ * directory entry should be matched with a drop database or drop tablespace
+ * WAL record by the end of crash recovery.
+ */
+typedef struct xl_missing_dir_key
+{
+ Oid spcNode;
+ Oid dbNode;
+} xl_missing_dir_key;
+
+typedef struct xl_missing_dir
+{
+ xl_missing_dir_key key;
+ char path[MAXPGPATH];
+} xl_missing_dir;
+
+static HTAB *missing_dir_tab = NULL;
+
+void
+XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path)
+{
+ xl_missing_dir_key key;
+ bool found;
+ xl_missing_dir *entry;
+
+ /*
+ * Database OID may be invalid but tablespace OID must be valid. If
+ * dbNode is InvalidOid, we are logging a missing tablespace directory,
+ * otherwise we are logging a missing database directory.
+ */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ {
+ /* create hash table when first needed */
+ HASHCTL ctl;
+
+ memset(&ctl, 0, sizeof(ctl));
+ ctl.keysize = sizeof(xl_missing_dir_key);
+ ctl.entrysize = sizeof(xl_missing_dir);
+
+ missing_dir_tab = hash_create("XLOG missing directory table",
+ 100,
+ &ctl,
+ HASH_ELEM | HASH_BLOBS);
+ }
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ entry = hash_search(missing_dir_tab, &key, HASH_ENTER, &found);
+
+ if (found)
+ {
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "missing directory %s (tablespace %d) already exists: %s",
+ path, spcNode, entry->path);
+ else
+ elog(DEBUG2, "missing directory %s (tablespace %d database %d) already exists: %s",
+ path, spcNode, dbNode, entry->path);
+ }
+ else
+ {
+ strlcpy(entry->path, path, sizeof(entry->path));
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "logged missing dir %s (tablespace %d)",
+ path, spcNode);
+ else
+ elog(DEBUG2, "logged missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ }
+}
+
+void
+XLogForgetMissingDir(Oid spcNode, Oid dbNode)
+{
+ xl_missing_dir_key key;
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ /* Database OID may be invalid but tablespace OID must be valid. */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ return;
+
+ if (hash_search(missing_dir_tab, &key, HASH_REMOVE, NULL) != NULL)
+ {
+ if (dbNode == InvalidOid)
+ {
+ elog(DEBUG2, "forgot missing dir (tablespace %d)", spcNode);
+ }
+ else
+ {
+ char *path = GetDatabasePath(dbNode, spcNode);
+
+ elog(DEBUG2, "forgot missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ pfree(path);
+ }
+ }
+}
+
+/*
+ * This is called at the end of crash recovery, before entering archive
+ * recovery on a standby. PANIC if the hash table is not empty.
+ */
+void
+XLogCheckMissingDirs(void)
+{
+ HASH_SEQ_STATUS status;
+ xl_missing_dir *hentry;
+ bool foundone = false;
+
+ if (missing_dir_tab == NULL)
+ return; /* nothing to do */
+
+ hash_seq_init(&status, missing_dir_tab);
+
+ while ((hentry = (xl_missing_dir *) hash_seq_search(&status)) != NULL)
+ {
+ elog(WARNING, "missing directory \"%s\" tablespace %d database %d",
+ hentry->path, hentry->key.spcNode, hentry->key.dbNode);
+ foundone = true;
+ }
+
+ if (foundone)
+ elog(PANIC, "WAL contains references to missing directories");
+
+ hash_destroy(missing_dir_tab);
+ missing_dir_tab = NULL;
+}
/* Report a reference to an invalid page */
static void
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index c37e3c9a9a..8994e9da99 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -30,6 +30,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -2382,7 +2383,9 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_rec *xlrec = (xl_dbase_create_rec *) XLogRecGetData(record);
char *src_path;
char *dst_path;
+ char *parent_path;
struct stat st;
+ bool skip = false;
src_path = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
dst_path = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
@@ -2400,6 +2403,55 @@ dbase_redo(XLogReaderState *record)
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
}
+ else if (!reachedConsistency)
+ {
+ /*
+ * It is possible that drop tablespace record appearing later in
+ * the WAL as already been replayed. That means we are replaying
+ * the create database record second time, as part of crash
+ * recovery. In that case, the tablespace directory has already
+ * been removed and the create database operation cannot be
+ * replayed. We should skip the replay but remember the missing
+ * tablespace directory, to be matched with a drop tablespace
+ * record later.
+ */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ if (!(stat(parent_path, &st) == 0 && S_ISDIR(st.st_mode)))
+ {
+ XLogReportMissingDir(xlrec->tablespace_id, InvalidOid, parent_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Target tablespace \"%s\" not found. We "
+ "expect to encounter a WAL record that "
+ "removes this directory before reaching "
+ "consistent state.", parent_path)));
+ }
+ pfree(parent_path);
+ }
+
+ /*
+ * Source directory may be missing. E.g. the template database used
+ * for creating this database may have been dropped, due to reasons
+ * noted above. Moving a database from one tablespace may also be a
+ * partner in the crime.
+ */
+ if (!(stat(src_path, &st) == 0 && S_ISDIR(st.st_mode)) &&
+ !reachedConsistency)
+ {
+ XLogReportMissingDir(xlrec->src_tablespace_id, xlrec->src_db_id, src_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Source database \"%s\" not found. We expect "
+ "to encounter a WAL record that removes this "
+ "directory before reaching consistent state.",
+ src_path)));
+ }
+
+ if (skip)
+ return;
/*
* Force dirty buffers out to disk, to ensure source database is
@@ -2462,6 +2514,10 @@ dbase_redo(XLogReaderState *record)
ereport(WARNING,
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
+
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->tablespace_ids[i], xlrec->db_id);
+
pfree(dst_path);
}
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 40514ab550..62ee0ca978 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -57,6 +57,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -1574,6 +1575,11 @@ tblspc_redo(XLogReaderState *record)
{
xl_tblspc_drop_rec *xlrec = (xl_tblspc_drop_rec *) XLogRecGetData(record);
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->ts_id, InvalidOid);
+
+ XLogFlush(record->EndRecPtr);
+
/*
* If we issued a WAL record for a drop tablespace it implies that
* there were no files in it at all when the DROP was done. That means
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 64708949db..5d9c20cae7 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -65,6 +65,10 @@ extern void XLogDropDatabase(Oid dbid);
extern void XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
BlockNumber nblocks);
+extern void XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path);
+extern void XLogForgetMissingDir(Oid spcNode, Oid dbNode);
+extern void XLogCheckMissingDirs(void);
+
/* Result codes for XLogReadBufferForRedo[Extended] */
typedef enum
{
--
2.27.0
----Next_Part(Wed_Mar__2_16_59_09_2022_031)----
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v13 3/3] Fix replay of create database records on standby
@ 2020-01-09 20:54 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 18+ messages in thread
From: Alvaro Herrera @ 2020-01-09 20:54 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch adds mechanism similar to invalid page hash table, to track
missing directories during crash recovery. If all the missing
directory references are matched with corresponding drop records at
the end of crash recovery, the standby can safely enter archive
recovery.
Bug identified by Paul Guo.
Authored by Paul Guo, Kyotaro Horiguchi and Asim R P.
---
src/backend/access/transam/xlog.c | 6 +
src/backend/access/transam/xlogutils.c | 145 +++++++++++++++++++++++++
src/backend/commands/dbcommands.c | 55 ++++++++++
src/backend/commands/tablespace.c | 5 +
src/include/access/xlogutils.h | 4 +
5 files changed, 215 insertions(+)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 5cda30836f..c6d5fc782f 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8313,6 +8313,12 @@ CheckRecoveryConsistency(void)
*/
XLogCheckInvalidPages();
+ /*
+ * Check if the XLOG sequence contained any unresolved references to
+ * missing directories.
+ */
+ XLogCheckMissingDirs();
+
reachedConsistency = true;
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index b33e0531ed..99abf8b2f4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -79,6 +79,151 @@ typedef struct xl_invalid_page
static HTAB *invalid_page_tab = NULL;
+/*
+ * If a create database WAL record is being replayed more than once during
+ * crash recovery on a standby, it is possible that either the tablespace
+ * directory or the template database directory is missing. This happens when
+ * the directories are removed by replay of subsequent drop records. Note
+ * that this problem happens only on standby and not on master. On master, a
+ * checkpoint is created at the end of create database operation. On standby,
+ * however, such a strategy (creating restart points during replay) is not
+ * viable because it will slow down WAL replay.
+ *
+ * The alternative is to track references to each missing directory
+ * encountered when performing crash recovery in the following hash table.
+ * Similar to invalid page table above, the expectation is that each missing
+ * directory entry should be matched with a drop database or drop tablespace
+ * WAL record by the end of crash recovery.
+ */
+typedef struct xl_missing_dir_key
+{
+ Oid spcNode;
+ Oid dbNode;
+} xl_missing_dir_key;
+
+typedef struct xl_missing_dir
+{
+ xl_missing_dir_key key;
+ char path[MAXPGPATH];
+} xl_missing_dir;
+
+static HTAB *missing_dir_tab = NULL;
+
+void
+XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path)
+{
+ xl_missing_dir_key key;
+ bool found;
+ xl_missing_dir *entry;
+
+ /*
+ * Database OID may be invalid but tablespace OID must be valid. If
+ * dbNode is InvalidOid, we are logging a missing tablespace directory,
+ * otherwise we are logging a missing database directory.
+ */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ {
+ /* create hash table when first needed */
+ HASHCTL ctl;
+
+ memset(&ctl, 0, sizeof(ctl));
+ ctl.keysize = sizeof(xl_missing_dir_key);
+ ctl.entrysize = sizeof(xl_missing_dir);
+
+ missing_dir_tab = hash_create("XLOG missing directory table",
+ 100,
+ &ctl,
+ HASH_ELEM | HASH_BLOBS);
+ }
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ entry = hash_search(missing_dir_tab, &key, HASH_ENTER, &found);
+
+ if (found)
+ {
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "missing directory %s (tablespace %d) already exists: %s",
+ path, spcNode, entry->path);
+ else
+ elog(DEBUG2, "missing directory %s (tablespace %d database %d) already exists: %s",
+ path, spcNode, dbNode, entry->path);
+ }
+ else
+ {
+ strlcpy(entry->path, path, sizeof(entry->path));
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "logged missing dir %s (tablespace %d)",
+ path, spcNode);
+ else
+ elog(DEBUG2, "logged missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ }
+}
+
+void
+XLogForgetMissingDir(Oid spcNode, Oid dbNode)
+{
+ xl_missing_dir_key key;
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ /* Database OID may be invalid but tablespace OID must be valid. */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ return;
+
+ if (hash_search(missing_dir_tab, &key, HASH_REMOVE, NULL) != NULL)
+ {
+ if (dbNode == InvalidOid)
+ {
+ elog(DEBUG2, "forgot missing dir (tablespace %d)", spcNode);
+ }
+ else
+ {
+ char *path = GetDatabasePath(dbNode, spcNode);
+
+ elog(DEBUG2, "forgot missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ pfree(path);
+ }
+ }
+}
+
+/*
+ * This is called at the end of crash recovery, before entering archive
+ * recovery on a standby. PANIC if the hash table is not empty.
+ */
+void
+XLogCheckMissingDirs(void)
+{
+ HASH_SEQ_STATUS status;
+ xl_missing_dir *hentry;
+ bool foundone = false;
+
+ if (missing_dir_tab == NULL)
+ return; /* nothing to do */
+
+ hash_seq_init(&status, missing_dir_tab);
+
+ while ((hentry = (xl_missing_dir *) hash_seq_search(&status)) != NULL)
+ {
+ elog(WARNING, "missing directory \"%s\" tablespace %d database %d",
+ hentry->path, hentry->key.spcNode, hentry->key.dbNode);
+ foundone = true;
+ }
+
+ if (foundone)
+ elog(PANIC, "WAL contains references to missing directories");
+
+ hash_destroy(missing_dir_tab);
+ missing_dir_tab = NULL;
+}
/* Report a reference to an invalid page */
static void
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 029fab48df..0f483edb71 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -2143,7 +2143,9 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_rec *xlrec = (xl_dbase_create_rec *) XLogRecGetData(record);
char *src_path;
char *dst_path;
+ char *parent_path;
struct stat st;
+ bool skip = false;
src_path = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
dst_path = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
@@ -2161,6 +2163,55 @@ dbase_redo(XLogReaderState *record)
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
}
+ else if (!reachedConsistency)
+ {
+ /*
+ * It is possible that drop tablespace record appearing later in
+ * the WAL as already been replayed. That means we are replaying
+ * the create database record second time, as part of crash
+ * recovery. In that case, the tablespace directory has already
+ * been removed and the create database operation cannot be
+ * replayed. We should skip the replay but remember the missing
+ * tablespace directory, to be matched with a drop tablespace
+ * record later.
+ */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ if (!(stat(parent_path, &st) == 0 && S_ISDIR(st.st_mode)))
+ {
+ XLogReportMissingDir(xlrec->tablespace_id, InvalidOid, parent_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Target tablespace \"%s\" not found. We "
+ "expect to encounter a WAL record that "
+ "removes this directory before reaching "
+ "consistent state.", parent_path)));
+ }
+ pfree(parent_path);
+ }
+
+ /*
+ * Source directory may be missing. E.g. the template database used
+ * for creating this database may have been dropped, due to reasons
+ * noted above. Moving a database from one tablespace may also be a
+ * partner in the crime.
+ */
+ if (!(stat(src_path, &st) == 0 && S_ISDIR(st.st_mode)) &&
+ !reachedConsistency)
+ {
+ XLogReportMissingDir(xlrec->src_tablespace_id, xlrec->src_db_id, src_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Source database \"%s\" not found. We expect "
+ "to encounter a WAL record that removes this "
+ "directory before reaching consistent state.",
+ src_path)));
+ }
+
+ if (skip)
+ return;
/*
* Force dirty buffers out to disk, to ensure source database is
@@ -2218,6 +2269,10 @@ dbase_redo(XLogReaderState *record)
ereport(WARNING,
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
+
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->tablespace_ids[i], xlrec->db_id);
+
pfree(dst_path);
}
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 4b96eec9df..0d5dfe007f 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -1527,6 +1527,11 @@ tblspc_redo(XLogReaderState *record)
{
xl_tblspc_drop_rec *xlrec = (xl_tblspc_drop_rec *) XLogRecGetData(record);
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->ts_id, InvalidOid);
+
+ XLogFlush(record->EndRecPtr);
+
/*
* If we issued a WAL record for a drop tablespace it implies that
* there were no files in it at all when the DROP was done. That means
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index eebc91f3a5..3341efc052 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -65,6 +65,10 @@ extern void XLogDropDatabase(Oid dbid);
extern void XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
BlockNumber nblocks);
+extern void XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path);
+extern void XLogForgetMissingDir(Oid spcNode, Oid dbNode);
+extern void XLogCheckMissingDirs(void);
+
/* Result codes for XLogReadBufferForRedo[Extended] */
typedef enum
{
--
2.27.0
----Next_Part(Mon_Nov__8_17_55_16_2021_435)----
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v14 3/3] Fix replay of create database records on standby
@ 2020-01-09 20:54 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 18+ messages in thread
From: Alvaro Herrera @ 2020-01-09 20:54 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch adds mechanism similar to invalid page hash table, to track
missing directories during crash recovery. If all the missing
directory references are matched with corresponding drop records at
the end of crash recovery, the standby can safely enter archive
recovery.
Bug identified by Paul Guo.
Authored by Paul Guo, Kyotaro Horiguchi and Asim R P.
---
src/backend/access/transam/xlog.c | 6 +
src/backend/access/transam/xlogutils.c | 145 +++++++++++++++++++++++++
src/backend/commands/dbcommands.c | 55 ++++++++++
src/backend/commands/tablespace.c | 5 +
src/include/access/xlogutils.h | 4 +
5 files changed, 215 insertions(+)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e073121a7e..badda1deb2 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8309,6 +8309,12 @@ CheckRecoveryConsistency(void)
*/
XLogCheckInvalidPages();
+ /*
+ * Check if the XLOG sequence contained any unresolved references to
+ * missing directories.
+ */
+ XLogCheckMissingDirs();
+
reachedConsistency = true;
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index b33e0531ed..99abf8b2f4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -79,6 +79,151 @@ typedef struct xl_invalid_page
static HTAB *invalid_page_tab = NULL;
+/*
+ * If a create database WAL record is being replayed more than once during
+ * crash recovery on a standby, it is possible that either the tablespace
+ * directory or the template database directory is missing. This happens when
+ * the directories are removed by replay of subsequent drop records. Note
+ * that this problem happens only on standby and not on master. On master, a
+ * checkpoint is created at the end of create database operation. On standby,
+ * however, such a strategy (creating restart points during replay) is not
+ * viable because it will slow down WAL replay.
+ *
+ * The alternative is to track references to each missing directory
+ * encountered when performing crash recovery in the following hash table.
+ * Similar to invalid page table above, the expectation is that each missing
+ * directory entry should be matched with a drop database or drop tablespace
+ * WAL record by the end of crash recovery.
+ */
+typedef struct xl_missing_dir_key
+{
+ Oid spcNode;
+ Oid dbNode;
+} xl_missing_dir_key;
+
+typedef struct xl_missing_dir
+{
+ xl_missing_dir_key key;
+ char path[MAXPGPATH];
+} xl_missing_dir;
+
+static HTAB *missing_dir_tab = NULL;
+
+void
+XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path)
+{
+ xl_missing_dir_key key;
+ bool found;
+ xl_missing_dir *entry;
+
+ /*
+ * Database OID may be invalid but tablespace OID must be valid. If
+ * dbNode is InvalidOid, we are logging a missing tablespace directory,
+ * otherwise we are logging a missing database directory.
+ */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ {
+ /* create hash table when first needed */
+ HASHCTL ctl;
+
+ memset(&ctl, 0, sizeof(ctl));
+ ctl.keysize = sizeof(xl_missing_dir_key);
+ ctl.entrysize = sizeof(xl_missing_dir);
+
+ missing_dir_tab = hash_create("XLOG missing directory table",
+ 100,
+ &ctl,
+ HASH_ELEM | HASH_BLOBS);
+ }
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ entry = hash_search(missing_dir_tab, &key, HASH_ENTER, &found);
+
+ if (found)
+ {
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "missing directory %s (tablespace %d) already exists: %s",
+ path, spcNode, entry->path);
+ else
+ elog(DEBUG2, "missing directory %s (tablespace %d database %d) already exists: %s",
+ path, spcNode, dbNode, entry->path);
+ }
+ else
+ {
+ strlcpy(entry->path, path, sizeof(entry->path));
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "logged missing dir %s (tablespace %d)",
+ path, spcNode);
+ else
+ elog(DEBUG2, "logged missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ }
+}
+
+void
+XLogForgetMissingDir(Oid spcNode, Oid dbNode)
+{
+ xl_missing_dir_key key;
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ /* Database OID may be invalid but tablespace OID must be valid. */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ return;
+
+ if (hash_search(missing_dir_tab, &key, HASH_REMOVE, NULL) != NULL)
+ {
+ if (dbNode == InvalidOid)
+ {
+ elog(DEBUG2, "forgot missing dir (tablespace %d)", spcNode);
+ }
+ else
+ {
+ char *path = GetDatabasePath(dbNode, spcNode);
+
+ elog(DEBUG2, "forgot missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ pfree(path);
+ }
+ }
+}
+
+/*
+ * This is called at the end of crash recovery, before entering archive
+ * recovery on a standby. PANIC if the hash table is not empty.
+ */
+void
+XLogCheckMissingDirs(void)
+{
+ HASH_SEQ_STATUS status;
+ xl_missing_dir *hentry;
+ bool foundone = false;
+
+ if (missing_dir_tab == NULL)
+ return; /* nothing to do */
+
+ hash_seq_init(&status, missing_dir_tab);
+
+ while ((hentry = (xl_missing_dir *) hash_seq_search(&status)) != NULL)
+ {
+ elog(WARNING, "missing directory \"%s\" tablespace %d database %d",
+ hentry->path, hentry->key.spcNode, hentry->key.dbNode);
+ foundone = true;
+ }
+
+ if (foundone)
+ elog(PANIC, "WAL contains references to missing directories");
+
+ hash_destroy(missing_dir_tab);
+ missing_dir_tab = NULL;
+}
/* Report a reference to an invalid page */
static void
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 029fab48df..0f483edb71 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -2143,7 +2143,9 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_rec *xlrec = (xl_dbase_create_rec *) XLogRecGetData(record);
char *src_path;
char *dst_path;
+ char *parent_path;
struct stat st;
+ bool skip = false;
src_path = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
dst_path = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
@@ -2161,6 +2163,55 @@ dbase_redo(XLogReaderState *record)
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
}
+ else if (!reachedConsistency)
+ {
+ /*
+ * It is possible that drop tablespace record appearing later in
+ * the WAL as already been replayed. That means we are replaying
+ * the create database record second time, as part of crash
+ * recovery. In that case, the tablespace directory has already
+ * been removed and the create database operation cannot be
+ * replayed. We should skip the replay but remember the missing
+ * tablespace directory, to be matched with a drop tablespace
+ * record later.
+ */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ if (!(stat(parent_path, &st) == 0 && S_ISDIR(st.st_mode)))
+ {
+ XLogReportMissingDir(xlrec->tablespace_id, InvalidOid, parent_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Target tablespace \"%s\" not found. We "
+ "expect to encounter a WAL record that "
+ "removes this directory before reaching "
+ "consistent state.", parent_path)));
+ }
+ pfree(parent_path);
+ }
+
+ /*
+ * Source directory may be missing. E.g. the template database used
+ * for creating this database may have been dropped, due to reasons
+ * noted above. Moving a database from one tablespace may also be a
+ * partner in the crime.
+ */
+ if (!(stat(src_path, &st) == 0 && S_ISDIR(st.st_mode)) &&
+ !reachedConsistency)
+ {
+ XLogReportMissingDir(xlrec->src_tablespace_id, xlrec->src_db_id, src_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Source database \"%s\" not found. We expect "
+ "to encounter a WAL record that removes this "
+ "directory before reaching consistent state.",
+ src_path)));
+ }
+
+ if (skip)
+ return;
/*
* Force dirty buffers out to disk, to ensure source database is
@@ -2218,6 +2269,10 @@ dbase_redo(XLogReaderState *record)
ereport(WARNING,
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
+
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->tablespace_ids[i], xlrec->db_id);
+
pfree(dst_path);
}
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 4b96eec9df..0d5dfe007f 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -1527,6 +1527,11 @@ tblspc_redo(XLogReaderState *record)
{
xl_tblspc_drop_rec *xlrec = (xl_tblspc_drop_rec *) XLogRecGetData(record);
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->ts_id, InvalidOid);
+
+ XLogFlush(record->EndRecPtr);
+
/*
* If we issued a WAL record for a drop tablespace it implies that
* there were no files in it at all when the DROP was done. That means
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index eebc91f3a5..3341efc052 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -65,6 +65,10 @@ extern void XLogDropDatabase(Oid dbid);
extern void XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
BlockNumber nblocks);
+extern void XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path);
+extern void XLogForgetMissingDir(Oid spcNode, Oid dbNode);
+extern void XLogCheckMissingDirs(void);
+
/* Result codes for XLogReadBufferForRedo[Extended] */
typedef enum
{
--
2.27.0
----Next_Part(Fri_Nov_12_16_43_27_2021_789)----
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v15 3/3] Fix replay of create database records on standby
@ 2020-01-09 20:54 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 18+ messages in thread
From: Alvaro Herrera @ 2020-01-09 20:54 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch adds mechanism similar to invalid page hash table, to track
missing directories during crash recovery. If all the missing
directory references are matched with corresponding drop records at
the end of crash recovery, the standby can safely enter archive
recovery.
Bug identified by Paul Guo.
Authored by Paul Guo, Kyotaro Horiguchi and Asim R P.
---
src/backend/access/transam/xlog.c | 6 +
src/backend/access/transam/xlogutils.c | 145 +++++++++++++++++++++++++
src/backend/commands/dbcommands.c | 55 ++++++++++
src/backend/commands/tablespace.c | 5 +
src/include/access/xlogutils.h | 4 +
5 files changed, 215 insertions(+)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c9d4cbf3ff..ec279c6158 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8314,6 +8314,12 @@ CheckRecoveryConsistency(void)
*/
XLogCheckInvalidPages();
+ /*
+ * Check if the XLOG sequence contained any unresolved references to
+ * missing directories.
+ */
+ XLogCheckMissingDirs();
+
reachedConsistency = true;
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 90e1c48390..cd00e0f01e 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -79,6 +79,151 @@ typedef struct xl_invalid_page
static HTAB *invalid_page_tab = NULL;
+/*
+ * If a create database WAL record is being replayed more than once during
+ * crash recovery on a standby, it is possible that either the tablespace
+ * directory or the template database directory is missing. This happens when
+ * the directories are removed by replay of subsequent drop records. Note
+ * that this problem happens only on standby and not on master. On master, a
+ * checkpoint is created at the end of create database operation. On standby,
+ * however, such a strategy (creating restart points during replay) is not
+ * viable because it will slow down WAL replay.
+ *
+ * The alternative is to track references to each missing directory
+ * encountered when performing crash recovery in the following hash table.
+ * Similar to invalid page table above, the expectation is that each missing
+ * directory entry should be matched with a drop database or drop tablespace
+ * WAL record by the end of crash recovery.
+ */
+typedef struct xl_missing_dir_key
+{
+ Oid spcNode;
+ Oid dbNode;
+} xl_missing_dir_key;
+
+typedef struct xl_missing_dir
+{
+ xl_missing_dir_key key;
+ char path[MAXPGPATH];
+} xl_missing_dir;
+
+static HTAB *missing_dir_tab = NULL;
+
+void
+XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path)
+{
+ xl_missing_dir_key key;
+ bool found;
+ xl_missing_dir *entry;
+
+ /*
+ * Database OID may be invalid but tablespace OID must be valid. If
+ * dbNode is InvalidOid, we are logging a missing tablespace directory,
+ * otherwise we are logging a missing database directory.
+ */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ {
+ /* create hash table when first needed */
+ HASHCTL ctl;
+
+ memset(&ctl, 0, sizeof(ctl));
+ ctl.keysize = sizeof(xl_missing_dir_key);
+ ctl.entrysize = sizeof(xl_missing_dir);
+
+ missing_dir_tab = hash_create("XLOG missing directory table",
+ 100,
+ &ctl,
+ HASH_ELEM | HASH_BLOBS);
+ }
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ entry = hash_search(missing_dir_tab, &key, HASH_ENTER, &found);
+
+ if (found)
+ {
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "missing directory %s (tablespace %d) already exists: %s",
+ path, spcNode, entry->path);
+ else
+ elog(DEBUG2, "missing directory %s (tablespace %d database %d) already exists: %s",
+ path, spcNode, dbNode, entry->path);
+ }
+ else
+ {
+ strlcpy(entry->path, path, sizeof(entry->path));
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "logged missing dir %s (tablespace %d)",
+ path, spcNode);
+ else
+ elog(DEBUG2, "logged missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ }
+}
+
+void
+XLogForgetMissingDir(Oid spcNode, Oid dbNode)
+{
+ xl_missing_dir_key key;
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ /* Database OID may be invalid but tablespace OID must be valid. */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ return;
+
+ if (hash_search(missing_dir_tab, &key, HASH_REMOVE, NULL) != NULL)
+ {
+ if (dbNode == InvalidOid)
+ {
+ elog(DEBUG2, "forgot missing dir (tablespace %d)", spcNode);
+ }
+ else
+ {
+ char *path = GetDatabasePath(dbNode, spcNode);
+
+ elog(DEBUG2, "forgot missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ pfree(path);
+ }
+ }
+}
+
+/*
+ * This is called at the end of crash recovery, before entering archive
+ * recovery on a standby. PANIC if the hash table is not empty.
+ */
+void
+XLogCheckMissingDirs(void)
+{
+ HASH_SEQ_STATUS status;
+ xl_missing_dir *hentry;
+ bool foundone = false;
+
+ if (missing_dir_tab == NULL)
+ return; /* nothing to do */
+
+ hash_seq_init(&status, missing_dir_tab);
+
+ while ((hentry = (xl_missing_dir *) hash_seq_search(&status)) != NULL)
+ {
+ elog(WARNING, "missing directory \"%s\" tablespace %d database %d",
+ hentry->path, hentry->key.spcNode, hentry->key.dbNode);
+ foundone = true;
+ }
+
+ if (foundone)
+ elog(PANIC, "WAL contains references to missing directories");
+
+ hash_destroy(missing_dir_tab);
+ missing_dir_tab = NULL;
+}
/* Report a reference to an invalid page */
static void
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 509d1a3e92..02b080e4ef 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -2143,7 +2143,9 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_rec *xlrec = (xl_dbase_create_rec *) XLogRecGetData(record);
char *src_path;
char *dst_path;
+ char *parent_path;
struct stat st;
+ bool skip = false;
src_path = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
dst_path = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
@@ -2161,6 +2163,55 @@ dbase_redo(XLogReaderState *record)
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
}
+ else if (!reachedConsistency)
+ {
+ /*
+ * It is possible that drop tablespace record appearing later in
+ * the WAL as already been replayed. That means we are replaying
+ * the create database record second time, as part of crash
+ * recovery. In that case, the tablespace directory has already
+ * been removed and the create database operation cannot be
+ * replayed. We should skip the replay but remember the missing
+ * tablespace directory, to be matched with a drop tablespace
+ * record later.
+ */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ if (!(stat(parent_path, &st) == 0 && S_ISDIR(st.st_mode)))
+ {
+ XLogReportMissingDir(xlrec->tablespace_id, InvalidOid, parent_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Target tablespace \"%s\" not found. We "
+ "expect to encounter a WAL record that "
+ "removes this directory before reaching "
+ "consistent state.", parent_path)));
+ }
+ pfree(parent_path);
+ }
+
+ /*
+ * Source directory may be missing. E.g. the template database used
+ * for creating this database may have been dropped, due to reasons
+ * noted above. Moving a database from one tablespace may also be a
+ * partner in the crime.
+ */
+ if (!(stat(src_path, &st) == 0 && S_ISDIR(st.st_mode)) &&
+ !reachedConsistency)
+ {
+ XLogReportMissingDir(xlrec->src_tablespace_id, xlrec->src_db_id, src_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Source database \"%s\" not found. We expect "
+ "to encounter a WAL record that removes this "
+ "directory before reaching consistent state.",
+ src_path)));
+ }
+
+ if (skip)
+ return;
/*
* Force dirty buffers out to disk, to ensure source database is
@@ -2218,6 +2269,10 @@ dbase_redo(XLogReaderState *record)
ereport(WARNING,
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
+
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->tablespace_ids[i], xlrec->db_id);
+
pfree(dst_path);
}
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index b2ccf5e06e..b2975a0bd2 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -1565,6 +1565,11 @@ tblspc_redo(XLogReaderState *record)
{
xl_tblspc_drop_rec *xlrec = (xl_tblspc_drop_rec *) XLogRecGetData(record);
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->ts_id, InvalidOid);
+
+ XLogFlush(record->EndRecPtr);
+
/*
* If we issued a WAL record for a drop tablespace it implies that
* there were no files in it at all when the DROP was done. That means
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 64708949db..5d9c20cae7 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -65,6 +65,10 @@ extern void XLogDropDatabase(Oid dbid);
extern void XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
BlockNumber nblocks);
+extern void XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path);
+extern void XLogForgetMissingDir(Oid spcNode, Oid dbNode);
+extern void XLogCheckMissingDirs(void);
+
/* Result codes for XLogReadBufferForRedo[Extended] */
typedef enum
{
--
2.27.0
----Next_Part(Thu_Jan_20_15_07_22_2022_795)----
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v15 3/3] Fix replay of create database records on standby
@ 2020-01-09 20:54 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 18+ messages in thread
From: Alvaro Herrera @ 2020-01-09 20:54 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch adds mechanism similar to invalid page hash table, to track
missing directories during crash recovery. If all the missing
directory references are matched with corresponding drop records at
the end of crash recovery, the standby can safely enter archive
recovery.
Bug identified by Paul Guo.
Authored by Paul Guo, Kyotaro Horiguchi and Asim R P.
---
src/backend/access/transam/xlog.c | 6 +
src/backend/access/transam/xlogutils.c | 145 +++++++++++++++++++++++++
src/backend/commands/dbcommands.c | 55 ++++++++++
src/backend/commands/tablespace.c | 5 +
src/include/access/xlogutils.h | 4 +
5 files changed, 215 insertions(+)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c9d4cbf3ff..ec279c6158 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8314,6 +8314,12 @@ CheckRecoveryConsistency(void)
*/
XLogCheckInvalidPages();
+ /*
+ * Check if the XLOG sequence contained any unresolved references to
+ * missing directories.
+ */
+ XLogCheckMissingDirs();
+
reachedConsistency = true;
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 90e1c48390..cd00e0f01e 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -79,6 +79,151 @@ typedef struct xl_invalid_page
static HTAB *invalid_page_tab = NULL;
+/*
+ * If a create database WAL record is being replayed more than once during
+ * crash recovery on a standby, it is possible that either the tablespace
+ * directory or the template database directory is missing. This happens when
+ * the directories are removed by replay of subsequent drop records. Note
+ * that this problem happens only on standby and not on master. On master, a
+ * checkpoint is created at the end of create database operation. On standby,
+ * however, such a strategy (creating restart points during replay) is not
+ * viable because it will slow down WAL replay.
+ *
+ * The alternative is to track references to each missing directory
+ * encountered when performing crash recovery in the following hash table.
+ * Similar to invalid page table above, the expectation is that each missing
+ * directory entry should be matched with a drop database or drop tablespace
+ * WAL record by the end of crash recovery.
+ */
+typedef struct xl_missing_dir_key
+{
+ Oid spcNode;
+ Oid dbNode;
+} xl_missing_dir_key;
+
+typedef struct xl_missing_dir
+{
+ xl_missing_dir_key key;
+ char path[MAXPGPATH];
+} xl_missing_dir;
+
+static HTAB *missing_dir_tab = NULL;
+
+void
+XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path)
+{
+ xl_missing_dir_key key;
+ bool found;
+ xl_missing_dir *entry;
+
+ /*
+ * Database OID may be invalid but tablespace OID must be valid. If
+ * dbNode is InvalidOid, we are logging a missing tablespace directory,
+ * otherwise we are logging a missing database directory.
+ */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ {
+ /* create hash table when first needed */
+ HASHCTL ctl;
+
+ memset(&ctl, 0, sizeof(ctl));
+ ctl.keysize = sizeof(xl_missing_dir_key);
+ ctl.entrysize = sizeof(xl_missing_dir);
+
+ missing_dir_tab = hash_create("XLOG missing directory table",
+ 100,
+ &ctl,
+ HASH_ELEM | HASH_BLOBS);
+ }
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ entry = hash_search(missing_dir_tab, &key, HASH_ENTER, &found);
+
+ if (found)
+ {
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "missing directory %s (tablespace %d) already exists: %s",
+ path, spcNode, entry->path);
+ else
+ elog(DEBUG2, "missing directory %s (tablespace %d database %d) already exists: %s",
+ path, spcNode, dbNode, entry->path);
+ }
+ else
+ {
+ strlcpy(entry->path, path, sizeof(entry->path));
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "logged missing dir %s (tablespace %d)",
+ path, spcNode);
+ else
+ elog(DEBUG2, "logged missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ }
+}
+
+void
+XLogForgetMissingDir(Oid spcNode, Oid dbNode)
+{
+ xl_missing_dir_key key;
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ /* Database OID may be invalid but tablespace OID must be valid. */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ return;
+
+ if (hash_search(missing_dir_tab, &key, HASH_REMOVE, NULL) != NULL)
+ {
+ if (dbNode == InvalidOid)
+ {
+ elog(DEBUG2, "forgot missing dir (tablespace %d)", spcNode);
+ }
+ else
+ {
+ char *path = GetDatabasePath(dbNode, spcNode);
+
+ elog(DEBUG2, "forgot missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ pfree(path);
+ }
+ }
+}
+
+/*
+ * This is called at the end of crash recovery, before entering archive
+ * recovery on a standby. PANIC if the hash table is not empty.
+ */
+void
+XLogCheckMissingDirs(void)
+{
+ HASH_SEQ_STATUS status;
+ xl_missing_dir *hentry;
+ bool foundone = false;
+
+ if (missing_dir_tab == NULL)
+ return; /* nothing to do */
+
+ hash_seq_init(&status, missing_dir_tab);
+
+ while ((hentry = (xl_missing_dir *) hash_seq_search(&status)) != NULL)
+ {
+ elog(WARNING, "missing directory \"%s\" tablespace %d database %d",
+ hentry->path, hentry->key.spcNode, hentry->key.dbNode);
+ foundone = true;
+ }
+
+ if (foundone)
+ elog(PANIC, "WAL contains references to missing directories");
+
+ hash_destroy(missing_dir_tab);
+ missing_dir_tab = NULL;
+}
/* Report a reference to an invalid page */
static void
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 509d1a3e92..02b080e4ef 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -2143,7 +2143,9 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_rec *xlrec = (xl_dbase_create_rec *) XLogRecGetData(record);
char *src_path;
char *dst_path;
+ char *parent_path;
struct stat st;
+ bool skip = false;
src_path = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
dst_path = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
@@ -2161,6 +2163,55 @@ dbase_redo(XLogReaderState *record)
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
}
+ else if (!reachedConsistency)
+ {
+ /*
+ * It is possible that drop tablespace record appearing later in
+ * the WAL as already been replayed. That means we are replaying
+ * the create database record second time, as part of crash
+ * recovery. In that case, the tablespace directory has already
+ * been removed and the create database operation cannot be
+ * replayed. We should skip the replay but remember the missing
+ * tablespace directory, to be matched with a drop tablespace
+ * record later.
+ */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ if (!(stat(parent_path, &st) == 0 && S_ISDIR(st.st_mode)))
+ {
+ XLogReportMissingDir(xlrec->tablespace_id, InvalidOid, parent_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Target tablespace \"%s\" not found. We "
+ "expect to encounter a WAL record that "
+ "removes this directory before reaching "
+ "consistent state.", parent_path)));
+ }
+ pfree(parent_path);
+ }
+
+ /*
+ * Source directory may be missing. E.g. the template database used
+ * for creating this database may have been dropped, due to reasons
+ * noted above. Moving a database from one tablespace may also be a
+ * partner in the crime.
+ */
+ if (!(stat(src_path, &st) == 0 && S_ISDIR(st.st_mode)) &&
+ !reachedConsistency)
+ {
+ XLogReportMissingDir(xlrec->src_tablespace_id, xlrec->src_db_id, src_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Source database \"%s\" not found. We expect "
+ "to encounter a WAL record that removes this "
+ "directory before reaching consistent state.",
+ src_path)));
+ }
+
+ if (skip)
+ return;
/*
* Force dirty buffers out to disk, to ensure source database is
@@ -2218,6 +2269,10 @@ dbase_redo(XLogReaderState *record)
ereport(WARNING,
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
+
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->tablespace_ids[i], xlrec->db_id);
+
pfree(dst_path);
}
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index b2ccf5e06e..b2975a0bd2 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -1565,6 +1565,11 @@ tblspc_redo(XLogReaderState *record)
{
xl_tblspc_drop_rec *xlrec = (xl_tblspc_drop_rec *) XLogRecGetData(record);
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->ts_id, InvalidOid);
+
+ XLogFlush(record->EndRecPtr);
+
/*
* If we issued a WAL record for a drop tablespace it implies that
* there were no files in it at all when the DROP was done. That means
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 64708949db..5d9c20cae7 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -65,6 +65,10 @@ extern void XLogDropDatabase(Oid dbid);
extern void XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
BlockNumber nblocks);
+extern void XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path);
+extern void XLogForgetMissingDir(Oid spcNode, Oid dbNode);
+extern void XLogCheckMissingDirs(void);
+
/* Result codes for XLogReadBufferForRedo[Extended] */
typedef enum
{
--
2.27.0
----Next_Part(Thu_Jan_20_17_19_04_2022_594)----
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v20] Fix replay of create database records on standby
@ 2022-03-07 08:10 P <apraveen@pivotal.io>
0 siblings, 0 replies; 18+ messages in thread
From: P @ 2022-03-07 08:10 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch adds mechanism similar to invalid page hash table, to track
missing directories during crash recovery. If all the missing
directory references are matched with corresponding drop records at
the end of crash recovery, the standby can safely enter archive
recovery.
Bug identified by Paul Guo.
Authored by Paul Guo, Kyotaro Horiguchi and Asim R P.
---
src/backend/access/transam/xlogrecovery.c | 6 +
src/backend/access/transam/xlogutils.c | 145 ++++++++++++++++++++
src/backend/commands/dbcommands.c | 56 ++++++++
src/backend/commands/tablespace.c | 16 +++
src/include/access/xlogutils.h | 4 +
src/test/recovery/t/029_replay_tsp_drops.pl | 62 +++++++++
6 files changed, 289 insertions(+)
create mode 100644 src/test/recovery/t/029_replay_tsp_drops.pl
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index f9f212680b..97fed1e04d 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -2043,6 +2043,12 @@ CheckRecoveryConsistency(void)
*/
XLogCheckInvalidPages();
+ /*
+ * Check if the XLOG sequence contained any unresolved references to
+ * missing directories.
+ */
+ XLogCheckMissingDirs();
+
reachedConsistency = true;
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 54d5f20734..3f8f7dadac 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -79,6 +79,151 @@ typedef struct xl_invalid_page
static HTAB *invalid_page_tab = NULL;
+/*
+ * If a create database WAL record is being replayed more than once during
+ * crash recovery on a standby, it is possible that either the tablespace
+ * directory or the template database directory is missing. This happens when
+ * the directories are removed by replay of subsequent drop records. Note
+ * that this problem happens only on standby and not on master. On master, a
+ * checkpoint is created at the end of create database operation. On standby,
+ * however, such a strategy (creating restart points during replay) is not
+ * viable because it will slow down WAL replay.
+ *
+ * The alternative is to track references to each missing directory
+ * encountered when performing crash recovery in the following hash table.
+ * Similar to invalid page table above, the expectation is that each missing
+ * directory entry should be matched with a drop database or drop tablespace
+ * WAL record by the end of crash recovery.
+ */
+typedef struct xl_missing_dir_key
+{
+ Oid spcNode;
+ Oid dbNode;
+} xl_missing_dir_key;
+
+typedef struct xl_missing_dir
+{
+ xl_missing_dir_key key;
+ char path[MAXPGPATH];
+} xl_missing_dir;
+
+static HTAB *missing_dir_tab = NULL;
+
+void
+XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path)
+{
+ xl_missing_dir_key key;
+ bool found;
+ xl_missing_dir *entry;
+
+ /*
+ * Database OID may be invalid but tablespace OID must be valid. If
+ * dbNode is InvalidOid, we are logging a missing tablespace directory,
+ * otherwise we are logging a missing database directory.
+ */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ {
+ /* create hash table when first needed */
+ HASHCTL ctl;
+
+ memset(&ctl, 0, sizeof(ctl));
+ ctl.keysize = sizeof(xl_missing_dir_key);
+ ctl.entrysize = sizeof(xl_missing_dir);
+
+ missing_dir_tab = hash_create("XLOG missing directory table",
+ 100,
+ &ctl,
+ HASH_ELEM | HASH_BLOBS);
+ }
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ entry = hash_search(missing_dir_tab, &key, HASH_ENTER, &found);
+
+ if (found)
+ {
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "missing directory %s (tablespace %d) already exists: %s",
+ path, spcNode, entry->path);
+ else
+ elog(DEBUG2, "missing directory %s (tablespace %d database %d) already exists: %s",
+ path, spcNode, dbNode, entry->path);
+ }
+ else
+ {
+ strlcpy(entry->path, path, sizeof(entry->path));
+ if (dbNode == InvalidOid)
+ elog(DEBUG2, "logged missing dir %s (tablespace %d)",
+ path, spcNode);
+ else
+ elog(DEBUG2, "logged missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ }
+}
+
+void
+XLogForgetMissingDir(Oid spcNode, Oid dbNode)
+{
+ xl_missing_dir_key key;
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ /* Database OID may be invalid but tablespace OID must be valid. */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ return;
+
+ if (hash_search(missing_dir_tab, &key, HASH_REMOVE, NULL) != NULL)
+ {
+ if (dbNode == InvalidOid)
+ {
+ elog(DEBUG2, "forgot missing dir (tablespace %d)", spcNode);
+ }
+ else
+ {
+ char *path = GetDatabasePath(dbNode, spcNode);
+
+ elog(DEBUG2, "forgot missing dir %s (tablespace %d database %d)",
+ path, spcNode, dbNode);
+ pfree(path);
+ }
+ }
+}
+
+/*
+ * This is called at the end of crash recovery, before entering archive
+ * recovery on a standby. PANIC if the hash table is not empty.
+ */
+void
+XLogCheckMissingDirs(void)
+{
+ HASH_SEQ_STATUS status;
+ xl_missing_dir *hentry;
+ bool foundone = false;
+
+ if (missing_dir_tab == NULL)
+ return; /* nothing to do */
+
+ hash_seq_init(&status, missing_dir_tab);
+
+ while ((hentry = (xl_missing_dir *) hash_seq_search(&status)) != NULL)
+ {
+ elog(WARNING, "missing directory \"%s\" tablespace %d database %d",
+ hentry->path, hentry->key.spcNode, hentry->key.dbNode);
+ foundone = true;
+ }
+
+ if (foundone)
+ elog(PANIC, "WAL contains references to missing directories");
+
+ hash_destroy(missing_dir_tab);
+ missing_dir_tab = NULL;
+}
/* Report a reference to an invalid page */
static void
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index c37e3c9a9a..8994e9da99 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -30,6 +30,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -2382,7 +2383,9 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_rec *xlrec = (xl_dbase_create_rec *) XLogRecGetData(record);
char *src_path;
char *dst_path;
+ char *parent_path;
struct stat st;
+ bool skip = false;
src_path = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
dst_path = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
@@ -2400,6 +2403,55 @@ dbase_redo(XLogReaderState *record)
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
}
+ else if (!reachedConsistency)
+ {
+ /*
+ * It is possible that drop tablespace record appearing later in
+ * the WAL as already been replayed. That means we are replaying
+ * the create database record second time, as part of crash
+ * recovery. In that case, the tablespace directory has already
+ * been removed and the create database operation cannot be
+ * replayed. We should skip the replay but remember the missing
+ * tablespace directory, to be matched with a drop tablespace
+ * record later.
+ */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ if (!(stat(parent_path, &st) == 0 && S_ISDIR(st.st_mode)))
+ {
+ XLogReportMissingDir(xlrec->tablespace_id, InvalidOid, parent_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Target tablespace \"%s\" not found. We "
+ "expect to encounter a WAL record that "
+ "removes this directory before reaching "
+ "consistent state.", parent_path)));
+ }
+ pfree(parent_path);
+ }
+
+ /*
+ * Source directory may be missing. E.g. the template database used
+ * for creating this database may have been dropped, due to reasons
+ * noted above. Moving a database from one tablespace may also be a
+ * partner in the crime.
+ */
+ if (!(stat(src_path, &st) == 0 && S_ISDIR(st.st_mode)) &&
+ !reachedConsistency)
+ {
+ XLogReportMissingDir(xlrec->src_tablespace_id, xlrec->src_db_id, src_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping create database WAL record"),
+ errdetail("Source database \"%s\" not found. We expect "
+ "to encounter a WAL record that removes this "
+ "directory before reaching consistent state.",
+ src_path)));
+ }
+
+ if (skip)
+ return;
/*
* Force dirty buffers out to disk, to ensure source database is
@@ -2462,6 +2514,10 @@ dbase_redo(XLogReaderState *record)
ereport(WARNING,
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
+
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->tablespace_ids[i], xlrec->db_id);
+
pfree(dst_path);
}
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 40514ab550..66bd28fc74 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -57,6 +57,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -1574,6 +1575,21 @@ tblspc_redo(XLogReaderState *record)
{
xl_tblspc_drop_rec *xlrec = (xl_tblspc_drop_rec *) XLogRecGetData(record);
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->ts_id, InvalidOid);
+
+ /*
+ * Before we remove the tablespace directory, update minimum recovery
+ * point to cover this WAL record. Once the tablespace is removed,
+ * there's no going back. This manually enforces the WAL-first rule.
+ * Doing this before the removal means that if the removal fails for
+ * some reason, the directory is left alone and needs to be manually
+ * removed. Alternatively you could update the minimum recovery point
+ * after removal, but that would leave a small window where the
+ * WAL-first rule could be violated.
+ */
+ XLogFlush(record->EndRecPtr);
+
/*
* If we issued a WAL record for a drop tablespace it implies that
* there were no files in it at all when the DROP was done. That means
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 64708949db..5d9c20cae7 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -65,6 +65,10 @@ extern void XLogDropDatabase(Oid dbid);
extern void XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
BlockNumber nblocks);
+extern void XLogReportMissingDir(Oid spcNode, Oid dbNode, char *path);
+extern void XLogForgetMissingDir(Oid spcNode, Oid dbNode);
+extern void XLogCheckMissingDirs(void);
+
/* Result codes for XLogReadBufferForRedo[Extended] */
typedef enum
{
diff --git a/src/test/recovery/t/029_replay_tsp_drops.pl b/src/test/recovery/t/029_replay_tsp_drops.pl
new file mode 100644
index 0000000000..de2a92661c
--- /dev/null
+++ b/src/test/recovery/t/029_replay_tsp_drops.pl
@@ -0,0 +1,62 @@
+# Copyright (c) 2022, PostgreSQL Global Development Group
+
+# Test recovery involving tablespace droppings. If recovery stops
+# after once tablespace is removed, the next recovery should properly
+# ignore the operations within the removed tablespaces.
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+#use File::Compare;
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary1');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+$node_primary->psql('postgres',
+qq[
+ SET allow_in_place_tablespaces=on;
+ CREATE TABLESPACE dropme_ts1 LOCATION '';
+ CREATE TABLESPACE dropme_ts2 LOCATION '';
+ CREATE TABLESPACE source_ts LOCATION '';
+ CREATE TABLESPACE target_ts LOCATION '';
+ CREATE DATABASE template_db IS_TEMPLATE = true;
+]);
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+my $node_standby = PostgreSQL::Test::Cluster->new('standby1');
+$node_standby->init_from_backup($node_primary, $backup_name, has_streaming => 1);
+$node_standby->start;
+
+# Make sure connection is made
+$node_primary->poll_query_until(
+ 'postgres', 'SELECT count(*) = 1 FROM pg_stat_replication');
+
+$node_standby->safe_psql('postgres', 'CHECKPOINT');
+
+# Do immediate shutdown just after a sequence of CREAT DATABASE / DROP
+# DATABASE / DROP TABLESPACE. This causes CREATE DATABASE WAL records
+# to be applied to already-removed directories.
+$node_primary->safe_psql('postgres',
+ q[CREATE DATABASE dropme_db1 WITH TABLESPACE dropme_ts1;
+ CREATE DATABASE dropme_db2 WITH TABLESPACE dropme_ts2;
+ CREATE DATABASE moveme_db TABLESPACE source_ts;
+ ALTER DATABASE moveme_db SET TABLESPACE target_ts;
+ CREATE DATABASE newdb TEMPLATE template_db;
+ ALTER DATABASE template_db IS_TEMPLATE = false;
+ DROP DATABASE dropme_db1;
+ DROP DATABASE dropme_db2; DROP TABLESPACE dropme_ts2;
+ DROP TABLESPACE source_ts;
+ DROP DATABASE template_db;]);
+
+$node_primary->wait_for_catchup($node_standby, 'replay',
+ $node_primary->lsn('replay'));
+$node_standby->stop('immediate');
+
+# Should restart ignoring directory creation error.
+is($node_standby->start(fail_ok => 1), 1);
+
+# Ensure that a missing tablespace directory during create database
+done_testing();
--
2.27.0
----Next_Part(Mon_Mar__7_17_39_27_2022_800)----
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v21] Fix replay of create database records on standby
@ 2022-03-21 11:34 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 18+ messages in thread
From: Alvaro Herrera @ 2022-03-21 11:34 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch adds mechanism similar to invalid page hash table, to track
missing directories during crash recovery. If all the missing
directory references are matched with corresponding drop records at
the end of crash recovery, the standby can safely enter archive
recovery.
Diagnosed-by: Paul Guo <paulguo@gmail.com>
Author: Paul Guo <paulguo@gmail.com>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Author: Asim R Praveen <apraveen@pivotal.io>
Discussion: https://postgr.es/m/CAEET0ZGx9AvioViLf7nbR_8tH9-=27DN5xWJ2P9-ROH16e4JUA@mail.gmail.com
---
src/backend/access/transam/xlogrecovery.c | 6 +
src/backend/access/transam/xlogutils.c | 159 +++++++++++++++++++-
src/backend/commands/dbcommands.c | 57 +++++++
src/backend/commands/tablespace.c | 17 +++
src/include/access/xlogutils.h | 4 +
src/test/recovery/t/029_replay_tsp_drops.pl | 67 +++++++++
src/tools/pgindent/typedefs.list | 2 +
7 files changed, 311 insertions(+), 1 deletion(-)
create mode 100644 src/test/recovery/t/029_replay_tsp_drops.pl
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 9feea3e6ec..f48d8d51fb 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -2043,6 +2043,12 @@ CheckRecoveryConsistency(void)
*/
XLogCheckInvalidPages();
+ /*
+ * Check if the XLOG sequence contained any unresolved references to
+ * missing directories.
+ */
+ XLogCheckMissingDirs();
+
reachedConsistency = true;
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 511f2f186f..8c1b8216be 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -54,6 +54,164 @@ bool InRecovery = false;
/* Are we in Hot Standby mode? Only valid in startup process, see xlogutils.h */
HotStandbyState standbyState = STANDBY_DISABLED;
+
+/*
+ * If a create database WAL record is being replayed more than once during
+ * crash recovery on a standby, it is possible that either the tablespace
+ * directory or the template database directory is missing. This happens when
+ * the directories are removed by replay of subsequent drop records. Note
+ * that this problem happens only on standby and not on master. On master, a
+ * checkpoint is created at the end of create database operation. On standby,
+ * however, such a strategy (creating restart points during replay) is not
+ * viable because it will slow down WAL replay.
+ *
+ * The alternative is to track references to each missing directory
+ * encountered when performing crash recovery in the following hash table.
+ * Similar to invalid page table above, the expectation is that each missing
+ * directory entry should be matched with a drop database or drop tablespace
+ * WAL record by the end of crash recovery.
+ */
+typedef struct xl_missing_dir_key
+{
+ Oid spcNode;
+ Oid dbNode;
+} xl_missing_dir_key;
+
+typedef struct xl_missing_dir
+{
+ xl_missing_dir_key key;
+ char path[MAXPGPATH];
+} xl_missing_dir;
+
+static HTAB *missing_dir_tab = NULL;
+
+
+/*
+ * Keep track of a directory that wasn't found while replaying database
+ * creation records. These should match up with tablespace removal records
+ * later in the WAL stream; we verify that before reaching consistency.
+ */
+void
+XLogRememberMissingDir(Oid spcNode, Oid dbNode, char *path)
+{
+ xl_missing_dir_key key;
+ bool found;
+ xl_missing_dir *entry;
+
+ /*
+ * Database OID may be invalid but tablespace OID must be valid. If
+ * dbNode is InvalidOid, we are logging a missing tablespace directory,
+ * otherwise we are logging a missing database directory.
+ */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ {
+ /* create hash table when first needed */
+ HASHCTL ctl;
+
+ memset(&ctl, 0, sizeof(ctl));
+ ctl.keysize = sizeof(xl_missing_dir_key);
+ ctl.entrysize = sizeof(xl_missing_dir);
+
+ missing_dir_tab = hash_create("XLOG missing directory table",
+ 100,
+ &ctl,
+ HASH_ELEM | HASH_BLOBS);
+ }
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ entry = hash_search(missing_dir_tab, &key, HASH_ENTER, &found);
+
+ if (found)
+ {
+ if (dbNode == InvalidOid)
+ elog(DEBUG1, "missing directory %s (tablespace %u) already exists: %s",
+ path, spcNode, entry->path);
+ else
+ elog(DEBUG1, "missing directory %s (tablespace %u database %u) already exists: %s",
+ path, spcNode, dbNode, entry->path);
+ }
+ else
+ {
+ strlcpy(entry->path, path, sizeof(entry->path));
+ if (dbNode == InvalidOid)
+ elog(DEBUG1, "logged missing dir %s (tablespace %u)",
+ path, spcNode);
+ else
+ elog(DEBUG1, "logged missing dir %s (tablespace %u database %u)",
+ path, spcNode, dbNode);
+ }
+}
+
+/*
+ * Remove an entry from the list of directories not found. This is to be done
+ * when the matching tablespace removal WAL record is found.
+ */
+void
+XLogForgetMissingDir(Oid spcNode, Oid dbNode)
+{
+ xl_missing_dir_key key;
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ /* Database OID may be invalid but tablespace OID must be valid. */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ return;
+
+ if (hash_search(missing_dir_tab, &key, HASH_REMOVE, NULL) != NULL)
+ {
+ if (dbNode == InvalidOid)
+ {
+ elog(DEBUG2, "forgot missing dir (tablespace %u)", spcNode);
+ }
+ else
+ {
+ char *path = GetDatabasePath(dbNode, spcNode);
+
+ elog(DEBUG2, "forgot missing dir %s (tablespace %u database %u)",
+ path, spcNode, dbNode);
+ pfree(path);
+ }
+ }
+}
+
+/*
+ * This is called at the end of crash recovery, before entering archive
+ * recovery on a standby. PANIC if the hash table is not empty.
+ */
+void
+XLogCheckMissingDirs(void)
+{
+ HASH_SEQ_STATUS status;
+ xl_missing_dir *hentry;
+ bool foundone = false;
+
+ if (missing_dir_tab == NULL)
+ return; /* nothing to do */
+
+ hash_seq_init(&status, missing_dir_tab);
+
+ while ((hentry = (xl_missing_dir *) hash_seq_search(&status)) != NULL)
+ {
+ elog(WARNING, "missing directory \"%s\" tablespace %u database %u",
+ hentry->path, hentry->key.spcNode, hentry->key.dbNode);
+ foundone = true;
+ }
+
+ if (foundone)
+ elog(PANIC, "WAL contains references to missing directories");
+
+ hash_destroy(missing_dir_tab);
+ missing_dir_tab = NULL;
+}
+
+
/*
* During XLOG replay, we may see XLOG records for incremental updates of
* pages that no longer exist, because their relation was later dropped or
@@ -79,7 +237,6 @@ typedef struct xl_invalid_page
static HTAB *invalid_page_tab = NULL;
-
/* Report a reference to an invalid page */
static void
report_invalid_page(int elevel, RelFileNode node, ForkNumber forkno,
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 623e5ec778..95771b06a2 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -30,6 +30,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -2483,7 +2484,9 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_rec *xlrec = (xl_dbase_create_rec *) XLogRecGetData(record);
char *src_path;
char *dst_path;
+ char *parent_path;
struct stat st;
+ bool skip = false;
src_path = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
dst_path = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
@@ -2501,6 +2504,56 @@ dbase_redo(XLogReaderState *record)
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
}
+ else if (!reachedConsistency)
+ {
+ /*
+ * It is possible that a drop tablespace record appearing later in
+ * WAL has already been replayed -- in other words, that we are
+ * replaying the database creation record a second time with no
+ * intervening checkpoint. In that case, the tablespace directory
+ * has already been removed and the create database operation
+ * cannot be replayed. Skip the replay itself, but remember the
+ * fact that the tablespace directory is missing, to be matched
+ * with the expected tablespace drop record later.
+ */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ if (!(stat(parent_path, &st) == 0 && S_ISDIR(st.st_mode)))
+ {
+ XLogRememberMissingDir(xlrec->tablespace_id, InvalidOid, parent_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping replay of database creation WAL record"),
+ errdetail("The target tablespace \"%s\" directory was not found.",
+ parent_path),
+ errhint("A future WAL record that removes the directory before reaching consistent mode is expected.")));
+ }
+ pfree(parent_path);
+ }
+
+ /*
+ * If the source directory is missing, skip the copy and make a note of
+ * it for later.
+ *
+ * One possible reason for this is that the template database used for
+ * creating this database may have been dropped, as noted above.
+ * Moving a database from one tablespace may also be a partner in the
+ * crime.
+ */
+ if (!(stat(src_path, &st) == 0 && S_ISDIR(st.st_mode)) &&
+ !reachedConsistency)
+ {
+ XLogRememberMissingDir(xlrec->src_tablespace_id, xlrec->src_db_id, src_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping replay of database creation WAL record"),
+ errdetail("The source database directory \"%s\" was not found.",
+ src_path),
+ errhint("A future WAL record that removes the directory before reaching consistent mode is expected.")));
+ }
+
+ if (skip)
+ return;
/*
* Force dirty buffers out to disk, to ensure source database is
@@ -2563,6 +2616,10 @@ dbase_redo(XLogReaderState *record)
ereport(WARNING,
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
+
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->tablespace_ids[i], xlrec->db_id);
+
pfree(dst_path);
}
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 40514ab550..55f40831da 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -57,6 +57,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -1574,6 +1575,22 @@ tblspc_redo(XLogReaderState *record)
{
xl_tblspc_drop_rec *xlrec = (xl_tblspc_drop_rec *) XLogRecGetData(record);
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->ts_id, InvalidOid);
+
+ /*
+ * Before we remove the tablespace directory, update minimum recovery
+ * point to cover this WAL record. Once the tablespace is removed,
+ * there's no going back. This manually enforces the WAL-first rule.
+ * Doing this before the removal means that if the removal fails for
+ * some reason, the directory is left alone and needs to be manually
+ * removed. Alternatively we could update the minimum recovery point
+ * after removal, but that would leave a small window where the
+ * WAL-first rule could be violated.
+ */
+ if (!reachedConsistency)
+ XLogFlush(record->EndRecPtr);
+
/*
* If we issued a WAL record for a drop tablespace it implies that
* there were no files in it at all when the DROP was done. That means
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 64708949db..8d48f003b0 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -65,6 +65,10 @@ extern void XLogDropDatabase(Oid dbid);
extern void XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
BlockNumber nblocks);
+extern void XLogRememberMissingDir(Oid spcNode, Oid dbNode, char *path);
+extern void XLogForgetMissingDir(Oid spcNode, Oid dbNode);
+extern void XLogCheckMissingDirs(void);
+
/* Result codes for XLogReadBufferForRedo[Extended] */
typedef enum
{
diff --git a/src/test/recovery/t/029_replay_tsp_drops.pl b/src/test/recovery/t/029_replay_tsp_drops.pl
new file mode 100644
index 0000000000..90a72be489
--- /dev/null
+++ b/src/test/recovery/t/029_replay_tsp_drops.pl
@@ -0,0 +1,67 @@
+# Copyright (c) 2022, PostgreSQL Global Development Group
+
+# Test recovery involving tablespace removal. If recovery stops
+# after once tablespace is removed, the next recovery should properly
+# ignore the operations within the removed tablespaces.
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary1');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+$node_primary->psql('postgres',
+qq[
+ SET allow_in_place_tablespaces=on;
+ CREATE TABLESPACE dropme_ts1 LOCATION '';
+ CREATE TABLESPACE dropme_ts2 LOCATION '';
+ CREATE TABLESPACE source_ts LOCATION '';
+ CREATE TABLESPACE target_ts LOCATION '';
+ CREATE DATABASE template_db IS_TEMPLATE = true;
+]);
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+my $node_standby = PostgreSQL::Test::Cluster->new('standby1');
+$node_standby->init_from_backup($node_primary, $backup_name, has_streaming => 1);
+$node_standby->start;
+
+# Make sure connection is made
+$node_primary->poll_query_until(
+ 'postgres', 'SELECT count(*) = 1 FROM pg_stat_replication');
+
+$node_standby->safe_psql('postgres', 'CHECKPOINT');
+
+# Do immediate shutdown just after a sequence of CREATE DATABASE / DROP
+# DATABASE / DROP TABLESPACE. This causes CREATE DATABASE WAL records
+# to be applied to already-removed directories.
+$node_primary->safe_psql('postgres',
+ q[CREATE DATABASE dropme_db1 WITH TABLESPACE dropme_ts1;
+ CREATE DATABASE dropme_db2 WITH TABLESPACE dropme_ts2;
+ CREATE DATABASE moveme_db TABLESPACE source_ts;
+ ALTER DATABASE moveme_db SET TABLESPACE target_ts;
+ CREATE DATABASE newdb TEMPLATE template_db;
+ ALTER DATABASE template_db IS_TEMPLATE = false;
+ DROP DATABASE dropme_db1;
+ DROP DATABASE dropme_db2; DROP TABLESPACE dropme_ts2;
+ DROP TABLESPACE source_ts;
+ DROP DATABASE template_db;]);
+
+$node_primary->wait_for_catchup($node_standby, 'replay',
+ $node_primary->lsn('replay'));
+$node_standby->stop('immediate');
+
+# Should restart ignoring directory creation error.
+is($node_standby->start, 1, "standby started successfully");
+
+my $log = PostgreSQL::Test::Utils::slurp_file($node_standby->logfile);
+like(
+ $log,
+ qr[WARNING: skipping replay of database creation WAL record],
+ "warning message is logged");
+
+done_testing();
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 93d5190508..4d58159b18 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3736,6 +3736,8 @@ xl_invalid_page
xl_invalid_page_key
xl_invalidations
xl_logical_message
+xl_missing_dir_key
+xl_missing_dir
xl_multi_insert_tuple
xl_multixact_create
xl_multixact_truncate
--
2.30.2
--bjr3nkgj6gtwrbgr--
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH 2/2] Fix replay of create database records on standby
@ 2022-03-28 07:29 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 0 replies; 18+ messages in thread
From: Kyotaro Horiguchi @ 2022-03-28 07:29 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the standby
would fail to recover in such a case. However, the directories could be
legitimately missing. Consider a sequence of WAL records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch adds a mechanism similar to invalid-page tracking, to keep a
tally of missing directories during crash recovery. If all the missing
directory references are matched with corresponding drop records at the
end of crash recovery, the standby can safely continue following the
primary.
Backpatch to from 10 to 12. This fix has already been committed to 13
and later.
A new TAP test file is added to verify the condition.
Diagnosed-by: Paul Guo <paulguo@gmail.com>
Author: Paul Guo <paulguo@gmail.com>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Author: Asim R Praveen <apraveen@pivotal.io>
Discussion: https://postgr.es/m/CAEET0ZGx9AvioViLf7nbR_8tH9-=27DN5xWJ2P9-ROH16e4JUA@mail.gmail.com
---
src/backend/access/transam/xlog.c | 6 +
src/backend/access/transam/xlogutils.c | 159 ++++++++++++++++++++++++-
src/backend/commands/dbcommands.c | 55 +++++++++
src/backend/commands/tablespace.c | 17 +++
src/include/access/xlogutils.h | 4 +
src/tools/pgindent/typedefs.list | 2 +
6 files changed, 242 insertions(+), 1 deletion(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 7141e5dca8..3d3342b714 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8006,6 +8006,12 @@ CheckRecoveryConsistency(void)
*/
XLogCheckInvalidPages();
+ /*
+ * Check if the XLOG sequence contained any unresolved references to
+ * missing directories.
+ */
+ XLogCheckMissingDirs();
+
reachedConsistency = true;
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 10a663bae6..11c40b7446 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -31,6 +31,164 @@
#include "utils/rel.h"
+
+/*
+ * If a create database WAL record is being replayed more than once during
+ * crash recovery on a standby, it is possible that either the tablespace
+ * directory or the template database directory is missing. This happens when
+ * the directories are removed by replay of subsequent drop records. Note
+ * that this problem happens only on standby and not on master. On master, a
+ * checkpoint is created at the end of create database operation. On standby,
+ * however, such a strategy (creating restart points during replay) is not
+ * viable because it will slow down WAL replay.
+ *
+ * The alternative is to track references to each missing directory
+ * encountered when performing crash recovery in the following hash table.
+ * Similar to invalid page table above, the expectation is that each missing
+ * directory entry should be matched with a drop database or drop tablespace
+ * WAL record by the end of crash recovery.
+ */
+typedef struct xl_missing_dir_key
+{
+ Oid spcNode;
+ Oid dbNode;
+} xl_missing_dir_key;
+
+typedef struct xl_missing_dir
+{
+ xl_missing_dir_key key;
+ char path[MAXPGPATH];
+} xl_missing_dir;
+
+static HTAB *missing_dir_tab = NULL;
+
+
+/*
+ * Keep track of a directory that wasn't found while replaying database
+ * creation records. These should match up with tablespace removal records
+ * later in the WAL stream; we verify that before reaching consistency.
+ */
+void
+XLogRememberMissingDir(Oid spcNode, Oid dbNode, char *path)
+{
+ xl_missing_dir_key key;
+ bool found;
+ xl_missing_dir *entry;
+
+ /*
+ * Database OID may be invalid but tablespace OID must be valid. If
+ * dbNode is InvalidOid, we are logging a missing tablespace directory,
+ * otherwise we are logging a missing database directory.
+ */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ {
+ /* create hash table when first needed */
+ HASHCTL ctl;
+
+ memset(&ctl, 0, sizeof(ctl));
+ ctl.keysize = sizeof(xl_missing_dir_key);
+ ctl.entrysize = sizeof(xl_missing_dir);
+
+ missing_dir_tab = hash_create("XLOG missing directory table",
+ 100,
+ &ctl,
+ HASH_ELEM | HASH_BLOBS);
+ }
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ entry = hash_search(missing_dir_tab, &key, HASH_ENTER, &found);
+
+ if (found)
+ {
+ if (dbNode == InvalidOid)
+ elog(DEBUG1, "missing directory %s (tablespace %u) already exists: %s",
+ path, spcNode, entry->path);
+ else
+ elog(DEBUG1, "missing directory %s (tablespace %u database %u) already exists: %s",
+ path, spcNode, dbNode, entry->path);
+ }
+ else
+ {
+ strlcpy(entry->path, path, sizeof(entry->path));
+ if (dbNode == InvalidOid)
+ elog(DEBUG1, "logged missing dir %s (tablespace %u)",
+ path, spcNode);
+ else
+ elog(DEBUG1, "logged missing dir %s (tablespace %u database %u)",
+ path, spcNode, dbNode);
+ }
+}
+
+/*
+ * Remove an entry from the list of directories not found. This is to be done
+ * when the matching tablespace removal WAL record is found.
+ */
+void
+XLogForgetMissingDir(Oid spcNode, Oid dbNode)
+{
+ xl_missing_dir_key key;
+
+ key.spcNode = spcNode;
+ key.dbNode = dbNode;
+
+ /* Database OID may be invalid but tablespace OID must be valid. */
+ Assert(OidIsValid(spcNode));
+
+ if (missing_dir_tab == NULL)
+ return;
+
+ if (hash_search(missing_dir_tab, &key, HASH_REMOVE, NULL) != NULL)
+ {
+ if (dbNode == InvalidOid)
+ {
+ elog(DEBUG2, "forgot missing dir (tablespace %u)", spcNode);
+ }
+ else
+ {
+ char *path = GetDatabasePath(dbNode, spcNode);
+
+ elog(DEBUG2, "forgot missing dir %s (tablespace %u database %u)",
+ path, spcNode, dbNode);
+ pfree(path);
+ }
+ }
+}
+
+/*
+ * This is called at the end of crash recovery, before entering archive
+ * recovery on a standby. PANIC if the hash table is not empty.
+ */
+void
+XLogCheckMissingDirs(void)
+{
+ HASH_SEQ_STATUS status;
+ xl_missing_dir *hentry;
+ bool foundone = false;
+
+ if (missing_dir_tab == NULL)
+ return; /* nothing to do */
+
+ hash_seq_init(&status, missing_dir_tab);
+
+ while ((hentry = (xl_missing_dir *) hash_seq_search(&status)) != NULL)
+ {
+ elog(WARNING, "missing directory \"%s\" tablespace %u database %u",
+ hentry->path, hentry->key.spcNode, hentry->key.dbNode);
+ foundone = true;
+ }
+
+ if (foundone)
+ elog(PANIC, "WAL contains references to missing directories");
+
+ hash_destroy(missing_dir_tab);
+ missing_dir_tab = NULL;
+}
+
+
/*
* During XLOG replay, we may see XLOG records for incremental updates of
* pages that no longer exist, because their relation was later dropped or
@@ -56,7 +214,6 @@ typedef struct xl_invalid_page
static HTAB *invalid_page_tab = NULL;
-
/* Report a reference to an invalid page */
static void
report_invalid_page(int elevel, RelFileNode node, ForkNumber forkno,
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 863f89f19d..44512a8a30 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -2108,7 +2108,9 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_rec *xlrec = (xl_dbase_create_rec *) XLogRecGetData(record);
char *src_path;
char *dst_path;
+ char *parent_path;
struct stat st;
+ bool skip = false;
src_path = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
dst_path = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
@@ -2126,6 +2128,56 @@ dbase_redo(XLogReaderState *record)
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
}
+ else if (!reachedConsistency)
+ {
+ /*
+ * It is possible that a drop tablespace record appearing later in
+ * WAL has already been replayed -- in other words, that we are
+ * replaying the database creation record a second time with no
+ * intervening checkpoint. In that case, the tablespace directory
+ * has already been removed and the create database operation
+ * cannot be replayed. Skip the replay itself, but remember the
+ * fact that the tablespace directory is missing, to be matched
+ * with the expected tablespace drop record later.
+ */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ if (!(stat(parent_path, &st) == 0 && S_ISDIR(st.st_mode)))
+ {
+ XLogRememberMissingDir(xlrec->tablespace_id, InvalidOid, parent_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping replay of database creation WAL record"),
+ errdetail("The target tablespace \"%s\" directory was not found.",
+ parent_path),
+ errhint("A future WAL record that removes the directory before reaching consistent mode is expected.")));
+ }
+ pfree(parent_path);
+ }
+
+ /*
+ * If the source directory is missing, skip the copy and make a note of
+ * it for later.
+ *
+ * One possible reason for this is that the template database used for
+ * creating this database may have been dropped, as noted above.
+ * Moving a database from one tablespace may also be a partner in the
+ * crime.
+ */
+ if (!(stat(src_path, &st) == 0 && S_ISDIR(st.st_mode)) &&
+ !reachedConsistency)
+ {
+ XLogRememberMissingDir(xlrec->src_tablespace_id, xlrec->src_db_id, src_path);
+ skip = true;
+ ereport(WARNING,
+ (errmsg("skipping replay of database creation WAL record"),
+ errdetail("The source database directory \"%s\" was not found.",
+ src_path),
+ errhint("A future WAL record that removes the directory before reaching consistent mode is expected.")));
+ }
+
+ if (skip)
+ return;
/*
* Force dirty buffers out to disk, to ensure source database is
@@ -2181,6 +2233,9 @@ dbase_redo(XLogReaderState *record)
(errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path)));
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->tablespace_id, xlrec->db_id);
+
if (InHotStandby)
{
/*
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index f060c24599..5b600a98ff 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -58,6 +58,7 @@
#include "access/xact.h"
#include "access/xlog.h"
#include "access/xloginsert.h"
+#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
#include "catalog/indexing.h"
@@ -1530,6 +1531,22 @@ tblspc_redo(XLogReaderState *record)
{
xl_tblspc_drop_rec *xlrec = (xl_tblspc_drop_rec *) XLogRecGetData(record);
+ if (!reachedConsistency)
+ XLogForgetMissingDir(xlrec->ts_id, InvalidOid);
+
+ /*
+ * Before we remove the tablespace directory, update minimum recovery
+ * point to cover this WAL record. Once the tablespace is removed,
+ * there's no going back. This manually enforces the WAL-first rule.
+ * Doing this before the removal means that if the removal fails for
+ * some reason, the directory is left alone and needs to be manually
+ * removed. Alternatively we could update the minimum recovery point
+ * after removal, but that would leave a small window where the
+ * WAL-first rule could be violated.
+ */
+ if (!reachedConsistency)
+ XLogFlush(record->EndRecPtr);
+
/*
* If we issued a WAL record for a drop tablespace it implies that
* there were no files in it at all when the DROP was done. That means
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 4105b59904..a17c204638 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -23,6 +23,10 @@ extern void XLogDropDatabase(Oid dbid);
extern void XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
BlockNumber nblocks);
+extern void XLogRememberMissingDir(Oid spcNode, Oid dbNode, char *path);
+extern void XLogForgetMissingDir(Oid spcNode, Oid dbNode);
+extern void XLogCheckMissingDirs(void);
+
/* Result codes for XLogReadBufferForRedo[Extended] */
typedef enum
{
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index daebb77387..bdf6b25d59 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3404,6 +3404,8 @@ xl_invalid_page
xl_invalid_page_key
xl_invalidations
xl_logical_message
+xl_missing_dir_key
+xl_missing_dir
xl_multi_insert_tuple
xl_multixact_create
xl_multixact_truncate
--
2.27.0
----Next_Part(Mon_Mar_28_17_20_42_2022_200)----
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v22] Fix replay of create database records on standby
@ 2022-04-05 06:31 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 0 replies; 18+ messages in thread
From: Kyotaro Horiguchi @ 2022-04-05 06:31 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch allows missing tablespaces to be created during recovery
before reaching consistency. The tablespaces are created as real
directories that should not exists but will be removed until reaching
consistency. CheckRecoveryConsistency is responsible to make sure they
have disappeared.
Similar to log_invalid_page mechanism, the GUC ignore_invalid_pages
turns into PANIC errors detected by this patch into WARNING, which
allows continueing recovery.
Diagnosed-by: Paul Guo <paulguo@gmail.com>
Author: Paul Guo <paulguo@gmail.com>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Author: Asim R Praveen <apraveen@pivotal.io>
Discussion: https://postgr.es/m/CAEET0ZGx9AvioViLf7nbR_8tH9-=27DN5xWJ2P9-ROH16e4JUA@mail.gmail.com
---
doc/src/sgml/config.sgml | 5 +-
src/backend/access/transam/xlogrecovery.c | 53 +++++++
src/backend/commands/dbcommands.c | 71 +++++++++
src/backend/commands/tablespace.c | 28 +---
src/backend/utils/misc/guc.c | 8 +-
src/include/access/xlogutils.h | 2 +
src/test/recovery/t/029_replay_tsp_drops.pl | 155 ++++++++++++++++++++
7 files changed, 290 insertions(+), 32 deletions(-)
create mode 100644 src/test/recovery/t/029_replay_tsp_drops.pl
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 43e4ade83e..c22229468b 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -11228,11 +11228,12 @@ LOG: CleanUpLock: deleting: lock(0xb7acd844) id(24688,24696,0,0,0,1)
<listitem>
<para>
If set to <literal>off</literal> (the default), detection of
- WAL records having references to invalid pages during
+ WAL records having references to invalid pages or
+ WAL records resulting in invalid directory operations during
recovery causes <productname>PostgreSQL</productname> to
raise a PANIC-level error, aborting the recovery. Setting
<varname>ignore_invalid_pages</varname> to <literal>on</literal>
- causes the system to ignore invalid page references in WAL records
+ causes the system to ignore invalid actions caused by such WAL records
(but still report a warning), and continue the recovery.
This behavior may <emphasis>cause crashes, data loss,
propagate or hide corruption, or other serious problems</emphasis>.
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 8d2395dae2..0889ba4b47 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -1986,6 +1986,51 @@ xlogrecovery_redo(XLogReaderState *record, TimeLineID replayTLI)
}
}
+/*
+ * Makes sure that ./pg_tblspc directory doesn't contain a real directory.
+ *
+ * This is intended to be called after reaching consistency.
+ * ignore_invalid_pages=on turns into the PANIC error into WARNING so that
+ * recovery can continue.
+ *
+ * Note that it is the normal behavior when allow_in_place_tablespaces=on, but
+ * we don't bother caring that case since it is a developer-only setting.
+ */
+static void
+CheckTablespaceDirectory(void)
+{
+ char *tblspc_path = "./pg_tblspc";
+ DIR *dir;
+ struct dirent *de;
+
+ dir = AllocateDir(tblspc_path);
+ while ((de = ReadDir(dir, tblspc_path)) != NULL)
+ {
+ char path[MAXPGPATH];
+ char *p;
+ struct stat st;
+
+ /* Skip entries of non-oid names */
+ for (p = de->d_name; *p && isdigit(*p); p++);
+ if (*p)
+ continue;
+
+ snprintf(path, MAXPGPATH, "%s/%s", tblspc_path, de->d_name);
+
+#ifndef WIN32
+ if (lstat(path, &st) < 0)
+ ereport(ERROR, errcode_for_file_access(),
+ errmsg("could not stat file \"%s\": %m", path));
+
+ if (!S_ISLNK(st.st_mode))
+#else
+ if (!pgwin32_is_junction(path))
+#endif
+ elog(ignore_invalid_pages ? WARNING : PANIC,
+ "real directory found in pg_tblspc directory: %s", de->d_name);
+ }
+}
+
/*
* Checks if recovery has reached a consistent state. When consistency is
* reached and we have a valid starting standby snapshot, tell postmaster
@@ -2051,6 +2096,14 @@ CheckRecoveryConsistency(void)
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
LSN_FORMAT_ARGS(lastReplayedEndRecPtr))));
+
+ /*
+ * Check that pg_tblspc doesn't contain a real
+ * directory. Database/CREATE_* records may create a tablespace
+ * directory that should have been removed until consistency is
+ * reached.
+ */
+ CheckTablespaceDirectory();
}
/*
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index df16533901..910101da01 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -30,6 +30,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -47,6 +48,7 @@
#include "commands/defrem.h"
#include "commands/seclabel.h"
#include "commands/tablespace.h"
+#include "common/file_perm.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "pgstat.h"
@@ -62,6 +64,7 @@
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
+#include "utils/guc.h"
#include "utils/pg_locale.h"
#include "utils/relmapper.h"
#include "utils/snapmgr.h"
@@ -135,6 +138,7 @@ static void CreateDirAndVersionFile(char *dbpath, Oid dbid, Oid tsid,
bool isRedo);
static void CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dboid, Oid src_tsid,
Oid dst_tsid);
+static void maybe_create_directory(char *path);
/*
* Create a new database using the WAL_LOG strategy.
@@ -3003,6 +3007,43 @@ get_database_name(Oid dbid)
return result;
}
+/*
+ * maybe_create_directory()
+ *
+ * During recovery, there's a case where we validly need to recover a missing
+ * tablespace directory so that recovery can continue. This happens when
+ * recovery wants to create a database but the holding tablespace has been
+ * removed before the server stopped. Since we expect that the directory will
+ * be gone before reaching recovery consistency, and we have no knowledge about
+ * the tablespace other than its OID here, we create a real directory under
+ * pg_tblspc here instead of restoring the symlink. ignore_invalid_pages=on
+ * reduces the error level so that recovery can continue.
+ */
+static void
+maybe_create_directory(char *path)
+{
+ struct stat st;
+
+ Assert(RecoveryInProgress());
+
+ if (stat(path, &st) == 0)
+ return;
+
+ /* XXX: Do we make sure that the path is under pg_tblspc? */
+
+ if (reachedConsistency && !ignore_invalid_pages)
+ ereport(PANIC,
+ errmsg("missing directory \"%s\"", path));
+
+ elog(reachedConsistency ? WARNING : DEBUG1,
+ "creating missing directory: %s", path);
+
+ if (pg_mkdir_p(path, pg_dir_create_mode) != 0)
+ ereport(PANIC,
+ errmsg("could not create missing directory \"%s\": %m", path));
+}
+
+
/*
* DATABASE resource manager's routines
*/
@@ -3039,6 +3080,30 @@ dbase_redo(XLogReaderState *record)
dst_path)));
}
+ if (stat(dst_path, &st) < 0)
+ {
+ char *parent_path;
+
+ if (errno != ENOENT)
+ ereport(FATAL,
+ errmsg("could not stat directory \"%s\": %m",
+ dst_path));
+
+ /* create the parent directory if needed and valid */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ maybe_create_directory(parent_path);
+ }
+
+ /*
+ * There's a case where the copy source directory is missing for the
+ * same reason above. Create the emtpy source directory so that
+ * copydir below doesn't fail. The directory will be dropped soon by
+ * recovery.
+ */
+ if (stat(src_path, &st) < 0 && errno == ENOENT)
+ maybe_create_directory(src_path);
+
/*
* Force dirty buffers out to disk, to ensure source database is
* up-to-date for the copy.
@@ -3057,9 +3122,15 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_wal_log_rec *xlrec =
(xl_dbase_create_wal_log_rec *) XLogRecGetData(record);
char *dbpath;
+ char *parent_path;
dbpath = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
+ /* create the parent directory if needed and valid */
+ parent_path = pstrdup(dbpath);
+ get_parent_directory(parent_path);
+ maybe_create_directory(parent_path);
+
/* Create the database directory with the version file. */
CreateDirAndVersionFile(dbpath, xlrec->db_id, xlrec->tablespace_id,
true);
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 40514ab550..675f578dfe 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -155,8 +155,6 @@ TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo)
/* Directory creation failed? */
if (MakePGDirectory(dir) < 0)
{
- char *parentdir;
-
/* Failure other than not exists or not in WAL replay? */
if (errno != ENOENT || !isRedo)
ereport(ERROR,
@@ -169,32 +167,8 @@ TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo)
* continue by creating simple parent directories rather
* than a symlink.
*/
-
- /* create two parents up if not exist */
- parentdir = pstrdup(dir);
- get_parent_directory(parentdir);
- get_parent_directory(parentdir);
- /* Can't create parent and it doesn't already exist? */
- if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not create directory \"%s\": %m",
- parentdir)));
- pfree(parentdir);
-
- /* create one parent up if not exist */
- parentdir = pstrdup(dir);
- get_parent_directory(parentdir);
- /* Can't create parent and it doesn't already exist? */
- if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not create directory \"%s\": %m",
- parentdir)));
- pfree(parentdir);
-
/* Create database directory */
- if (MakePGDirectory(dir) < 0)
+ if (pg_mkdir_p(dir, pg_dir_create_mode) < 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create directory \"%s\": %m",
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 9e8ab1420d..9134a73d3d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -42,6 +42,7 @@
#include "access/xact.h"
#include "access/xlog_internal.h"
#include "access/xlogrecovery.h"
+#include "access/xlogutils.h"
#include "catalog/namespace.h"
#include "catalog/objectaccess.h"
#include "catalog/pg_authid.h"
@@ -139,7 +140,6 @@ extern int CommitSiblings;
extern char *default_tablespace;
extern char *temp_tablespaces;
extern bool ignore_checksum_failure;
-extern bool ignore_invalid_pages;
extern bool synchronize_seqscans;
#ifdef TRACE_SYNCSCAN
@@ -1304,10 +1304,12 @@ static struct config_bool ConfigureNamesBool[] =
{"ignore_invalid_pages", PGC_POSTMASTER, DEVELOPER_OPTIONS,
gettext_noop("Continues recovery after an invalid pages failure."),
gettext_noop("Detection of WAL records having references to "
- "invalid pages during recovery causes PostgreSQL to "
+ "invalid pages or WAL records resulting in invalid "
+ "directory operations during "
+ "recovery that cause PostgreSQL"
"raise a PANIC-level error, aborting the recovery. "
"Setting ignore_invalid_pages to true causes "
- "the system to ignore invalid page references "
+ "the system to ignore those inconsistencies "
"in WAL records (but still report a warning), "
"and continue recovery. This behavior may cause "
"crashes, data loss, propagate or hide corruption, "
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 64708949db..d88661997f 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -54,6 +54,8 @@ typedef enum
extern HotStandbyState standbyState;
+extern bool ignore_invalid_pages;
+
#define InHotStandby (standbyState >= STANDBY_SNAPSHOT_PENDING)
diff --git a/src/test/recovery/t/029_replay_tsp_drops.pl b/src/test/recovery/t/029_replay_tsp_drops.pl
new file mode 100644
index 0000000000..b401ab8072
--- /dev/null
+++ b/src/test/recovery/t/029_replay_tsp_drops.pl
@@ -0,0 +1,155 @@
+
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+
+#
+# Tests relating to PostgreSQL crash recovery and redo
+#
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+sub test_tablespace
+{
+ my ($strategy) = @_;
+
+ my $node_primary = PostgreSQL::Test::Cluster->new("primary1_$strategy");
+ $node_primary->init(allows_streaming => 1);
+ $node_primary->start;
+ $node_primary->psql('postgres',
+ qq[
+ SET allow_in_place_tablespaces=on;
+ CREATE TABLESPACE dropme_ts1 LOCATION '';
+ CREATE TABLESPACE dropme_ts2 LOCATION '';
+ CREATE TABLESPACE source_ts LOCATION '';
+ CREATE TABLESPACE target_ts LOCATION '';
+ CREATE DATABASE template_db IS_TEMPLATE = true;
+ ]);
+ my $backup_name = 'my_backup';
+ $node_primary->backup($backup_name);
+
+ my $node_standby = PostgreSQL::Test::Cluster->new("standby2_$strategy");
+ $node_standby->init_from_backup($node_primary, $backup_name, has_streaming => 1);
+ $node_standby->append_conf('postgresql.conf', "ignore_invalid_pages = on");
+ $node_standby->start;
+
+ # Make sure connection is made
+ $node_primary->poll_query_until(
+ 'postgres', 'SELECT count(*) = 1 FROM pg_stat_replication');
+
+ $node_standby->safe_psql('postgres', 'CHECKPOINT');
+
+ # Do immediate shutdown just after a sequence of CREAT DATABASE / DROP
+ # DATABASE / DROP TABLESPACE. This causes CREATE DATABASE WAL records
+ # to be applied to already-removed directories.
+ my $query = q[
+ CREATE DATABASE dropme_db1 WITH TABLESPACE dropme_ts1 STRATEGY=<STRATEGY>;
+ CREATE TABLE t (a int) TABLESPACE dropme_ts2;
+ CREATE DATABASE dropme_db2 WITH TABLESPACE dropme_ts2 STRATEGY=<STRATEGY>;
+ CREATE DATABASE moveme_db TABLESPACE source_ts STRATEGY=<STRATEGY>;
+ ALTER DATABASE moveme_db SET TABLESPACE target_ts;
+ CREATE DATABASE newdb TEMPLATE template_db STRATEGY=<STRATEGY>;
+ ALTER DATABASE template_db IS_TEMPLATE = false;
+ DROP DATABASE dropme_db1;
+ DROP TABLE t;
+ DROP DATABASE dropme_db2; DROP TABLESPACE dropme_ts2;
+ DROP TABLESPACE source_ts;
+ DROP DATABASE template_db;];
+
+ $query =~ s/<STRATEGY>/$strategy/g;
+ $node_primary->safe_psql('postgres', $query);
+ $node_primary->wait_for_catchup($node_standby, 'replay',
+ $node_primary->lsn('replay'));
+
+ # show "create missing directory" log message
+ $node_standby->safe_psql('postgres',
+ "ALTER SYSTEM SET log_min_messages TO debug1;");
+ $node_standby->stop('immediate');
+ # Should restart ignoring directory creation error.
+ is($node_standby->start(fail_ok => 1), 1);
+ $node_standby->stop('immediate');
+}
+
+test_tablespace("FILE_COPY");
+test_tablespace("WAL_LOG");
+
+# Ensure that a missing tablespace directory during create database
+# replay immediately causes panic if the standby has already reached
+# consistent state (archive recovery is in progress). This is
+# effective only for CREATE DATABASE WITH STRATEGY=FILE_COPY.
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary2');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+
+# Create tablespace
+$node_primary->safe_psql('postgres', q[
+ SET allow_in_place_tablespaces=on;
+ CREATE TABLESPACE ts1 LOCATION '']);
+$node_primary->safe_psql('postgres', "CREATE DATABASE db1 WITH TABLESPACE ts1 STRATEGY=FILE_COPY");
+
+# Take backup
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+my $node_standby = PostgreSQL::Test::Cluster->new('standby3');
+$node_standby->init_from_backup($node_primary, $backup_name, has_streaming => 1);
+$node_standby->append_conf('postgresql.conf', "ignore_invalid_pages = on");
+$node_standby->start;
+
+# Make sure standby reached consistency and starts accepting connections
+$node_standby->poll_query_until('postgres', 'SELECT 1', '1');
+
+# Remove standby tablespace directory so it will be missing when
+# replay resumes.
+my $tspoid = $node_standby->safe_psql('postgres',
+ "SELECT oid FROM pg_tablespace WHERE spcname = 'ts1';");
+my $tspdir = $node_standby->data_dir . "/pg_tblspc/$tspoid";
+File::Path::rmtree($tspdir);
+
+my $logstart = get_log_size($node_standby);
+
+# Create a database in the tablespace and a table in default tablespace
+$node_primary->safe_psql('postgres',
+ q[CREATE TABLE should_not_replay_insertion(a int);
+ CREATE DATABASE db2 WITH TABLESPACE ts1 STRATEGY=FILE_COPY;
+ INSERT INTO should_not_replay_insertion VALUES (1);]);
+
+# Standby should fail and should not silently skip replaying the wal
+# In this test, PANIC turns into WARNING by ignore_invalid_pages.
+# Check the log messages instead of confirming standby failure.
+my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
+while ($max_attempts-- >= 0)
+{
+ last if (find_in_log(
+ $node_standby,
+ "WARNING: creating missing directory: pg_tblspc/",
+ $logstart));
+ sleep 1;
+}
+ok($max_attempts > 0, "invalid directory creation is detected");
+
+done_testing();
+
+
+# return the size of logfile of $node in bytes
+sub get_log_size
+{
+ my ($node) = @_;
+
+ return (stat $node->logfile)[7];
+}
+
+# find $pat in logfile of $node after $off-th byte
+sub find_in_log
+{
+ my ($node, $pat, $off) = @_;
+
+ $off = 0 unless defined $off;
+ my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
+ return 0 if (length($log) <= $off);
+
+ $log = substr($log, $off);
+
+ return $log =~ m/$pat/;
+}
--
2.27.0
----Next_Part(Tue_Apr__5_16_38_06_2022_540)----
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v23] Fix replay of create database records on standby
@ 2022-04-05 06:31 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 0 replies; 18+ messages in thread
From: Kyotaro Horiguchi @ 2022-04-05 06:31 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch allows missing tablespaces to be created during recovery
before reaching consistency. The tablespaces are created as real
directories that should not exists but will be removed until reaching
consistency. CheckRecoveryConsistency is responsible to make sure they
have disappeared.
Similar to log_invalid_page mechanism, the GUC ignore_invalid_pages
turns into PANIC errors detected by this patch into WARNING, which
allows continueing recovery.
Diagnosed-by: Paul Guo <paulguo@gmail.com>
Author: Paul Guo <paulguo@gmail.com>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Author: Asim R Praveen <apraveen@pivotal.io>
Discussion: https://postgr.es/m/CAEET0ZGx9AvioViLf7nbR_8tH9-=27DN5xWJ2P9-ROH16e4JUA@mail.gmail.com
---
doc/src/sgml/config.sgml | 5 +-
src/backend/access/transam/xlogrecovery.c | 55 +++++++
src/backend/commands/dbcommands.c | 71 +++++++++
src/backend/commands/tablespace.c | 28 +---
src/backend/utils/misc/guc.c | 8 +-
src/include/access/xlogutils.h | 2 +
src/test/recovery/t/029_replay_tsp_drops.pl | 155 ++++++++++++++++++++
7 files changed, 292 insertions(+), 32 deletions(-)
create mode 100644 src/test/recovery/t/029_replay_tsp_drops.pl
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 43e4ade83e..c22229468b 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -11228,11 +11228,12 @@ LOG: CleanUpLock: deleting: lock(0xb7acd844) id(24688,24696,0,0,0,1)
<listitem>
<para>
If set to <literal>off</literal> (the default), detection of
- WAL records having references to invalid pages during
+ WAL records having references to invalid pages or
+ WAL records resulting in invalid directory operations during
recovery causes <productname>PostgreSQL</productname> to
raise a PANIC-level error, aborting the recovery. Setting
<varname>ignore_invalid_pages</varname> to <literal>on</literal>
- causes the system to ignore invalid page references in WAL records
+ causes the system to ignore invalid actions caused by such WAL records
(but still report a warning), and continue the recovery.
This behavior may <emphasis>cause crashes, data loss,
propagate or hide corruption, or other serious problems</emphasis>.
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 8d2395dae2..18dcc452ca 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -1986,6 +1986,53 @@ xlogrecovery_redo(XLogReaderState *record, TimeLineID replayTLI)
}
}
+/*
+ * Makes sure that ./pg_tblspc directory doesn't contain a real directory.
+ *
+ * This is intended to be called after reaching consistency.
+ * ignore_invalid_pages=on turns into the PANIC error into WARNING so that
+ * recovery can continue.
+ *
+ * Note that it is the normal behavior when allow_in_place_tablespaces=on, but
+ * we don't bother caring that case since it is a developer-only setting.
+ */
+static void
+CheckTablespaceDirectory(void)
+{
+ char *tblspc_path = "./pg_tblspc";
+ DIR *dir;
+ struct dirent *de;
+
+ dir = AllocateDir(tblspc_path);
+ while ((de = ReadDir(dir, tblspc_path)) != NULL)
+ {
+ char path[MAXPGPATH];
+ char *p;
+#ifndef WIN32
+ struct stat st;
+#endif
+
+ /* Skip entries of non-oid names */
+ for (p = de->d_name; *p && isdigit(*p); p++);
+ if (*p)
+ continue;
+
+ snprintf(path, MAXPGPATH, "%s/%s", tblspc_path, de->d_name);
+
+#ifndef WIN32
+ if (lstat(path, &st) < 0)
+ ereport(ERROR, errcode_for_file_access(),
+ errmsg("could not stat file \"%s\": %m", path));
+
+ if (!S_ISLNK(st.st_mode))
+#else
+ if (!pgwin32_is_junction(path))
+#endif
+ elog(ignore_invalid_pages ? WARNING : PANIC,
+ "real directory found in pg_tblspc directory: %s", de->d_name);
+ }
+}
+
/*
* Checks if recovery has reached a consistent state. When consistency is
* reached and we have a valid starting standby snapshot, tell postmaster
@@ -2051,6 +2098,14 @@ CheckRecoveryConsistency(void)
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
LSN_FORMAT_ARGS(lastReplayedEndRecPtr))));
+
+ /*
+ * Check that pg_tblspc doesn't contain a real
+ * directory. Database/CREATE_* records may create a tablespace
+ * directory that should have been removed until consistency is
+ * reached.
+ */
+ CheckTablespaceDirectory();
}
/*
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index df16533901..910101da01 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -30,6 +30,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -47,6 +48,7 @@
#include "commands/defrem.h"
#include "commands/seclabel.h"
#include "commands/tablespace.h"
+#include "common/file_perm.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "pgstat.h"
@@ -62,6 +64,7 @@
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
+#include "utils/guc.h"
#include "utils/pg_locale.h"
#include "utils/relmapper.h"
#include "utils/snapmgr.h"
@@ -135,6 +138,7 @@ static void CreateDirAndVersionFile(char *dbpath, Oid dbid, Oid tsid,
bool isRedo);
static void CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dboid, Oid src_tsid,
Oid dst_tsid);
+static void maybe_create_directory(char *path);
/*
* Create a new database using the WAL_LOG strategy.
@@ -3003,6 +3007,43 @@ get_database_name(Oid dbid)
return result;
}
+/*
+ * maybe_create_directory()
+ *
+ * During recovery, there's a case where we validly need to recover a missing
+ * tablespace directory so that recovery can continue. This happens when
+ * recovery wants to create a database but the holding tablespace has been
+ * removed before the server stopped. Since we expect that the directory will
+ * be gone before reaching recovery consistency, and we have no knowledge about
+ * the tablespace other than its OID here, we create a real directory under
+ * pg_tblspc here instead of restoring the symlink. ignore_invalid_pages=on
+ * reduces the error level so that recovery can continue.
+ */
+static void
+maybe_create_directory(char *path)
+{
+ struct stat st;
+
+ Assert(RecoveryInProgress());
+
+ if (stat(path, &st) == 0)
+ return;
+
+ /* XXX: Do we make sure that the path is under pg_tblspc? */
+
+ if (reachedConsistency && !ignore_invalid_pages)
+ ereport(PANIC,
+ errmsg("missing directory \"%s\"", path));
+
+ elog(reachedConsistency ? WARNING : DEBUG1,
+ "creating missing directory: %s", path);
+
+ if (pg_mkdir_p(path, pg_dir_create_mode) != 0)
+ ereport(PANIC,
+ errmsg("could not create missing directory \"%s\": %m", path));
+}
+
+
/*
* DATABASE resource manager's routines
*/
@@ -3039,6 +3080,30 @@ dbase_redo(XLogReaderState *record)
dst_path)));
}
+ if (stat(dst_path, &st) < 0)
+ {
+ char *parent_path;
+
+ if (errno != ENOENT)
+ ereport(FATAL,
+ errmsg("could not stat directory \"%s\": %m",
+ dst_path));
+
+ /* create the parent directory if needed and valid */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ maybe_create_directory(parent_path);
+ }
+
+ /*
+ * There's a case where the copy source directory is missing for the
+ * same reason above. Create the emtpy source directory so that
+ * copydir below doesn't fail. The directory will be dropped soon by
+ * recovery.
+ */
+ if (stat(src_path, &st) < 0 && errno == ENOENT)
+ maybe_create_directory(src_path);
+
/*
* Force dirty buffers out to disk, to ensure source database is
* up-to-date for the copy.
@@ -3057,9 +3122,15 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_wal_log_rec *xlrec =
(xl_dbase_create_wal_log_rec *) XLogRecGetData(record);
char *dbpath;
+ char *parent_path;
dbpath = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
+ /* create the parent directory if needed and valid */
+ parent_path = pstrdup(dbpath);
+ get_parent_directory(parent_path);
+ maybe_create_directory(parent_path);
+
/* Create the database directory with the version file. */
CreateDirAndVersionFile(dbpath, xlrec->db_id, xlrec->tablespace_id,
true);
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 40514ab550..675f578dfe 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -155,8 +155,6 @@ TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo)
/* Directory creation failed? */
if (MakePGDirectory(dir) < 0)
{
- char *parentdir;
-
/* Failure other than not exists or not in WAL replay? */
if (errno != ENOENT || !isRedo)
ereport(ERROR,
@@ -169,32 +167,8 @@ TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo)
* continue by creating simple parent directories rather
* than a symlink.
*/
-
- /* create two parents up if not exist */
- parentdir = pstrdup(dir);
- get_parent_directory(parentdir);
- get_parent_directory(parentdir);
- /* Can't create parent and it doesn't already exist? */
- if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not create directory \"%s\": %m",
- parentdir)));
- pfree(parentdir);
-
- /* create one parent up if not exist */
- parentdir = pstrdup(dir);
- get_parent_directory(parentdir);
- /* Can't create parent and it doesn't already exist? */
- if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not create directory \"%s\": %m",
- parentdir)));
- pfree(parentdir);
-
/* Create database directory */
- if (MakePGDirectory(dir) < 0)
+ if (pg_mkdir_p(dir, pg_dir_create_mode) < 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create directory \"%s\": %m",
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 9e8ab1420d..9134a73d3d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -42,6 +42,7 @@
#include "access/xact.h"
#include "access/xlog_internal.h"
#include "access/xlogrecovery.h"
+#include "access/xlogutils.h"
#include "catalog/namespace.h"
#include "catalog/objectaccess.h"
#include "catalog/pg_authid.h"
@@ -139,7 +140,6 @@ extern int CommitSiblings;
extern char *default_tablespace;
extern char *temp_tablespaces;
extern bool ignore_checksum_failure;
-extern bool ignore_invalid_pages;
extern bool synchronize_seqscans;
#ifdef TRACE_SYNCSCAN
@@ -1304,10 +1304,12 @@ static struct config_bool ConfigureNamesBool[] =
{"ignore_invalid_pages", PGC_POSTMASTER, DEVELOPER_OPTIONS,
gettext_noop("Continues recovery after an invalid pages failure."),
gettext_noop("Detection of WAL records having references to "
- "invalid pages during recovery causes PostgreSQL to "
+ "invalid pages or WAL records resulting in invalid "
+ "directory operations during "
+ "recovery that cause PostgreSQL"
"raise a PANIC-level error, aborting the recovery. "
"Setting ignore_invalid_pages to true causes "
- "the system to ignore invalid page references "
+ "the system to ignore those inconsistencies "
"in WAL records (but still report a warning), "
"and continue recovery. This behavior may cause "
"crashes, data loss, propagate or hide corruption, "
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 64708949db..d88661997f 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -54,6 +54,8 @@ typedef enum
extern HotStandbyState standbyState;
+extern bool ignore_invalid_pages;
+
#define InHotStandby (standbyState >= STANDBY_SNAPSHOT_PENDING)
diff --git a/src/test/recovery/t/029_replay_tsp_drops.pl b/src/test/recovery/t/029_replay_tsp_drops.pl
new file mode 100644
index 0000000000..b401ab8072
--- /dev/null
+++ b/src/test/recovery/t/029_replay_tsp_drops.pl
@@ -0,0 +1,155 @@
+
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+
+#
+# Tests relating to PostgreSQL crash recovery and redo
+#
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+sub test_tablespace
+{
+ my ($strategy) = @_;
+
+ my $node_primary = PostgreSQL::Test::Cluster->new("primary1_$strategy");
+ $node_primary->init(allows_streaming => 1);
+ $node_primary->start;
+ $node_primary->psql('postgres',
+ qq[
+ SET allow_in_place_tablespaces=on;
+ CREATE TABLESPACE dropme_ts1 LOCATION '';
+ CREATE TABLESPACE dropme_ts2 LOCATION '';
+ CREATE TABLESPACE source_ts LOCATION '';
+ CREATE TABLESPACE target_ts LOCATION '';
+ CREATE DATABASE template_db IS_TEMPLATE = true;
+ ]);
+ my $backup_name = 'my_backup';
+ $node_primary->backup($backup_name);
+
+ my $node_standby = PostgreSQL::Test::Cluster->new("standby2_$strategy");
+ $node_standby->init_from_backup($node_primary, $backup_name, has_streaming => 1);
+ $node_standby->append_conf('postgresql.conf', "ignore_invalid_pages = on");
+ $node_standby->start;
+
+ # Make sure connection is made
+ $node_primary->poll_query_until(
+ 'postgres', 'SELECT count(*) = 1 FROM pg_stat_replication');
+
+ $node_standby->safe_psql('postgres', 'CHECKPOINT');
+
+ # Do immediate shutdown just after a sequence of CREAT DATABASE / DROP
+ # DATABASE / DROP TABLESPACE. This causes CREATE DATABASE WAL records
+ # to be applied to already-removed directories.
+ my $query = q[
+ CREATE DATABASE dropme_db1 WITH TABLESPACE dropme_ts1 STRATEGY=<STRATEGY>;
+ CREATE TABLE t (a int) TABLESPACE dropme_ts2;
+ CREATE DATABASE dropme_db2 WITH TABLESPACE dropme_ts2 STRATEGY=<STRATEGY>;
+ CREATE DATABASE moveme_db TABLESPACE source_ts STRATEGY=<STRATEGY>;
+ ALTER DATABASE moveme_db SET TABLESPACE target_ts;
+ CREATE DATABASE newdb TEMPLATE template_db STRATEGY=<STRATEGY>;
+ ALTER DATABASE template_db IS_TEMPLATE = false;
+ DROP DATABASE dropme_db1;
+ DROP TABLE t;
+ DROP DATABASE dropme_db2; DROP TABLESPACE dropme_ts2;
+ DROP TABLESPACE source_ts;
+ DROP DATABASE template_db;];
+
+ $query =~ s/<STRATEGY>/$strategy/g;
+ $node_primary->safe_psql('postgres', $query);
+ $node_primary->wait_for_catchup($node_standby, 'replay',
+ $node_primary->lsn('replay'));
+
+ # show "create missing directory" log message
+ $node_standby->safe_psql('postgres',
+ "ALTER SYSTEM SET log_min_messages TO debug1;");
+ $node_standby->stop('immediate');
+ # Should restart ignoring directory creation error.
+ is($node_standby->start(fail_ok => 1), 1);
+ $node_standby->stop('immediate');
+}
+
+test_tablespace("FILE_COPY");
+test_tablespace("WAL_LOG");
+
+# Ensure that a missing tablespace directory during create database
+# replay immediately causes panic if the standby has already reached
+# consistent state (archive recovery is in progress). This is
+# effective only for CREATE DATABASE WITH STRATEGY=FILE_COPY.
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary2');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+
+# Create tablespace
+$node_primary->safe_psql('postgres', q[
+ SET allow_in_place_tablespaces=on;
+ CREATE TABLESPACE ts1 LOCATION '']);
+$node_primary->safe_psql('postgres', "CREATE DATABASE db1 WITH TABLESPACE ts1 STRATEGY=FILE_COPY");
+
+# Take backup
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+my $node_standby = PostgreSQL::Test::Cluster->new('standby3');
+$node_standby->init_from_backup($node_primary, $backup_name, has_streaming => 1);
+$node_standby->append_conf('postgresql.conf', "ignore_invalid_pages = on");
+$node_standby->start;
+
+# Make sure standby reached consistency and starts accepting connections
+$node_standby->poll_query_until('postgres', 'SELECT 1', '1');
+
+# Remove standby tablespace directory so it will be missing when
+# replay resumes.
+my $tspoid = $node_standby->safe_psql('postgres',
+ "SELECT oid FROM pg_tablespace WHERE spcname = 'ts1';");
+my $tspdir = $node_standby->data_dir . "/pg_tblspc/$tspoid";
+File::Path::rmtree($tspdir);
+
+my $logstart = get_log_size($node_standby);
+
+# Create a database in the tablespace and a table in default tablespace
+$node_primary->safe_psql('postgres',
+ q[CREATE TABLE should_not_replay_insertion(a int);
+ CREATE DATABASE db2 WITH TABLESPACE ts1 STRATEGY=FILE_COPY;
+ INSERT INTO should_not_replay_insertion VALUES (1);]);
+
+# Standby should fail and should not silently skip replaying the wal
+# In this test, PANIC turns into WARNING by ignore_invalid_pages.
+# Check the log messages instead of confirming standby failure.
+my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
+while ($max_attempts-- >= 0)
+{
+ last if (find_in_log(
+ $node_standby,
+ "WARNING: creating missing directory: pg_tblspc/",
+ $logstart));
+ sleep 1;
+}
+ok($max_attempts > 0, "invalid directory creation is detected");
+
+done_testing();
+
+
+# return the size of logfile of $node in bytes
+sub get_log_size
+{
+ my ($node) = @_;
+
+ return (stat $node->logfile)[7];
+}
+
+# find $pat in logfile of $node after $off-th byte
+sub find_in_log
+{
+ my ($node, $pat, $off) = @_;
+
+ $off = 0 unless defined $off;
+ my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
+ return 0 if (length($log) <= $off);
+
+ $log = substr($log, $off);
+
+ return $log =~ m/$pat/;
+}
--
2.27.0
----Next_Part(Tue_Apr__5_17_18_57_2022_390)----
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v25 1/4] Fix replay of create database records on standby
@ 2022-07-13 16:14 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 18+ messages in thread
From: Alvaro Herrera @ 2022-07-13 16:14 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch allows missing tablespaces to be created during recovery
before reaching consistency. The tablespaces are created as real
directories that should not exists but will be removed until reaching
consistency. CheckRecoveryConsistency is responsible to make sure they
have disappeared.
Similar to log_invalid_page mechanism, the GUC ignore_invalid_pages
turns into PANIC errors detected by this patch into WARNING, which
allows continueing recovery.
Diagnosed-by: Paul Guo <paulguo@gmail.com>
Author: Paul Guo <paulguo@gmail.com>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Author: Asim R Praveen <apraveen@pivotal.io>
Discussion: https://postgr.es/m/CAEET0ZGx9AvioViLf7nbR_8tH9-=27DN5xWJ2P9-ROH16e4JUA@mail.gmail.com
---
doc/src/sgml/config.sgml | 5 +-
src/backend/access/transam/xlogrecovery.c | 59 ++++++++
src/backend/commands/dbcommands.c | 71 +++++++++
src/backend/commands/tablespace.c | 28 +---
src/backend/utils/misc/guc.c | 8 +-
src/include/access/xlogutils.h | 2 +
src/test/recovery/t/029_replay_tsp_drops.pl | 155 ++++++++++++++++++++
7 files changed, 296 insertions(+), 32 deletions(-)
create mode 100644 src/test/recovery/t/029_replay_tsp_drops.pl
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 37fd80388c..1e1c8c1cb7 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -11363,11 +11363,12 @@ LOG: CleanUpLock: deleting: lock(0xb7acd844) id(24688,24696,0,0,0,1)
<listitem>
<para>
If set to <literal>off</literal> (the default), detection of
- WAL records having references to invalid pages during
+ WAL records having references to invalid pages or
+ WAL records resulting in invalid directory operations during
recovery causes <productname>PostgreSQL</productname> to
raise a PANIC-level error, aborting the recovery. Setting
<varname>ignore_invalid_pages</varname> to <literal>on</literal>
- causes the system to ignore invalid page references in WAL records
+ causes the system to ignore invalid actions caused by such WAL records
(but still report a warning), and continue the recovery.
This behavior may <emphasis>cause crashes, data loss,
propagate or hide corruption, or other serious problems</emphasis>.
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 5d6f1b5e46..ae81244e06 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -2008,6 +2008,57 @@ xlogrecovery_redo(XLogReaderState *record, TimeLineID replayTLI)
}
}
+/*
+ * Makes sure that ./pg_tblspc directory doesn't contain a real directory.
+ *
+ * This is intended to be called after reaching consistency.
+ * ignore_invalid_pages=on turns into the PANIC error into WARNING so that
+ * recovery can continue.
+ *
+ * This can't be checked in allow_in_place_tablespaces mode, so skip it in
+ * that case.
+ */
+static void
+CheckTablespaceDirectory(void)
+{
+ char *tblspc_path = "./pg_tblspc";
+ DIR *dir;
+ struct dirent *de;
+
+ /* Do not check for this when test tablespaces are in use */
+ if (allow_in_place_tablespaces)
+ return;
+
+ dir = AllocateDir(tblspc_path);
+ while ((de = ReadDir(dir, tblspc_path)) != NULL)
+ {
+ char path[MAXPGPATH];
+ char *p;
+#ifndef WIN32
+ struct stat st;
+#endif
+
+ /* Skip entries of non-oid names */
+ for (p = de->d_name; *p && isdigit(*p); p++);
+ if (*p)
+ continue;
+
+ snprintf(path, MAXPGPATH, "%s/%s", tblspc_path, de->d_name);
+
+#ifndef WIN32
+ if (lstat(path, &st) < 0)
+ ereport(ERROR, errcode_for_file_access(),
+ errmsg("could not stat file \"%s\": %m", path));
+
+ if (!S_ISLNK(st.st_mode))
+#else
+ if (!pgwin32_is_junction(path))
+#endif
+ elog(ignore_invalid_pages ? WARNING : PANIC,
+ "real directory found in pg_tblspc directory: %s", de->d_name);
+ }
+}
+
/*
* Checks if recovery has reached a consistent state. When consistency is
* reached and we have a valid starting standby snapshot, tell postmaster
@@ -2072,6 +2123,14 @@ CheckRecoveryConsistency(void)
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
LSN_FORMAT_ARGS(lastReplayedEndRecPtr))));
+
+ /*
+ * Check that pg_tblspc doesn't contain a real
+ * directory. Database/CREATE_* records may create a tablespace
+ * directory that should have been removed until consistency is
+ * reached.
+ */
+ CheckTablespaceDirectory();
}
/*
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 1901b434c5..0f860030fa 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -30,6 +30,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -47,6 +48,7 @@
#include "commands/defrem.h"
#include "commands/seclabel.h"
#include "commands/tablespace.h"
+#include "common/file_perm.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "pgstat.h"
@@ -62,6 +64,7 @@
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
+#include "utils/guc.h"
#include "utils/pg_locale.h"
#include "utils/relmapper.h"
#include "utils/snapmgr.h"
@@ -135,6 +138,7 @@ static void CreateDirAndVersionFile(char *dbpath, Oid dbid, Oid tsid,
bool isRedo);
static void CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dboid, Oid src_tsid,
Oid dst_tsid);
+static void maybe_create_directory(char *path);
/*
* Create a new database using the WAL_LOG strategy.
@@ -3008,6 +3012,43 @@ get_database_name(Oid dbid)
return result;
}
+/*
+ * maybe_create_directory()
+ *
+ * During recovery, there's a case where we validly need to recover a missing
+ * tablespace directory so that recovery can continue. This happens when
+ * recovery wants to create a database but the holding tablespace has been
+ * removed before the server stopped. Since we expect that the directory will
+ * be gone before reaching recovery consistency, and we have no knowledge about
+ * the tablespace other than its OID here, we create a real directory under
+ * pg_tblspc here instead of restoring the symlink. ignore_invalid_pages=on
+ * reduces the error level so that recovery can continue.
+ */
+static void
+maybe_create_directory(char *path)
+{
+ struct stat st;
+
+ Assert(RecoveryInProgress());
+
+ if (stat(path, &st) == 0)
+ return;
+
+ /* XXX: Do we make sure that the path is under pg_tblspc? */
+
+ if (reachedConsistency && !ignore_invalid_pages)
+ ereport(PANIC,
+ errmsg("missing directory \"%s\"", path));
+
+ elog(reachedConsistency ? WARNING : DEBUG1,
+ "creating missing directory: %s", path);
+
+ if (pg_mkdir_p(path, pg_dir_create_mode) != 0)
+ ereport(PANIC,
+ errmsg("could not create missing directory \"%s\": %m", path));
+}
+
+
/*
* DATABASE resource manager's routines
*/
@@ -3044,6 +3085,30 @@ dbase_redo(XLogReaderState *record)
dst_path)));
}
+ if (stat(dst_path, &st) < 0)
+ {
+ char *parent_path;
+
+ if (errno != ENOENT)
+ ereport(FATAL,
+ errmsg("could not stat directory \"%s\": %m",
+ dst_path));
+
+ /* create the parent directory if needed and valid */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ maybe_create_directory(parent_path);
+ }
+
+ /*
+ * There's a case where the copy source directory is missing for the
+ * same reason above. Create the emtpy source directory so that
+ * copydir below doesn't fail. The directory will be dropped soon by
+ * recovery.
+ */
+ if (stat(src_path, &st) < 0 && errno == ENOENT)
+ maybe_create_directory(src_path);
+
/*
* Force dirty buffers out to disk, to ensure source database is
* up-to-date for the copy.
@@ -3068,9 +3133,15 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_wal_log_rec *xlrec =
(xl_dbase_create_wal_log_rec *) XLogRecGetData(record);
char *dbpath;
+ char *parent_path;
dbpath = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
+ /* create the parent directory if needed and valid */
+ parent_path = pstrdup(dbpath);
+ get_parent_directory(parent_path);
+ maybe_create_directory(parent_path);
+
/* Create the database directory with the version file. */
CreateDirAndVersionFile(dbpath, xlrec->db_id, xlrec->tablespace_id,
true);
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index c8bdd9992a..ddb031a83f 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -156,8 +156,6 @@ TablespaceCreateDbspace(Oid spcOid, Oid dbOid, bool isRedo)
/* Directory creation failed? */
if (MakePGDirectory(dir) < 0)
{
- char *parentdir;
-
/* Failure other than not exists or not in WAL replay? */
if (errno != ENOENT || !isRedo)
ereport(ERROR,
@@ -170,32 +168,8 @@ TablespaceCreateDbspace(Oid spcOid, Oid dbOid, bool isRedo)
* continue by creating simple parent directories rather
* than a symlink.
*/
-
- /* create two parents up if not exist */
- parentdir = pstrdup(dir);
- get_parent_directory(parentdir);
- get_parent_directory(parentdir);
- /* Can't create parent and it doesn't already exist? */
- if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not create directory \"%s\": %m",
- parentdir)));
- pfree(parentdir);
-
- /* create one parent up if not exist */
- parentdir = pstrdup(dir);
- get_parent_directory(parentdir);
- /* Can't create parent and it doesn't already exist? */
- if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not create directory \"%s\": %m",
- parentdir)));
- pfree(parentdir);
-
/* Create database directory */
- if (MakePGDirectory(dir) < 0)
+ if (pg_mkdir_p(dir, pg_dir_create_mode) < 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create directory \"%s\": %m",
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 0328029d43..cd6fa84e22 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -43,6 +43,7 @@
#include "access/xlog_internal.h"
#include "access/xlogprefetcher.h"
#include "access/xlogrecovery.h"
+#include "access/xlogutils.h"
#include "catalog/namespace.h"
#include "catalog/objectaccess.h"
#include "catalog/pg_authid.h"
@@ -141,7 +142,6 @@ extern int CommitSiblings;
extern char *default_tablespace;
extern char *temp_tablespaces;
extern bool ignore_checksum_failure;
-extern bool ignore_invalid_pages;
extern bool synchronize_seqscans;
#ifdef TRACE_SYNCSCAN
@@ -1336,10 +1336,12 @@ static struct config_bool ConfigureNamesBool[] =
{"ignore_invalid_pages", PGC_POSTMASTER, DEVELOPER_OPTIONS,
gettext_noop("Continues recovery after an invalid pages failure."),
gettext_noop("Detection of WAL records having references to "
- "invalid pages during recovery causes PostgreSQL to "
+ "invalid pages or WAL records resulting in invalid "
+ "directory operations during "
+ "recovery that cause PostgreSQL"
"raise a PANIC-level error, aborting the recovery. "
"Setting ignore_invalid_pages to true causes "
- "the system to ignore invalid page references "
+ "the system to ignore those inconsistencies "
"in WAL records (but still report a warning), "
"and continue recovery. This behavior may cause "
"crashes, data loss, propagate or hide corruption, "
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index ef182977bf..f203fdf539 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -54,6 +54,8 @@ typedef enum
extern PGDLLIMPORT HotStandbyState standbyState;
+extern bool ignore_invalid_pages;
+
#define InHotStandby (standbyState >= STANDBY_SNAPSHOT_PENDING)
diff --git a/src/test/recovery/t/029_replay_tsp_drops.pl b/src/test/recovery/t/029_replay_tsp_drops.pl
new file mode 100644
index 0000000000..d537fe0ce5
--- /dev/null
+++ b/src/test/recovery/t/029_replay_tsp_drops.pl
@@ -0,0 +1,155 @@
+
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+
+#
+# Tests relating to PostgreSQL crash recovery and redo
+#
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+sub test_tablespace
+{
+ my ($strategy) = @_;
+
+ my $node_primary = PostgreSQL::Test::Cluster->new("primary1_$strategy");
+ $node_primary->init(allows_streaming => 1);
+ $node_primary->start;
+ $node_primary->psql('postgres',
+ qq[
+ SET allow_in_place_tablespaces=on;
+ CREATE TABLESPACE dropme_ts1 LOCATION '';
+ CREATE TABLESPACE dropme_ts2 LOCATION '';
+ CREATE TABLESPACE source_ts LOCATION '';
+ CREATE TABLESPACE target_ts LOCATION '';
+ CREATE DATABASE template_db IS_TEMPLATE = true;
+ ]);
+ my $backup_name = 'my_backup';
+ $node_primary->backup($backup_name);
+
+ my $node_standby = PostgreSQL::Test::Cluster->new("standby2_$strategy");
+ $node_standby->init_from_backup($node_primary, $backup_name, has_streaming => 1);
+ $node_standby->append_conf('postgresql.conf', "ignore_invalid_pages = on");
+ $node_standby->start;
+
+ # Make sure connection is made
+ $node_primary->poll_query_until(
+ 'postgres', 'SELECT count(*) = 1 FROM pg_stat_replication');
+
+ $node_standby->safe_psql('postgres', 'CHECKPOINT');
+
+ # Do immediate shutdown just after a sequence of CREAT DATABASE / DROP
+ # DATABASE / DROP TABLESPACE. This causes CREATE DATABASE WAL records
+ # to be applied to already-removed directories.
+ my $query = q[
+ CREATE DATABASE dropme_db1 WITH TABLESPACE dropme_ts1 STRATEGY=<STRATEGY>;
+ CREATE TABLE t (a int) TABLESPACE dropme_ts2;
+ CREATE DATABASE dropme_db2 WITH TABLESPACE dropme_ts2 STRATEGY=<STRATEGY>;
+ CREATE DATABASE moveme_db TABLESPACE source_ts STRATEGY=<STRATEGY>;
+ ALTER DATABASE moveme_db SET TABLESPACE target_ts;
+ CREATE DATABASE newdb TEMPLATE template_db STRATEGY=<STRATEGY>;
+ ALTER DATABASE template_db IS_TEMPLATE = false;
+ DROP DATABASE dropme_db1;
+ DROP TABLE t;
+ DROP DATABASE dropme_db2; DROP TABLESPACE dropme_ts2;
+ DROP TABLESPACE source_ts;
+ DROP DATABASE template_db;];
+
+ $query =~ s/<STRATEGY>/$strategy/g;
+ $node_primary->safe_psql('postgres', $query);
+ $node_primary->wait_for_catchup($node_standby, 'replay',
+ $node_primary->lsn('replay'));
+
+ # show "create missing directory" log message
+ $node_standby->safe_psql('postgres',
+ "ALTER SYSTEM SET log_min_messages TO debug1;");
+ $node_standby->stop('immediate');
+ # Should restart ignoring directory creation error.
+ is($node_standby->start(fail_ok => 1), 1);
+ $node_standby->stop('immediate');
+}
+
+test_tablespace("FILE_COPY");
+test_tablespace("WAL_LOG");
+
+# Ensure that a missing tablespace directory during create database
+# replay immediately causes panic if the standby has already reached
+# consistent state (archive recovery is in progress). This is
+# effective only for CREATE DATABASE WITH STRATEGY=FILE_COPY.
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary2');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+
+# Create tablespace
+$node_primary->safe_psql('postgres', q[
+ SET allow_in_place_tablespaces=on;
+ CREATE TABLESPACE ts1 LOCATION '']);
+$node_primary->safe_psql('postgres', "CREATE DATABASE db1 WITH TABLESPACE ts1 STRATEGY=FILE_COPY");
+
+# Take backup
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+my $node_standby = PostgreSQL::Test::Cluster->new('standby3');
+$node_standby->init_from_backup($node_primary, $backup_name, has_streaming => 1);
+$node_standby->append_conf('postgresql.conf', "ignore_invalid_pages = on");
+$node_standby->start;
+
+# Make sure standby reached consistency and starts accepting connections
+$node_standby->poll_query_until('postgres', 'SELECT 1', '1');
+
+# Remove standby tablespace directory so it will be missing when
+# replay resumes.
+my $tspoid = $node_standby->safe_psql('postgres',
+ "SELECT oid FROM pg_tablespace WHERE spcname = 'ts1';");
+my $tspdir = $node_standby->data_dir . "/pg_tblspc/$tspoid";
+File::Path::rmtree($tspdir);
+
+my $logstart = get_log_size($node_standby);
+
+# Create a database in the tablespace and a table in default tablespace
+$node_primary->safe_psql('postgres',
+ q[CREATE TABLE should_not_replay_insertion(a int);
+ CREATE DATABASE db2 WITH TABLESPACE ts1 STRATEGY=FILE_COPY;
+ INSERT INTO should_not_replay_insertion VALUES (1);]);
+
+# Standby should fail and should not silently skip replaying the wal
+# In this test, PANIC turns into WARNING by ignore_invalid_pages.
+# Check the log messages instead of confirming standby failure.
+my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
+while ($max_attempts-- >= 0)
+{
+ last if (find_in_log(
+ $node_standby,
+ "WARNING: creating missing directory: pg_tblspc/",
+ $logstart));
+ sleep 1;
+}
+ok($max_attempts > 0, "invalid directory creation is detected");
+
+done_testing();
+
+
+# return the size of logfile of $node in bytes
+sub get_log_size
+{
+ my ($node) = @_;
+
+ return (stat $node->logfile)[7];
+}
+
+# find $pat in logfile of $node after $off-th byte
+sub find_in_log
+{
+ my ($node, $pat, $off) = @_;
+
+ $off = 0 unless defined $off;
+ my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
+ return 0 if (length($log) <= $off);
+
+ $log = substr($log, $off);
+
+ return $log =~ m/$pat/;
+}
--
2.30.2
--hwk7mjf7k3wvtowq
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v25-0002-split-is_path_tslink-as-a-new-routine.patch"
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v24] Fix replay of create database records on standby
@ 2022-07-13 16:14 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 18+ messages in thread
From: Alvaro Herrera @ 2022-07-13 16:14 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch allows missing tablespaces to be created during recovery
before reaching consistency. The tablespaces are created as real
directories that should not exists but will be removed until reaching
consistency. CheckRecoveryConsistency is responsible to make sure they
have disappeared.
Similar to log_invalid_page mechanism, the GUC ignore_invalid_pages
turns into PANIC errors detected by this patch into WARNING, which
allows continueing recovery.
Diagnosed-by: Paul Guo <paulguo@gmail.com>
Author: Paul Guo <paulguo@gmail.com>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Author: Asim R Praveen <apraveen@pivotal.io>
Discussion: https://postgr.es/m/CAEET0ZGx9AvioViLf7nbR_8tH9-=27DN5xWJ2P9-ROH16e4JUA@mail.gmail.com
---
doc/src/sgml/config.sgml | 5 +-
src/backend/access/transam/xlogrecovery.c | 59 ++++++++
src/backend/commands/dbcommands.c | 71 +++++++++
src/backend/commands/tablespace.c | 28 +---
src/backend/utils/misc/guc.c | 8 +-
src/include/access/xlogutils.h | 2 +
src/test/recovery/t/029_replay_tsp_drops.pl | 155 ++++++++++++++++++++
7 files changed, 296 insertions(+), 32 deletions(-)
create mode 100644 src/test/recovery/t/029_replay_tsp_drops.pl
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 37fd80388c..1e1c8c1cb7 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -11363,11 +11363,12 @@ LOG: CleanUpLock: deleting: lock(0xb7acd844) id(24688,24696,0,0,0,1)
<listitem>
<para>
If set to <literal>off</literal> (the default), detection of
- WAL records having references to invalid pages during
+ WAL records having references to invalid pages or
+ WAL records resulting in invalid directory operations during
recovery causes <productname>PostgreSQL</productname> to
raise a PANIC-level error, aborting the recovery. Setting
<varname>ignore_invalid_pages</varname> to <literal>on</literal>
- causes the system to ignore invalid page references in WAL records
+ causes the system to ignore invalid actions caused by such WAL records
(but still report a warning), and continue the recovery.
This behavior may <emphasis>cause crashes, data loss,
propagate or hide corruption, or other serious problems</emphasis>.
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 5d6f1b5e46..ae81244e06 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -2008,6 +2008,57 @@ xlogrecovery_redo(XLogReaderState *record, TimeLineID replayTLI)
}
}
+/*
+ * Makes sure that ./pg_tblspc directory doesn't contain a real directory.
+ *
+ * This is intended to be called after reaching consistency.
+ * ignore_invalid_pages=on turns into the PANIC error into WARNING so that
+ * recovery can continue.
+ *
+ * This can't be checked in allow_in_place_tablespaces mode, so skip it in
+ * that case.
+ */
+static void
+CheckTablespaceDirectory(void)
+{
+ char *tblspc_path = "./pg_tblspc";
+ DIR *dir;
+ struct dirent *de;
+
+ /* Do not check for this when test tablespaces are in use */
+ if (allow_in_place_tablespaces)
+ return;
+
+ dir = AllocateDir(tblspc_path);
+ while ((de = ReadDir(dir, tblspc_path)) != NULL)
+ {
+ char path[MAXPGPATH];
+ char *p;
+#ifndef WIN32
+ struct stat st;
+#endif
+
+ /* Skip entries of non-oid names */
+ for (p = de->d_name; *p && isdigit(*p); p++);
+ if (*p)
+ continue;
+
+ snprintf(path, MAXPGPATH, "%s/%s", tblspc_path, de->d_name);
+
+#ifndef WIN32
+ if (lstat(path, &st) < 0)
+ ereport(ERROR, errcode_for_file_access(),
+ errmsg("could not stat file \"%s\": %m", path));
+
+ if (!S_ISLNK(st.st_mode))
+#else
+ if (!pgwin32_is_junction(path))
+#endif
+ elog(ignore_invalid_pages ? WARNING : PANIC,
+ "real directory found in pg_tblspc directory: %s", de->d_name);
+ }
+}
+
/*
* Checks if recovery has reached a consistent state. When consistency is
* reached and we have a valid starting standby snapshot, tell postmaster
@@ -2072,6 +2123,14 @@ CheckRecoveryConsistency(void)
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
LSN_FORMAT_ARGS(lastReplayedEndRecPtr))));
+
+ /*
+ * Check that pg_tblspc doesn't contain a real
+ * directory. Database/CREATE_* records may create a tablespace
+ * directory that should have been removed until consistency is
+ * reached.
+ */
+ CheckTablespaceDirectory();
}
/*
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 1901b434c5..0f860030fa 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -30,6 +30,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -47,6 +48,7 @@
#include "commands/defrem.h"
#include "commands/seclabel.h"
#include "commands/tablespace.h"
+#include "common/file_perm.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "pgstat.h"
@@ -62,6 +64,7 @@
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
+#include "utils/guc.h"
#include "utils/pg_locale.h"
#include "utils/relmapper.h"
#include "utils/snapmgr.h"
@@ -135,6 +138,7 @@ static void CreateDirAndVersionFile(char *dbpath, Oid dbid, Oid tsid,
bool isRedo);
static void CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dboid, Oid src_tsid,
Oid dst_tsid);
+static void maybe_create_directory(char *path);
/*
* Create a new database using the WAL_LOG strategy.
@@ -3008,6 +3012,43 @@ get_database_name(Oid dbid)
return result;
}
+/*
+ * maybe_create_directory()
+ *
+ * During recovery, there's a case where we validly need to recover a missing
+ * tablespace directory so that recovery can continue. This happens when
+ * recovery wants to create a database but the holding tablespace has been
+ * removed before the server stopped. Since we expect that the directory will
+ * be gone before reaching recovery consistency, and we have no knowledge about
+ * the tablespace other than its OID here, we create a real directory under
+ * pg_tblspc here instead of restoring the symlink. ignore_invalid_pages=on
+ * reduces the error level so that recovery can continue.
+ */
+static void
+maybe_create_directory(char *path)
+{
+ struct stat st;
+
+ Assert(RecoveryInProgress());
+
+ if (stat(path, &st) == 0)
+ return;
+
+ /* XXX: Do we make sure that the path is under pg_tblspc? */
+
+ if (reachedConsistency && !ignore_invalid_pages)
+ ereport(PANIC,
+ errmsg("missing directory \"%s\"", path));
+
+ elog(reachedConsistency ? WARNING : DEBUG1,
+ "creating missing directory: %s", path);
+
+ if (pg_mkdir_p(path, pg_dir_create_mode) != 0)
+ ereport(PANIC,
+ errmsg("could not create missing directory \"%s\": %m", path));
+}
+
+
/*
* DATABASE resource manager's routines
*/
@@ -3044,6 +3085,30 @@ dbase_redo(XLogReaderState *record)
dst_path)));
}
+ if (stat(dst_path, &st) < 0)
+ {
+ char *parent_path;
+
+ if (errno != ENOENT)
+ ereport(FATAL,
+ errmsg("could not stat directory \"%s\": %m",
+ dst_path));
+
+ /* create the parent directory if needed and valid */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ maybe_create_directory(parent_path);
+ }
+
+ /*
+ * There's a case where the copy source directory is missing for the
+ * same reason above. Create the emtpy source directory so that
+ * copydir below doesn't fail. The directory will be dropped soon by
+ * recovery.
+ */
+ if (stat(src_path, &st) < 0 && errno == ENOENT)
+ maybe_create_directory(src_path);
+
/*
* Force dirty buffers out to disk, to ensure source database is
* up-to-date for the copy.
@@ -3068,9 +3133,15 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_wal_log_rec *xlrec =
(xl_dbase_create_wal_log_rec *) XLogRecGetData(record);
char *dbpath;
+ char *parent_path;
dbpath = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
+ /* create the parent directory if needed and valid */
+ parent_path = pstrdup(dbpath);
+ get_parent_directory(parent_path);
+ maybe_create_directory(parent_path);
+
/* Create the database directory with the version file. */
CreateDirAndVersionFile(dbpath, xlrec->db_id, xlrec->tablespace_id,
true);
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index c8bdd9992a..ddb031a83f 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -156,8 +156,6 @@ TablespaceCreateDbspace(Oid spcOid, Oid dbOid, bool isRedo)
/* Directory creation failed? */
if (MakePGDirectory(dir) < 0)
{
- char *parentdir;
-
/* Failure other than not exists or not in WAL replay? */
if (errno != ENOENT || !isRedo)
ereport(ERROR,
@@ -170,32 +168,8 @@ TablespaceCreateDbspace(Oid spcOid, Oid dbOid, bool isRedo)
* continue by creating simple parent directories rather
* than a symlink.
*/
-
- /* create two parents up if not exist */
- parentdir = pstrdup(dir);
- get_parent_directory(parentdir);
- get_parent_directory(parentdir);
- /* Can't create parent and it doesn't already exist? */
- if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not create directory \"%s\": %m",
- parentdir)));
- pfree(parentdir);
-
- /* create one parent up if not exist */
- parentdir = pstrdup(dir);
- get_parent_directory(parentdir);
- /* Can't create parent and it doesn't already exist? */
- if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not create directory \"%s\": %m",
- parentdir)));
- pfree(parentdir);
-
/* Create database directory */
- if (MakePGDirectory(dir) < 0)
+ if (pg_mkdir_p(dir, pg_dir_create_mode) < 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create directory \"%s\": %m",
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 0328029d43..cd6fa84e22 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -43,6 +43,7 @@
#include "access/xlog_internal.h"
#include "access/xlogprefetcher.h"
#include "access/xlogrecovery.h"
+#include "access/xlogutils.h"
#include "catalog/namespace.h"
#include "catalog/objectaccess.h"
#include "catalog/pg_authid.h"
@@ -141,7 +142,6 @@ extern int CommitSiblings;
extern char *default_tablespace;
extern char *temp_tablespaces;
extern bool ignore_checksum_failure;
-extern bool ignore_invalid_pages;
extern bool synchronize_seqscans;
#ifdef TRACE_SYNCSCAN
@@ -1336,10 +1336,12 @@ static struct config_bool ConfigureNamesBool[] =
{"ignore_invalid_pages", PGC_POSTMASTER, DEVELOPER_OPTIONS,
gettext_noop("Continues recovery after an invalid pages failure."),
gettext_noop("Detection of WAL records having references to "
- "invalid pages during recovery causes PostgreSQL to "
+ "invalid pages or WAL records resulting in invalid "
+ "directory operations during "
+ "recovery that cause PostgreSQL"
"raise a PANIC-level error, aborting the recovery. "
"Setting ignore_invalid_pages to true causes "
- "the system to ignore invalid page references "
+ "the system to ignore those inconsistencies "
"in WAL records (but still report a warning), "
"and continue recovery. This behavior may cause "
"crashes, data loss, propagate or hide corruption, "
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index ef182977bf..f203fdf539 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -54,6 +54,8 @@ typedef enum
extern PGDLLIMPORT HotStandbyState standbyState;
+extern bool ignore_invalid_pages;
+
#define InHotStandby (standbyState >= STANDBY_SNAPSHOT_PENDING)
diff --git a/src/test/recovery/t/029_replay_tsp_drops.pl b/src/test/recovery/t/029_replay_tsp_drops.pl
new file mode 100644
index 0000000000..d537fe0ce5
--- /dev/null
+++ b/src/test/recovery/t/029_replay_tsp_drops.pl
@@ -0,0 +1,155 @@
+
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+
+#
+# Tests relating to PostgreSQL crash recovery and redo
+#
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+sub test_tablespace
+{
+ my ($strategy) = @_;
+
+ my $node_primary = PostgreSQL::Test::Cluster->new("primary1_$strategy");
+ $node_primary->init(allows_streaming => 1);
+ $node_primary->start;
+ $node_primary->psql('postgres',
+ qq[
+ SET allow_in_place_tablespaces=on;
+ CREATE TABLESPACE dropme_ts1 LOCATION '';
+ CREATE TABLESPACE dropme_ts2 LOCATION '';
+ CREATE TABLESPACE source_ts LOCATION '';
+ CREATE TABLESPACE target_ts LOCATION '';
+ CREATE DATABASE template_db IS_TEMPLATE = true;
+ ]);
+ my $backup_name = 'my_backup';
+ $node_primary->backup($backup_name);
+
+ my $node_standby = PostgreSQL::Test::Cluster->new("standby2_$strategy");
+ $node_standby->init_from_backup($node_primary, $backup_name, has_streaming => 1);
+ $node_standby->append_conf('postgresql.conf', "ignore_invalid_pages = on");
+ $node_standby->start;
+
+ # Make sure connection is made
+ $node_primary->poll_query_until(
+ 'postgres', 'SELECT count(*) = 1 FROM pg_stat_replication');
+
+ $node_standby->safe_psql('postgres', 'CHECKPOINT');
+
+ # Do immediate shutdown just after a sequence of CREAT DATABASE / DROP
+ # DATABASE / DROP TABLESPACE. This causes CREATE DATABASE WAL records
+ # to be applied to already-removed directories.
+ my $query = q[
+ CREATE DATABASE dropme_db1 WITH TABLESPACE dropme_ts1 STRATEGY=<STRATEGY>;
+ CREATE TABLE t (a int) TABLESPACE dropme_ts2;
+ CREATE DATABASE dropme_db2 WITH TABLESPACE dropme_ts2 STRATEGY=<STRATEGY>;
+ CREATE DATABASE moveme_db TABLESPACE source_ts STRATEGY=<STRATEGY>;
+ ALTER DATABASE moveme_db SET TABLESPACE target_ts;
+ CREATE DATABASE newdb TEMPLATE template_db STRATEGY=<STRATEGY>;
+ ALTER DATABASE template_db IS_TEMPLATE = false;
+ DROP DATABASE dropme_db1;
+ DROP TABLE t;
+ DROP DATABASE dropme_db2; DROP TABLESPACE dropme_ts2;
+ DROP TABLESPACE source_ts;
+ DROP DATABASE template_db;];
+
+ $query =~ s/<STRATEGY>/$strategy/g;
+ $node_primary->safe_psql('postgres', $query);
+ $node_primary->wait_for_catchup($node_standby, 'replay',
+ $node_primary->lsn('replay'));
+
+ # show "create missing directory" log message
+ $node_standby->safe_psql('postgres',
+ "ALTER SYSTEM SET log_min_messages TO debug1;");
+ $node_standby->stop('immediate');
+ # Should restart ignoring directory creation error.
+ is($node_standby->start(fail_ok => 1), 1);
+ $node_standby->stop('immediate');
+}
+
+test_tablespace("FILE_COPY");
+test_tablespace("WAL_LOG");
+
+# Ensure that a missing tablespace directory during create database
+# replay immediately causes panic if the standby has already reached
+# consistent state (archive recovery is in progress). This is
+# effective only for CREATE DATABASE WITH STRATEGY=FILE_COPY.
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary2');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+
+# Create tablespace
+$node_primary->safe_psql('postgres', q[
+ SET allow_in_place_tablespaces=on;
+ CREATE TABLESPACE ts1 LOCATION '']);
+$node_primary->safe_psql('postgres', "CREATE DATABASE db1 WITH TABLESPACE ts1 STRATEGY=FILE_COPY");
+
+# Take backup
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+my $node_standby = PostgreSQL::Test::Cluster->new('standby3');
+$node_standby->init_from_backup($node_primary, $backup_name, has_streaming => 1);
+$node_standby->append_conf('postgresql.conf', "ignore_invalid_pages = on");
+$node_standby->start;
+
+# Make sure standby reached consistency and starts accepting connections
+$node_standby->poll_query_until('postgres', 'SELECT 1', '1');
+
+# Remove standby tablespace directory so it will be missing when
+# replay resumes.
+my $tspoid = $node_standby->safe_psql('postgres',
+ "SELECT oid FROM pg_tablespace WHERE spcname = 'ts1';");
+my $tspdir = $node_standby->data_dir . "/pg_tblspc/$tspoid";
+File::Path::rmtree($tspdir);
+
+my $logstart = get_log_size($node_standby);
+
+# Create a database in the tablespace and a table in default tablespace
+$node_primary->safe_psql('postgres',
+ q[CREATE TABLE should_not_replay_insertion(a int);
+ CREATE DATABASE db2 WITH TABLESPACE ts1 STRATEGY=FILE_COPY;
+ INSERT INTO should_not_replay_insertion VALUES (1);]);
+
+# Standby should fail and should not silently skip replaying the wal
+# In this test, PANIC turns into WARNING by ignore_invalid_pages.
+# Check the log messages instead of confirming standby failure.
+my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
+while ($max_attempts-- >= 0)
+{
+ last if (find_in_log(
+ $node_standby,
+ "WARNING: creating missing directory: pg_tblspc/",
+ $logstart));
+ sleep 1;
+}
+ok($max_attempts > 0, "invalid directory creation is detected");
+
+done_testing();
+
+
+# return the size of logfile of $node in bytes
+sub get_log_size
+{
+ my ($node) = @_;
+
+ return (stat $node->logfile)[7];
+}
+
+# find $pat in logfile of $node after $off-th byte
+sub find_in_log
+{
+ my ($node, $pat, $off) = @_;
+
+ $off = 0 unless defined $off;
+ my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
+ return 0 if (length($log) <= $off);
+
+ $log = substr($log, $off);
+
+ return $log =~ m/$pat/;
+}
--
2.30.2
--fjxxjvhdbrq5f7rb--
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v26] Fix replay of create database records on standby
@ 2022-07-13 16:14 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 18+ messages in thread
From: Alvaro Herrera @ 2022-07-13 16:14 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories when
replaying create database WAL records. Prior to this patch, the
standby would fail to recover in such a case. However, the
directories could be legitimately missing. Consider a sequence of WAL
records as follows:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the tablespace
directory, the standby crashes and has to replay the create database
record again, the crash recovery must be able to move on.
This patch allows missing tablespaces to be created during recovery
before reaching consistency. The tablespaces are created as real
directories that should not exists but will be removed until reaching
consistency. CheckRecoveryConsistency is responsible to make sure they
have disappeared.
The problems detected by this new code are reported as PANIC, except
when allow_in_place_tablespaces is set to ON, in which case they are
WARNING. Apart from making tests possible, this gives users an escape
hatch in case things don't go as planned.
Diagnosed-by: Paul Guo <paulguo@gmail.com>
Author: Paul Guo <paulguo@gmail.com>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Author: Asim R Praveen <apraveen@pivotal.io>
Discussion: https://postgr.es/m/CAEET0ZGx9AvioViLf7nbR_8tH9-=27DN5xWJ2P9-ROH16e4JUA@mail.gmail.com
---
src/backend/access/transam/xlogrecovery.c | 54 +++++++
src/backend/commands/dbcommands.c | 77 ++++++++++
src/backend/commands/tablespace.c | 28 +---
src/test/recovery/t/033_replay_tsp_drops.pl | 155 ++++++++++++++++++++
4 files changed, 287 insertions(+), 27 deletions(-)
create mode 100644 src/test/recovery/t/033_replay_tsp_drops.pl
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 5d6f1b5e46..850ab6d7e6 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -42,6 +42,7 @@
#include "access/xlogutils.h"
#include "catalog/pg_control.h"
#include "commands/tablespace.h"
+#include "common/file_utils.h"
#include "miscadmin.h"
#include "pgstat.h"
#include "postmaster/bgwriter.h"
@@ -2008,6 +2009,51 @@ xlogrecovery_redo(XLogReaderState *record, TimeLineID replayTLI)
}
}
+/*
+ * Verify that, in non-test mode, ./pg_tblspc doesn't contain any real
+ * directories.
+ *
+ * Replay of database creation XLOG records for databases that were later
+ * dropped can create fake directories in pg_tblspc. By the time consistency
+ * is reached these directories should have been removed; here we verify
+ * that this did indeed happen. This is to be called at the point where
+ * consistent state is reached.
+ *
+ * allow_in_place_tablespaces turns the PANIC into a WARNING, which is
+ * useful for testing purposes, and also allows for an escape hatch in case
+ * things go south.
+ */
+static void
+CheckTablespaceDirectory(void)
+{
+ DIR *dir;
+ struct dirent *de;
+
+ dir = AllocateDir("pg_tblspc");
+ while ((de = ReadDir(dir, "pg_tblspc")) != NULL)
+ {
+ char path[MAXPGPATH + 10];
+
+ /* Skip entries of non-oid names */
+ if (strspn(de->d_name, "0123456789") != strlen(de->d_name))
+ continue;
+
+ snprintf(path, sizeof(path), "pg_tblspc/%s", de->d_name);
+
+#ifdef WIN32
+ if (!pgwin32_is_junction(path))
+#else
+ if (get_dirent_type(path, de, false, ERROR) != PGFILETYPE_LNK)
+#endif
+ ereport(allow_in_place_tablespaces ? WARNING : PANIC,
+ (errcode(ERRCODE_DATA_CORRUPTED),
+ errmsg("unexpected directory entry \"%s\" found in %s",
+ de->d_name, "pg_tblspc/"),
+ errdetail("All directory entries in pg_tblspc/ should be symbolic links."),
+ errhint("Remove those directories, or set allow_in_place_tablespaces to ON transiently to let recovery complete.")));
+ }
+}
+
/*
* Checks if recovery has reached a consistent state. When consistency is
* reached and we have a valid starting standby snapshot, tell postmaster
@@ -2068,6 +2114,14 @@ CheckRecoveryConsistency(void)
*/
XLogCheckInvalidPages();
+ /*
+ * Check that pg_tblspc doesn't contain any real directories. Replay
+ * of Database/CREATE_* records may have created ficticious tablespace
+ * directories that should have been removed by the time consistency
+ * was reached.
+ */
+ CheckTablespaceDirectory();
+
reachedConsistency = true;
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 099d369b2f..95844bbb69 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -30,6 +30,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -47,6 +48,7 @@
#include "commands/defrem.h"
#include "commands/seclabel.h"
#include "commands/tablespace.h"
+#include "common/file_perm.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "pgstat.h"
@@ -62,6 +64,7 @@
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
+#include "utils/guc.h"
#include "utils/pg_locale.h"
#include "utils/relmapper.h"
#include "utils/snapmgr.h"
@@ -135,6 +138,7 @@ static void CreateDirAndVersionFile(char *dbpath, Oid dbid, Oid tsid,
bool isRedo);
static void CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dboid, Oid src_tsid,
Oid dst_tsid);
+static void recovery_create_dbdir(char *path, bool only_tblspc);
/*
* Create a new database using the WAL_LOG strategy.
@@ -2995,6 +2999,45 @@ get_database_name(Oid dbid)
return result;
}
+/*
+ * recovery_create_dbdir()
+ *
+ * During recovery, there's a case where we validly need to recover a missing
+ * tablespace directory so that recovery can continue. This happens when
+ * recovery wants to create a database but the holding tablespace has been
+ * removed before the server stopped. Since we expect that the directory will
+ * be gone before reaching recovery consistency, and we have no knowledge about
+ * the tablespace other than its OID here, we create a real directory under
+ * pg_tblspc here instead of restoring the symlink.
+ *
+ * If only_tblspc is true, then the requested directory must be in pg_tblspc/
+ */
+static void
+recovery_create_dbdir(char *path, bool only_tblspc)
+{
+ struct stat st;
+
+ Assert(RecoveryInProgress());
+
+ if (stat(path, &st) == 0)
+ return;
+
+ if (only_tblspc && strstr(path, "pg_tblspc/") == NULL)
+ elog(PANIC, "requested to created invalid directory: %s", path);
+
+ if (reachedConsistency && !allow_in_place_tablespaces)
+ ereport(PANIC,
+ errmsg("missing directory \"%s\"", path));
+
+ elog(reachedConsistency ? WARNING : DEBUG1,
+ "creating missing directory: %s", path);
+
+ if (pg_mkdir_p(path, pg_dir_create_mode) != 0)
+ ereport(PANIC,
+ errmsg("could not create missing directory \"%s\": %m", path));
+}
+
+
/*
* DATABASE resource manager's routines
*/
@@ -3012,6 +3055,7 @@ dbase_redo(XLogReaderState *record)
(xl_dbase_create_file_copy_rec *) XLogRecGetData(record);
char *src_path;
char *dst_path;
+ char *parent_path;
struct stat st;
src_path = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
@@ -3031,6 +3075,33 @@ dbase_redo(XLogReaderState *record)
dst_path)));
}
+ /*
+ * If the parent of the target path doesn't exist, create it now. This
+ * enables us to create the target underneath later.
+ */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ if (stat(parent_path, &st) < 0)
+ {
+ if (errno != ENOENT)
+ ereport(FATAL,
+ errmsg("could not stat directory \"%s\": %m",
+ dst_path));
+
+ /* create the parent directory if needed and valid */
+ recovery_create_dbdir(parent_path, true);
+ }
+ pfree(parent_path);
+
+ /*
+ * There's a case where the copy source directory is missing for the
+ * same reason above. Create the emtpy source directory so that
+ * copydir below doesn't fail. The directory will be dropped soon by
+ * recovery.
+ */
+ if (stat(src_path, &st) < 0 && errno == ENOENT)
+ recovery_create_dbdir(src_path, false);
+
/*
* Force dirty buffers out to disk, to ensure source database is
* up-to-date for the copy.
@@ -3055,9 +3126,15 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_wal_log_rec *xlrec =
(xl_dbase_create_wal_log_rec *) XLogRecGetData(record);
char *dbpath;
+ char *parent_path;
dbpath = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
+ /* create the parent directory if needed and valid */
+ parent_path = pstrdup(dbpath);
+ get_parent_directory(parent_path);
+ recovery_create_dbdir(parent_path, true);
+
/* Create the database directory with the version file. */
CreateDirAndVersionFile(dbpath, xlrec->db_id, xlrec->tablespace_id,
true);
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index cb7d46089a..a23097399e 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -156,8 +156,6 @@ TablespaceCreateDbspace(Oid spcOid, Oid dbOid, bool isRedo)
/* Directory creation failed? */
if (MakePGDirectory(dir) < 0)
{
- char *parentdir;
-
/* Failure other than not exists or not in WAL replay? */
if (errno != ENOENT || !isRedo)
ereport(ERROR,
@@ -170,32 +168,8 @@ TablespaceCreateDbspace(Oid spcOid, Oid dbOid, bool isRedo)
* continue by creating simple parent directories rather
* than a symlink.
*/
-
- /* create two parents up if not exist */
- parentdir = pstrdup(dir);
- get_parent_directory(parentdir);
- get_parent_directory(parentdir);
- /* Can't create parent and it doesn't already exist? */
- if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not create directory \"%s\": %m",
- parentdir)));
- pfree(parentdir);
-
- /* create one parent up if not exist */
- parentdir = pstrdup(dir);
- get_parent_directory(parentdir);
- /* Can't create parent and it doesn't already exist? */
- if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not create directory \"%s\": %m",
- parentdir)));
- pfree(parentdir);
-
/* Create database directory */
- if (MakePGDirectory(dir) < 0)
+ if (pg_mkdir_p(dir, pg_dir_create_mode) < 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create directory \"%s\": %m",
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
new file mode 100644
index 0000000000..0986df45e6
--- /dev/null
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -0,0 +1,155 @@
+
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+
+# Test replay of tablespace/database creation/drop
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+sub test_tablespace
+{
+ my ($strategy) = @_;
+
+ my $node_primary = PostgreSQL::Test::Cluster->new("primary1_$strategy");
+ $node_primary->init(allows_streaming => 1);
+ $node_primary->start;
+ $node_primary->psql('postgres',
+ qq[
+ SET allow_in_place_tablespaces=on;
+ CREATE TABLESPACE dropme_ts1 LOCATION '';
+ CREATE TABLESPACE dropme_ts2 LOCATION '';
+ CREATE TABLESPACE source_ts LOCATION '';
+ CREATE TABLESPACE target_ts LOCATION '';
+ CREATE DATABASE template_db IS_TEMPLATE = true;
+ ]);
+ my $backup_name = 'my_backup';
+ $node_primary->backup($backup_name);
+
+ my $node_standby = PostgreSQL::Test::Cluster->new("standby2_$strategy");
+ $node_standby->init_from_backup($node_primary, $backup_name, has_streaming => 1);
+ $node_standby->append_conf('postgresql.conf', "allow_in_place_tablespaces = on");
+ $node_standby->start;
+
+ # Make sure connection is made
+ $node_primary->poll_query_until(
+ 'postgres', 'SELECT count(*) = 1 FROM pg_stat_replication');
+
+ $node_standby->safe_psql('postgres', 'CHECKPOINT');
+
+ # Do immediate shutdown just after a sequence of CREAT DATABASE / DROP
+ # DATABASE / DROP TABLESPACE. This causes CREATE DATABASE WAL records
+ # to be applied to already-removed directories.
+ my $query = q[
+ CREATE DATABASE dropme_db1 WITH TABLESPACE dropme_ts1 STRATEGY=<STRATEGY>;
+ CREATE TABLE t (a int) TABLESPACE dropme_ts2;
+ CREATE DATABASE dropme_db2 WITH TABLESPACE dropme_ts2 STRATEGY=<STRATEGY>;
+ CREATE DATABASE moveme_db TABLESPACE source_ts STRATEGY=<STRATEGY>;
+ ALTER DATABASE moveme_db SET TABLESPACE target_ts;
+ CREATE DATABASE newdb TEMPLATE template_db STRATEGY=<STRATEGY>;
+ ALTER DATABASE template_db IS_TEMPLATE = false;
+ DROP DATABASE dropme_db1;
+ DROP TABLE t;
+ DROP DATABASE dropme_db2; DROP TABLESPACE dropme_ts2;
+ DROP TABLESPACE source_ts;
+ DROP DATABASE template_db;];
+
+ $query =~ s/<STRATEGY>/$strategy/g;
+ $node_primary->safe_psql('postgres', $query);
+ $node_primary->wait_for_catchup($node_standby, 'replay',
+ $node_primary->lsn('replay'));
+
+ # show "create missing directory" log message
+ $node_standby->safe_psql('postgres',
+ "ALTER SYSTEM SET log_min_messages TO debug1;");
+ $node_standby->stop('immediate');
+ # Should restart ignoring directory creation error.
+ is($node_standby->start(fail_ok => 1), 1, "standby node started for $strategy");
+ $node_standby->stop('immediate');
+}
+
+test_tablespace("FILE_COPY");
+test_tablespace("WAL_LOG");
+
+# Ensure that a missing tablespace directory during create database
+# replay immediately causes panic if the standby has already reached
+# consistent state (archive recovery is in progress). This is
+# effective only for CREATE DATABASE WITH STRATEGY=FILE_COPY.
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary2');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+
+# Create tablespace
+$node_primary->safe_psql('postgres', q[
+ SET allow_in_place_tablespaces=on;
+ CREATE TABLESPACE ts1 LOCATION '']);
+$node_primary->safe_psql('postgres', "CREATE DATABASE db1 WITH TABLESPACE ts1 STRATEGY=FILE_COPY");
+
+# Take backup
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+my $node_standby = PostgreSQL::Test::Cluster->new('standby3');
+$node_standby->init_from_backup($node_primary, $backup_name, has_streaming => 1);
+$node_standby->append_conf('postgresql.conf', "allow_in_place_tablespaces = on");
+$node_standby->start;
+
+# Make sure standby reached consistency and starts accepting connections
+$node_standby->poll_query_until('postgres', 'SELECT 1', '1');
+
+# Remove standby tablespace directory so it will be missing when
+# replay resumes.
+my $tspoid = $node_standby->safe_psql('postgres',
+ "SELECT oid FROM pg_tablespace WHERE spcname = 'ts1';");
+my $tspdir = $node_standby->data_dir . "/pg_tblspc/$tspoid";
+File::Path::rmtree($tspdir);
+
+my $logstart = get_log_size($node_standby);
+
+# Create a database in the tablespace and a table in default tablespace
+$node_primary->safe_psql('postgres',
+ q[CREATE TABLE should_not_replay_insertion(a int);
+ CREATE DATABASE db2 WITH TABLESPACE ts1 STRATEGY=FILE_COPY;
+ INSERT INTO should_not_replay_insertion VALUES (1);]);
+
+# Standby should fail and should not silently skip replaying the wal
+# In this test, PANIC turns into WARNING by allow_in_place_tablespaces.
+# Check the log messages instead of confirming standby failure.
+my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
+while ($max_attempts-- >= 0)
+{
+ last if (find_in_log(
+ $node_standby,
+ "WARNING: creating missing directory: pg_tblspc/",
+ $logstart));
+ sleep 1;
+}
+ok($max_attempts > 0, "invalid directory creation is detected");
+
+done_testing();
+
+
+# return the size of logfile of $node in bytes
+sub get_log_size
+{
+ my ($node) = @_;
+
+ return (stat $node->logfile)[7];
+}
+
+# find $pat in logfile of $node after $off-th byte
+sub find_in_log
+{
+ my ($node, $pat, $off) = @_;
+
+ $off = 0 unless defined $off;
+ my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
+ return 0 if (length($log) <= $off);
+
+ $log = substr($log, $off);
+
+ return $log =~ m/$pat/;
+}
--
2.30.2
--ugp6tm4ilhq6dt7x--
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v27] Fix replay of create database records on standby
@ 2022-07-27 18:22 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 18+ messages in thread
From: Alvaro Herrera @ 2022-07-27 18:22 UTC (permalink / raw)
Crash recovery on standby may encounter missing directories
when replaying database-creation WAL records. Prior to this
patch, the standby would fail to recover in such a case;
however, the directories could be legitimately missing.
Consider the following sequence of commands:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the
tablespace directory, the standby crashes and has to replay the
create database record again, crash recovery must be able to continue.
A fix for this problem was already attempted in 49d9cfc68bf4, but it
was reverted because of design issues. This new version is based
on Robert Haas' proposal: any missing tablespaces are created
during recovery before reaching consistency. Tablespaces
are created as real directories, and should be deleted
by later replay. CheckRecoveryConsistency ensures
they have disappeared.
The problems detected by this new code are reported as PANIC,
except when allow_in_place_tablespaces is set to ON, in which
case they are WARNING. Apart from making tests possible, this
gives users an escape hatch in case things don't go as planned.
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Author: Asim R Praveen <apraveen@pivotal.io>
Author: Paul Guo <paulguo@gmail.com>
Reviewed-by: Anastasia Lubennikova <lubennikovaav@gmail.com> (older versions)
Reviewed-by: Fujii Masao <masao.fujii@oss.nttdata.com> (older versions)
Reviewed-by: Michaël Paquier <michael@paquier.xyz>
Diagnosed-by: Paul Guo <paulguo@gmail.com>
Discussion: https://postgr.es/m/CAEET0ZGx9AvioViLf7nbR_8tH9-=27DN5xWJ2P9-ROH16e4JUA@mail.gmail.com
---
src/backend/access/transam/xlogrecovery.c | 50 ++++++
src/backend/commands/dbcommands.c | 77 +++++++++
src/backend/commands/tablespace.c | 40 ++---
src/test/recovery/t/033_replay_tsp_drops.pl | 169 ++++++++++++++++++++
4 files changed, 305 insertions(+), 31 deletions(-)
create mode 100644 src/test/recovery/t/033_replay_tsp_drops.pl
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index e383c2123a..27e02fbfcd 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -42,6 +42,7 @@
#include "access/xlogutils.h"
#include "catalog/pg_control.h"
#include "commands/tablespace.h"
+#include "common/file_utils.h"
#include "miscadmin.h"
#include "pgstat.h"
#include "postmaster/bgwriter.h"
@@ -2008,6 +2009,47 @@ xlogrecovery_redo(XLogReaderState *record, TimeLineID replayTLI)
}
}
+/*
+ * Verify that, in non-test mode, ./pg_tblspc doesn't contain any real
+ * directories.
+ *
+ * Replay of database creation XLOG records for databases that were later
+ * dropped can create fake directories in pg_tblspc. By the time consistency
+ * is reached these directories should have been removed; here we verify
+ * that this did indeed happen. This is to be called at the point where
+ * consistent state is reached.
+ *
+ * allow_in_place_tablespaces turns the PANIC into a WARNING, which is
+ * useful for testing purposes, and also allows for an escape hatch in case
+ * things go south.
+ */
+static void
+CheckTablespaceDirectory(void)
+{
+ DIR *dir;
+ struct dirent *de;
+
+ dir = AllocateDir("pg_tblspc");
+ while ((de = ReadDir(dir, "pg_tblspc")) != NULL)
+ {
+ char path[MAXPGPATH + 10];
+
+ /* Skip entries of non-oid names */
+ if (strspn(de->d_name, "0123456789") != strlen(de->d_name))
+ continue;
+
+ snprintf(path, sizeof(path), "pg_tblspc/%s", de->d_name);
+
+ if (get_dirent_type(path, de, false, ERROR) != PGFILETYPE_LNK)
+ ereport(allow_in_place_tablespaces ? WARNING : PANIC,
+ (errcode(ERRCODE_DATA_CORRUPTED),
+ errmsg("unexpected directory entry \"%s\" found in %s",
+ de->d_name, "pg_tblspc/"),
+ errdetail("All directory entries in pg_tblspc/ should be symbolic links."),
+ errhint("Remove those directories, or set allow_in_place_tablespaces to ON transiently to let recovery complete.")));
+ }
+}
+
/*
* Checks if recovery has reached a consistent state. When consistency is
* reached and we have a valid starting standby snapshot, tell postmaster
@@ -2068,6 +2110,14 @@ CheckRecoveryConsistency(void)
*/
XLogCheckInvalidPages();
+ /*
+ * Check that pg_tblspc doesn't contain any real directories. Replay
+ * of Database/CREATE_* records may have created ficticious tablespace
+ * directories that should have been removed by the time consistency
+ * was reached.
+ */
+ CheckTablespaceDirectory();
+
reachedConsistency = true;
ereport(LOG,
(errmsg("consistent recovery state reached at %X/%X",
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 099d369b2f..95844bbb69 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -30,6 +30,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "access/xloginsert.h"
+#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
@@ -47,6 +48,7 @@
#include "commands/defrem.h"
#include "commands/seclabel.h"
#include "commands/tablespace.h"
+#include "common/file_perm.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "pgstat.h"
@@ -62,6 +64,7 @@
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
+#include "utils/guc.h"
#include "utils/pg_locale.h"
#include "utils/relmapper.h"
#include "utils/snapmgr.h"
@@ -135,6 +138,7 @@ static void CreateDirAndVersionFile(char *dbpath, Oid dbid, Oid tsid,
bool isRedo);
static void CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dboid, Oid src_tsid,
Oid dst_tsid);
+static void recovery_create_dbdir(char *path, bool only_tblspc);
/*
* Create a new database using the WAL_LOG strategy.
@@ -2995,6 +2999,45 @@ get_database_name(Oid dbid)
return result;
}
+/*
+ * recovery_create_dbdir()
+ *
+ * During recovery, there's a case where we validly need to recover a missing
+ * tablespace directory so that recovery can continue. This happens when
+ * recovery wants to create a database but the holding tablespace has been
+ * removed before the server stopped. Since we expect that the directory will
+ * be gone before reaching recovery consistency, and we have no knowledge about
+ * the tablespace other than its OID here, we create a real directory under
+ * pg_tblspc here instead of restoring the symlink.
+ *
+ * If only_tblspc is true, then the requested directory must be in pg_tblspc/
+ */
+static void
+recovery_create_dbdir(char *path, bool only_tblspc)
+{
+ struct stat st;
+
+ Assert(RecoveryInProgress());
+
+ if (stat(path, &st) == 0)
+ return;
+
+ if (only_tblspc && strstr(path, "pg_tblspc/") == NULL)
+ elog(PANIC, "requested to created invalid directory: %s", path);
+
+ if (reachedConsistency && !allow_in_place_tablespaces)
+ ereport(PANIC,
+ errmsg("missing directory \"%s\"", path));
+
+ elog(reachedConsistency ? WARNING : DEBUG1,
+ "creating missing directory: %s", path);
+
+ if (pg_mkdir_p(path, pg_dir_create_mode) != 0)
+ ereport(PANIC,
+ errmsg("could not create missing directory \"%s\": %m", path));
+}
+
+
/*
* DATABASE resource manager's routines
*/
@@ -3012,6 +3055,7 @@ dbase_redo(XLogReaderState *record)
(xl_dbase_create_file_copy_rec *) XLogRecGetData(record);
char *src_path;
char *dst_path;
+ char *parent_path;
struct stat st;
src_path = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
@@ -3031,6 +3075,33 @@ dbase_redo(XLogReaderState *record)
dst_path)));
}
+ /*
+ * If the parent of the target path doesn't exist, create it now. This
+ * enables us to create the target underneath later.
+ */
+ parent_path = pstrdup(dst_path);
+ get_parent_directory(parent_path);
+ if (stat(parent_path, &st) < 0)
+ {
+ if (errno != ENOENT)
+ ereport(FATAL,
+ errmsg("could not stat directory \"%s\": %m",
+ dst_path));
+
+ /* create the parent directory if needed and valid */
+ recovery_create_dbdir(parent_path, true);
+ }
+ pfree(parent_path);
+
+ /*
+ * There's a case where the copy source directory is missing for the
+ * same reason above. Create the emtpy source directory so that
+ * copydir below doesn't fail. The directory will be dropped soon by
+ * recovery.
+ */
+ if (stat(src_path, &st) < 0 && errno == ENOENT)
+ recovery_create_dbdir(src_path, false);
+
/*
* Force dirty buffers out to disk, to ensure source database is
* up-to-date for the copy.
@@ -3055,9 +3126,15 @@ dbase_redo(XLogReaderState *record)
xl_dbase_create_wal_log_rec *xlrec =
(xl_dbase_create_wal_log_rec *) XLogRecGetData(record);
char *dbpath;
+ char *parent_path;
dbpath = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
+ /* create the parent directory if needed and valid */
+ parent_path = pstrdup(dbpath);
+ get_parent_directory(parent_path);
+ recovery_create_dbdir(parent_path, true);
+
/* Create the database directory with the version file. */
CreateDirAndVersionFile(dbpath, xlrec->db_id, xlrec->tablespace_id,
true);
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index cb7d46089a..570ce3dbd5 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -156,8 +156,6 @@ TablespaceCreateDbspace(Oid spcOid, Oid dbOid, bool isRedo)
/* Directory creation failed? */
if (MakePGDirectory(dir) < 0)
{
- char *parentdir;
-
/* Failure other than not exists or not in WAL replay? */
if (errno != ENOENT || !isRedo)
ereport(ERROR,
@@ -166,36 +164,16 @@ TablespaceCreateDbspace(Oid spcOid, Oid dbOid, bool isRedo)
dir)));
/*
- * Parent directories are missing during WAL replay, so
- * continue by creating simple parent directories rather
- * than a symlink.
+ * During WAL replay, it's conceivable that several levels
+ * of directories are missing if tablespaces are dropped
+ * further ahead of the WAL stream than we're currently
+ * replaying. An easy way forward is to create them as
+ * plain directories and hope they are removed by further
+ * WAL replay if necessary. If this also fails, there is
+ * trouble we cannot get out of, so just report that and
+ * bail out.
*/
-
- /* create two parents up if not exist */
- parentdir = pstrdup(dir);
- get_parent_directory(parentdir);
- get_parent_directory(parentdir);
- /* Can't create parent and it doesn't already exist? */
- if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not create directory \"%s\": %m",
- parentdir)));
- pfree(parentdir);
-
- /* create one parent up if not exist */
- parentdir = pstrdup(dir);
- get_parent_directory(parentdir);
- /* Can't create parent and it doesn't already exist? */
- if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not create directory \"%s\": %m",
- parentdir)));
- pfree(parentdir);
-
- /* Create database directory */
- if (MakePGDirectory(dir) < 0)
+ if (pg_mkdir_p(dir, pg_dir_create_mode) < 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create directory \"%s\": %m",
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
new file mode 100644
index 0000000000..9b74cb09ac
--- /dev/null
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -0,0 +1,169 @@
+
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+
+# Test replay of tablespace/database creation/drop
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+sub test_tablespace
+{
+ my ($strategy) = @_;
+
+ my $node_primary = PostgreSQL::Test::Cluster->new("primary1_$strategy");
+ $node_primary->init(allows_streaming => 1);
+ $node_primary->start;
+ $node_primary->psql(
+ 'postgres',
+ qq[
+ SET allow_in_place_tablespaces=on;
+ CREATE TABLESPACE dropme_ts1 LOCATION '';
+ CREATE TABLESPACE dropme_ts2 LOCATION '';
+ CREATE TABLESPACE source_ts LOCATION '';
+ CREATE TABLESPACE target_ts LOCATION '';
+ CREATE DATABASE template_db IS_TEMPLATE = true;
+ ]);
+ my $backup_name = 'my_backup';
+ $node_primary->backup($backup_name);
+
+ my $node_standby = PostgreSQL::Test::Cluster->new("standby2_$strategy");
+ $node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+ $node_standby->append_conf('postgresql.conf',
+ "allow_in_place_tablespaces = on");
+ $node_standby->start;
+
+ # Make sure connection is made
+ $node_primary->poll_query_until('postgres',
+ 'SELECT count(*) = 1 FROM pg_stat_replication');
+
+ $node_standby->safe_psql('postgres', 'CHECKPOINT');
+
+ # Do immediate shutdown just after a sequence of CREAT DATABASE / DROP
+ # DATABASE / DROP TABLESPACE. This causes CREATE DATABASE WAL records
+ # to be applied to already-removed directories.
+ my $query = q[
+ CREATE DATABASE dropme_db1 WITH TABLESPACE dropme_ts1 STRATEGY=<STRATEGY>;
+ CREATE TABLE t (a int) TABLESPACE dropme_ts2;
+ CREATE DATABASE dropme_db2 WITH TABLESPACE dropme_ts2 STRATEGY=<STRATEGY>;
+ CREATE DATABASE moveme_db TABLESPACE source_ts STRATEGY=<STRATEGY>;
+ ALTER DATABASE moveme_db SET TABLESPACE target_ts;
+ CREATE DATABASE newdb TEMPLATE template_db STRATEGY=<STRATEGY>;
+ ALTER DATABASE template_db IS_TEMPLATE = false;
+ DROP DATABASE dropme_db1;
+ DROP TABLE t;
+ DROP DATABASE dropme_db2; DROP TABLESPACE dropme_ts2;
+ DROP TABLESPACE source_ts;
+ DROP DATABASE template_db;
+ ];
+
+ $query =~ s/<STRATEGY>/$strategy/g;
+ $node_primary->safe_psql('postgres', $query);
+ $node_primary->wait_for_catchup($node_standby, 'replay',
+ $node_primary->lsn('write'));
+
+ # show "create missing directory" log message
+ $node_standby->safe_psql('postgres',
+ "ALTER SYSTEM SET log_min_messages TO debug1;");
+ $node_standby->stop('immediate');
+ # Should restart ignoring directory creation error.
+ is($node_standby->start(fail_ok => 1),
+ 1, "standby node started for $strategy");
+ $node_standby->stop('immediate');
+}
+
+test_tablespace("FILE_COPY");
+test_tablespace("WAL_LOG");
+
+# Ensure that a missing tablespace directory during create database
+# replay immediately causes panic if the standby has already reached
+# consistent state (archive recovery is in progress). This is
+# effective only for CREATE DATABASE WITH STRATEGY=FILE_COPY.
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary2');
+$node_primary->init(allows_streaming => 1);
+$node_primary->start;
+
+# Create tablespace
+$node_primary->safe_psql(
+ 'postgres', q[
+ SET allow_in_place_tablespaces=on;
+ CREATE TABLESPACE ts1 LOCATION ''
+ ]);
+$node_primary->safe_psql('postgres',
+ "CREATE DATABASE db1 WITH TABLESPACE ts1 STRATEGY=FILE_COPY");
+
+# Take backup
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+my $node_standby = PostgreSQL::Test::Cluster->new('standby3');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->append_conf('postgresql.conf',
+ "allow_in_place_tablespaces = on");
+$node_standby->start;
+
+# Make sure standby reached consistency and starts accepting connections
+$node_standby->poll_query_until('postgres', 'SELECT 1', '1');
+
+# Remove standby tablespace directory so it will be missing when
+# replay resumes.
+my $tspoid = $node_standby->safe_psql('postgres',
+ "SELECT oid FROM pg_tablespace WHERE spcname = 'ts1';");
+my $tspdir = $node_standby->data_dir . "/pg_tblspc/$tspoid";
+File::Path::rmtree($tspdir);
+
+my $logstart = get_log_size($node_standby);
+
+# Create a database in the tablespace and a table in default tablespace
+$node_primary->safe_psql(
+ 'postgres',
+ q[
+ CREATE TABLE should_not_replay_insertion(a int);
+ CREATE DATABASE db2 WITH TABLESPACE ts1 STRATEGY=FILE_COPY;
+ INSERT INTO should_not_replay_insertion VALUES (1);
+ ]);
+
+# Standby should fail and should not silently skip replaying the wal
+# In this test, PANIC turns into WARNING by allow_in_place_tablespaces.
+# Check the log messages instead of confirming standby failure.
+my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
+while ($max_attempts-- >= 0)
+{
+ last
+ if (
+ find_in_log(
+ $node_standby, "WARNING: creating missing directory: pg_tblspc/",
+ $logstart));
+ sleep 1;
+}
+ok($max_attempts > 0, "invalid directory creation is detected");
+
+done_testing();
+
+
+# return the size of logfile of $node in bytes
+sub get_log_size
+{
+ my ($node) = @_;
+
+ return (stat $node->logfile)[7];
+}
+
+# find $pat in logfile of $node after $off-th byte
+sub find_in_log
+{
+ my ($node, $pat, $off) = @_;
+
+ $off = 0 unless defined $off;
+ my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
+ return 0 if (length($log) <= $off);
+
+ $log = substr($log, $off);
+
+ return $log =~ m/$pat/;
+}
--
2.30.2
--muecx67dktuttuk3--
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH] Allow an extention to be updated without a script
@ 2023-01-30 08:36 Yugo Nagata <nagata@sraoss.co.jp>
0 siblings, 0 replies; 18+ messages in thread
From: Yugo Nagata @ 2023-01-30 08:36 UTC (permalink / raw)
When we don't need to execute any command to update an extension from one
version to the next, we can specify a list of such updates following
the pattern 'old_version--target_version' into a new option
updates_without_script. For example, specifying '1.1--1.2, 1.3--1.4'
means updates from version 1.1 to version 1.2, and from version 1.3
to version 1.4 don't need an update script. User doesn't need to
provide an update script that doesn't contain any command for such
updates.
The updated path is determined based on both the names of update scripts
and the list in updates_without_script. If an update script is provided,
the script will be executed even if this update is specified in
updates_without_script.
---
doc/src/sgml/extend.sgml | 30 +++-
src/backend/commands/extension.c | 161 +++++++++++++++---
src/test/modules/test_extensions/Makefile | 3 +-
.../expected/test_extensions.out | 6 +
.../test_extensions/sql/test_extensions.sql | 8 +
.../test_extensions/test_ext9--1.0.sql | 0
.../test_extensions/test_ext9--2.0--3.0.sql | 0
.../modules/test_extensions/test_ext9.control | 5 +
8 files changed, 182 insertions(+), 31 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext9--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext9--2.0--3.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext9.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 46e873a166..1c4c978264 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -807,6 +807,17 @@ RETURNS anycompatible AS ...
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><varname>updates_without_script</varname> (<type>string</type>)</term>
+ <listitem>
+ <para>
+ A list of updates that do not need update scripts following the pattern
+ <literal><replaceable>old_version</replaceable>--<replaceable>target_version</replaceable></literal>,
+ for example <literal>updates_without_script = '1.1--1.2, 1.3--1.4'</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
<para>
@@ -818,8 +829,9 @@ RETURNS anycompatible AS ...
Secondary control files follow the same format as the primary control
file. Any parameters set in a secondary control file override the
primary control file when installing or updating to that version of
- the extension. However, the parameters <varname>directory</varname> and
- <varname>default_version</varname> cannot be set in a secondary control file.
+ the extension. However, the parameters <varname>directory</varname>,
+ <varname>default_version</varname>, and <varname>updates_without_script</varname>
+ cannot be set in a secondary control file.
</para>
<para>
@@ -1092,6 +1104,20 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
objects, they are automatically dissociated from the extension.
</para>
+ <para>
+ If you don't need to execute any command to update an extension from one
+ version to the next, provide an update script that doesn't contain
+ any command or specify a list of such updates following the pattern
+ <literal><replaceable>old_version</replaceable>--<replaceable>target_version</replaceable></literal>
+ into <varname>updates_without_script</varname>. For example,
+ <literal>updates_without_script = '1.1--1.2, 1.3--1.4'</literal>
+ means updates from version <literal>1.1</literal> to version <literal>1.2</literal>
+ and from version <literal>1.3</literal> to version <literal>1.4</literal>
+ don't need an update script. Note that even if an update is specified in
+ <varname>updates_without_script</varname>, if a corresponding update script
+ is provided, the update script will be executed.
+ </para>
+
<para>
If an extension has secondary control files, the control parameters
that are used for an update script are those associated with the script's
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index cf1b1ca571..2475f96030 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -90,6 +90,7 @@ typedef struct ExtensionControlFile
bool trusted; /* allow becoming superuser on the fly? */
int encoding; /* encoding of the script file, or -1 */
List *requires; /* names of prerequisite extensions */
+ List *updates_without_script; /* updates that don't need a script */
} ExtensionControlFile;
/*
@@ -98,14 +99,24 @@ typedef struct ExtensionControlFile
typedef struct ExtensionVersionInfo
{
char *name; /* name of the starting version */
- List *reachable; /* List of ExtensionVersionInfo's */
+ List *reachable; /* List of ExtensionUpdateStep's */
bool installable; /* does this version have an install script? */
+ bool without_script; /* reachable from the previous without a script? */
/* working state for Dijkstra's algorithm: */
bool distance_known; /* is distance from start known yet? */
int distance; /* current worst-case distance estimate */
struct ExtensionVersionInfo *previous; /* current best predecessor */
} ExtensionVersionInfo;
+/*
+ * Internal data structure for each step in an update path
+ */
+typedef struct ExtensionUpdateStep
+{
+ ExtensionVersionInfo *next_version; /* the next version */
+ bool without_script; /* reachable to the next without a script? */
+} ExtensionUpdateStep;
+
/* Local functions */
static List *find_update_path(List *evi_list,
ExtensionVersionInfo *evi_start,
@@ -606,6 +617,27 @@ parse_extension_control_file(ExtensionControlFile *control,
item->name)));
}
}
+ else if (strcmp(item->name, "updates_without_script") == 0)
+ {
+ /* Need a modifiable copy of string */
+ char *rawnames = pstrdup(item->value);
+
+ if (version)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("parameter \"%s\" cannot be set in a secondary extension control file",
+ item->name)));
+
+ /* Parse string into list of identifiers */
+ if (!SplitIdentifierString(rawnames, ',', &control->updates_without_script))
+ {
+ /* syntax error in name list */
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" must be a list of \"old_version--target_version\"",
+ item->name)));
+ }
+ }
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -1092,6 +1124,7 @@ get_ext_ver_info(const char *versionname, List **evi_list)
evi->name = pstrdup(versionname);
evi->reachable = NIL;
evi->installable = false;
+ evi->without_script = false;
/* initialize for later application of Dijkstra's algorithm */
evi->distance_known = false;
evi->distance = INT_MAX;
@@ -1130,10 +1163,44 @@ get_nearest_unprocessed_vertex(List *evi_list)
return evi;
}
+/*
+ * Extract version name(s) from a string in 'vername--vername2' format
+ */
+static bool
+extract_version_names(const char *str, char **vername, char **vername2)
+{
+ *vername = pstrdup(str);
+ *vername2 = strstr(*vername, "--");
+ if (*vername2)
+ {
+ **vername2 = '\0'; /* terminate first version */
+ *vername2 += 2; /* and point to second */
+
+ /* if there's a third --, it's bogus, ignore it */
+ if (strstr(*vername2, "--"))
+ return false;
+ }
+ return true;
+}
+
+/*
+ * Make ExensionUpdateStep data
+ */
+static ExtensionUpdateStep *
+make_ext_update_step(ExtensionVersionInfo *evi, bool without_script)
+{
+ ExtensionUpdateStep *step = (ExtensionUpdateStep *) palloc(sizeof(ExtensionUpdateStep));
+
+ step->next_version = evi;
+ step->without_script = without_script;
+
+ return step;
+}
+
/*
* Obtain information about the set of update scripts available for the
* specified extension. The result is a List of ExtensionVersionInfo
- * structs, each with a subsidiary list of the ExtensionVersionInfos for
+ * structs, each with a subsidiary list of the ExtensionUpdateSteps for
* the versions that can be reached in one step from that version.
*/
static List *
@@ -1144,11 +1211,13 @@ get_ext_ver_list(ExtensionControlFile *control)
char *location;
DIR *dir;
struct dirent *de;
+ ListCell *lc;
location = get_extension_script_directory(control);
dir = AllocateDir(location);
while ((de = ReadDir(dir, location)) != NULL)
{
+ char *vernames;
char *vername;
char *vername2;
ExtensionVersionInfo *evi;
@@ -1165,29 +1234,43 @@ get_ext_ver_list(ExtensionControlFile *control)
continue;
/* extract version name(s) from 'extname--something.sql' filename */
- vername = pstrdup(de->d_name + extnamelen + 2);
- *strrchr(vername, '.') = '\0';
- vername2 = strstr(vername, "--");
+ vernames = pstrdup(de->d_name + extnamelen + 2);
+ *strrchr(vernames, '.') = '\0';
+ if (!extract_version_names(vernames, &vername, &vername2))
+ continue;
+
+ /* Create ExtensionVersionInfos and link them together */
+ evi = get_ext_ver_info(vername, &evi_list);
if (!vername2)
{
- /* It's an install, not update, script; record its version name */
- evi = get_ext_ver_info(vername, &evi_list);
+ /* It's an install, not update, script. */
evi->installable = true;
continue;
}
- *vername2 = '\0'; /* terminate first version */
- vername2 += 2; /* and point to second */
+ evi2 = get_ext_ver_info(vername2, &evi_list);
+ evi->reachable = lappend(evi->reachable, make_ext_update_step(evi2, false));
+ }
+ FreeDir(dir);
- /* if there's a third --, it's bogus, ignore it */
- if (strstr(vername2, "--"))
+ /*
+ * Obtain version information from 'old-version--new-version' list in
+ * updates_without_script option
+ */
+ foreach (lc, control->updates_without_script)
+ {
+ char *vernames = (char *) lfirst(lc);
+ char *vername;
+ char *vername2;
+ ExtensionVersionInfo *evi;
+ ExtensionVersionInfo *evi2;
+
+ if (!extract_version_names(vernames, &vername, &vername2))
continue;
- /* Create ExtensionVersionInfos and link them together */
evi = get_ext_ver_info(vername, &evi_list);
evi2 = get_ext_ver_info(vername2, &evi_list);
- evi->reachable = lappend(evi->reachable, evi2);
+ evi->reachable = lappend(evi->reachable, make_ext_update_step(evi2, true));
}
- FreeDir(dir);
return evi_list;
}
@@ -1196,8 +1279,9 @@ get_ext_ver_list(ExtensionControlFile *control)
* Given an initial and final version name, identify the sequence of update
* scripts that have to be applied to perform that update.
*
- * Result is a List of names of versions to transition through (the initial
- * version is *not* included).
+ * Result is a List of the ExtensionUpdateSteps to transition through (the
+ * initial version infomration is *not* included). Returns NIL if no such
+ * path.
*/
static List *
identify_update_path(ExtensionControlFile *control,
@@ -1239,8 +1323,9 @@ identify_update_path(ExtensionControlFile *control,
* been used for this before, and the initialization done by get_ext_ver_info
* is still good. Otherwise, reinitialize all transient fields used here.
*
- * Result is a List of names of versions to transition through (the initial
- * version is *not* included). Returns NIL if no such path.
+ * Result is a List of the ExtensionUpdateSteps to transition through (the
+ * initial version infomration is *not* included). Returns NIL if no such
+ * path.
*/
static List *
find_update_path(List *evi_list,
@@ -1280,7 +1365,8 @@ find_update_path(List *evi_list,
break; /* found shortest path to target */
foreach(lc, evi->reachable)
{
- ExtensionVersionInfo *evi2 = (ExtensionVersionInfo *) lfirst(lc);
+ ExtensionUpdateStep *step = (ExtensionUpdateStep *) lfirst(lc);
+ ExtensionVersionInfo *evi2 = step->next_version;
int newdist;
/* if reject_indirect, treat installable versions as unreachable */
@@ -1291,6 +1377,7 @@ find_update_path(List *evi_list,
{
evi2->distance = newdist;
evi2->previous = evi;
+ evi2->without_script = step->without_script;
}
else if (newdist == evi2->distance &&
evi2->previous != NULL &&
@@ -1305,6 +1392,16 @@ find_update_path(List *evi_list,
* entries get visited.
*/
evi2->previous = evi;
+ evi2->without_script = step->without_script;
+ }
+ else if (evi == evi2->previous &&
+ evi->without_script != evi2->without_script)
+ {
+ /*
+ * If it is reachable both with and without an update script,
+ * we prefer to use the script.
+ */
+ evi2->without_script = false;
}
}
}
@@ -1313,10 +1410,10 @@ find_update_path(List *evi_list,
if (!evi_target->distance_known)
return NIL;
- /* Build and return list of version names representing the update path */
+ /* Build and return list of update steps representing the update path */
result = NIL;
for (evi = evi_target; evi != evi_start; evi = evi->previous)
- result = lcons(evi->name, result);
+ result = lcons(make_ext_update_step(evi, evi->without_script), result);
return result;
}
@@ -2332,7 +2429,8 @@ pg_extension_update_paths(PG_FUNCTION_ARGS)
appendStringInfoString(&pathbuf, evi1->name);
foreach(lcv, path)
{
- char *versionName = (char *) lfirst(lcv);
+ ExtensionUpdateStep *step = (ExtensionUpdateStep *) lfirst(lcv);
+ char *versionName = step->next_version->name;
appendStringInfoString(&pathbuf, "--");
appendStringInfoString(&pathbuf, versionName);
@@ -3047,7 +3145,8 @@ ApplyExtensionUpdates(Oid extensionOid,
foreach(lcv, updateVersions)
{
- char *versionName = (char *) lfirst(lcv);
+ ExtensionUpdateStep *step = (ExtensionUpdateStep *) lfirst(lcv);
+ char *versionName = step->next_version->name;
ExtensionControlFile *control;
char *schemaName;
Oid schemaOid;
@@ -3167,12 +3266,18 @@ ApplyExtensionUpdates(Oid extensionOid,
InvokeObjectPostAlterHook(ExtensionRelationId, extensionOid, 0);
/*
- * Finally, execute the update script file
+ * Finally, execute the update script file if we have
*/
- execute_extension_script(extensionOid, control,
- oldVersionName, versionName,
- requiredSchemas,
- schemaName, schemaOid);
+ if (!step->without_script)
+ execute_extension_script(extensionOid, control,
+ oldVersionName, versionName,
+ requiredSchemas,
+ schemaName, schemaOid);
+ /*
+ * Otherwise, advance the command counter to make the catalog change visible
+ */
+ else
+ CommandCounterIncrement();
/*
* Update prior-version name and loop around. Since
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index c3139ab0fc..e386b11d9d 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -4,12 +4,13 @@ MODULE = test_extensions
PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
- test_ext7 test_ext8 test_ext_cine test_ext_cor \
+ test_ext7 test_ext8 test_ext9 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
test_ext_evttrig
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
+ test_ext9--1.0.sql test_ext9--2.0--3.0.sql \
test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql \
test_ext_cor--1.0.sql \
test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 821fed38d1..f1788d6c1c 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -121,6 +121,12 @@ Objects in extension "test_ext8"
-- dropping it should still work
drop extension test_ext8;
+-- test updates_without_script
+create extension test_ext9;
+drop extension test_ext9;
+create extension test_ext9 version '1.0';
+alter extension test_ext9 update to '4.0';
+drop extension test_ext9;
-- Test creation of extension in temporary schema with two-phase commit,
-- which should not work. This function wrapper is useful for portability.
-- Avoid noise caused by CONTEXT and NOTICE messages including the temporary
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 41b6cddf0b..16e6c9f94a 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -65,6 +65,14 @@ end';
-- dropping it should still work
drop extension test_ext8;
+-- test updates_without_script
+create extension test_ext9;
+drop extension test_ext9;
+create extension test_ext9 version '1.0';
+alter extension test_ext9 update to '4.0';
+drop extension test_ext9;
+
+
-- Test creation of extension in temporary schema with two-phase commit,
-- which should not work. This function wrapper is useful for portability.
diff --git a/src/test/modules/test_extensions/test_ext9--1.0.sql b/src/test/modules/test_extensions/test_ext9--1.0.sql
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/src/test/modules/test_extensions/test_ext9--2.0--3.0.sql b/src/test/modules/test_extensions/test_ext9--2.0--3.0.sql
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/src/test/modules/test_extensions/test_ext9.control b/src/test/modules/test_extensions/test_ext9.control
new file mode 100644
index 0000000000..5c7b77938e
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext9.control
@@ -0,0 +1,5 @@
+comment = 'Test extension 9'
+default_version = '4.0'
+schema = 'public'
+relocatable = false
+updates_without_script = '1.0--2.0, 3.0--4.0'
--
2.25.1
--Multipart=_Tue__31_Jan_2023_05_25_02_+0900_xx78gCVqd=kbvlGG--
^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2023-01-30 08:36 UTC | newest]
Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-01-09 20:54 [PATCH v16 3/3] Fix replay of create database records on standby Alvaro Herrera <alvherre@alvh.no-ip.org>
2020-01-09 20:54 [PATCH v17 3/3] Fix replay of create database records on standby Alvaro Herrera <alvherre@alvh.no-ip.org>
2020-01-09 20:54 [PATCH v18 3/3] Fix replay of create database records on standby Alvaro Herrera <alvherre@alvh.no-ip.org>
2020-01-09 20:54 [PATCH v13 3/3] Fix replay of create database records on standby Alvaro Herrera <alvherre@alvh.no-ip.org>
2020-01-09 20:54 [PATCH v14 3/3] Fix replay of create database records on standby Alvaro Herrera <alvherre@alvh.no-ip.org>
2020-01-09 20:54 [PATCH v15 3/3] Fix replay of create database records on standby Alvaro Herrera <alvherre@alvh.no-ip.org>
2020-01-09 20:54 [PATCH v15 3/3] Fix replay of create database records on standby Alvaro Herrera <alvherre@alvh.no-ip.org>
2020-01-09 20:54 [PATCH v8 3/3] Fix replay of create database records on standby Alvaro Herrera <alvherre@alvh.no-ip.org>
2022-03-07 08:10 [PATCH v20] Fix replay of create database records on standby P <apraveen@pivotal.io>
2022-03-21 11:34 [PATCH v21] Fix replay of create database records on standby Alvaro Herrera <alvherre@alvh.no-ip.org>
2022-03-28 07:29 [PATCH 2/2] Fix replay of create database records on standby Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-04-05 06:31 [PATCH v22] Fix replay of create database records on standby Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-04-05 06:31 [PATCH v23] Fix replay of create database records on standby Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-07-13 16:14 [PATCH v24] Fix replay of create database records on standby Alvaro Herrera <alvherre@alvh.no-ip.org>
2022-07-13 16:14 [PATCH v25 1/4] Fix replay of create database records on standby Alvaro Herrera <alvherre@alvh.no-ip.org>
2022-07-13 16:14 [PATCH v26] Fix replay of create database records on standby Alvaro Herrera <alvherre@alvh.no-ip.org>
2022-07-27 18:22 [PATCH v27] Fix replay of create database records on standby Alvaro Herrera <alvherre@alvh.no-ip.org>
2023-01-30 08:36 [PATCH] Allow an extention to be updated without a script Yugo Nagata <nagata@sraoss.co.jp>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox