($INBOX_DIR/description missing)help / color / mirror / Atom feed
[PATCH 5/5] Remove the GUC stats_temp_directory 3+ messages / 3 participants [nested] [flat]
* [PATCH 5/5] Remove the GUC stats_temp_directory @ 2018-11-27 05:42 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Kyotaro Horiguchi @ 2018-11-27 05:42 UTC (permalink / raw) The guc used to specifie the directory to store temporary statistics files. It is no longer needed by the stats collector but still used by the programs in bin and contirb, and maybe other extensions. Thus this patch removes the GUC but some backing variables and macro definitions are left alone for backward comptibility. --- doc/src/sgml/backup.sgml | 2 -- doc/src/sgml/config.sgml | 19 ------------- doc/src/sgml/monitoring.sgml | 7 +---- doc/src/sgml/storage.sgml | 3 +- src/backend/postmaster/pgstat.c | 13 ++++----- src/backend/replication/basebackup.c | 13 ++------- src/backend/utils/misc/guc.c | 41 --------------------------- src/backend/utils/misc/postgresql.conf.sample | 1 - src/include/pgstat.h | 5 +++- src/test/perl/PostgresNode.pm | 4 --- 10 files changed, 14 insertions(+), 94 deletions(-) diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml index 9d4c000df0..bbae70221e 100644 --- a/doc/src/sgml/backup.sgml +++ b/doc/src/sgml/backup.sgml @@ -1145,8 +1145,6 @@ SELECT pg_stop_backup(); <filename>pg_snapshots/</filename>, <filename>pg_stat_tmp/</filename>, and <filename>pg_subtrans/</filename> (but not the directories themselves) can be omitted from the backup as they will be initialized on postmaster startup. - If <xref linkend="guc-stats-temp-directory"/> is set and is under the data - directory then the contents of that directory can also be omitted. </para> <para> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 84341a30e5..6ad619de47 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -6827,25 +6827,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; </listitem> </varlistentry> - <varlistentry id="guc-stats-temp-directory" xreflabel="stats_temp_directory"> - <term><varname>stats_temp_directory</varname> (<type>string</type>) - <indexterm> - <primary><varname>stats_temp_directory</varname> configuration parameter</primary> - </indexterm> - </term> - <listitem> - <para> - Sets the directory to store temporary statistics data in. This can be - a path relative to the data directory or an absolute path. The default - is <filename>pg_stat_tmp</filename>. Pointing this at a RAM-based - file system will decrease physical I/O requirements and can lead to - improved performance. - This parameter can only be set in the <filename>postgresql.conf</filename> - file or on the server command line. - </para> - </listitem> - </varlistentry> - </variablelist> </sect2> diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 990995c17b..2a2adaa0f7 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -195,12 +195,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser <para> The statistics collector transmits the collected information to other - <productname>PostgreSQL</productname> processes through temporary files. - These files are stored in the directory named by the - <xref linkend="guc-stats-temp-directory"/> parameter, - <filename>pg_stat_tmp</filename> by default. - For better performance, <varname>stats_temp_directory</varname> can be - pointed at a RAM-based file system, decreasing physical I/O requirements. + <productname>PostgreSQL</productname> processes through shared memory. When the server shuts down cleanly, a permanent copy of the statistics data is stored in the <filename>pg_stat</filename> subdirectory, so that statistics can be retained across server restarts. When recovery is diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml index 1047c77a63..b47f8d084e 100644 --- a/doc/src/sgml/storage.sgml +++ b/doc/src/sgml/storage.sgml @@ -122,8 +122,7 @@ Item <row> <entry><filename>pg_stat_tmp</filename></entry> - <entry>Subdirectory containing temporary files for the statistics - subsystem</entry> + <entry>Subdirectory containing ephemeral files for extensions</entry> </row> <row> diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index 514ea78a68..64e1374288 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -108,15 +108,12 @@ bool pgstat_track_counts = false; int pgstat_track_functions = TRACK_FUNC_OFF; int pgstat_track_activity_query_size = 1024; -/* ---------- - * Built from GUC parameter - * ---------- +/* + * This used to be a GUC variable and is no longer used in this file, but left + * alone just for backward comptibility for extensions, having the default + * value. */ -char *pgstat_stat_directory = NULL; - -/* No longer used, but will be removed with GUC */ -char *pgstat_stat_filename = NULL; -char *pgstat_stat_tmpname = NULL; +char *pgstat_stat_directory = PG_STAT_TMP_DIR; #define StatsLock (&StatsShmem->StatsMainLock) diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c index c2978a949a..e784088747 100644 --- a/src/backend/replication/basebackup.c +++ b/src/backend/replication/basebackup.c @@ -230,11 +230,8 @@ perform_base_backup(basebackup_options *opt) TimeLineID endtli; StringInfo labelfile; StringInfo tblspc_map_file = NULL; - int datadirpathlen; List *tablespaces = NIL; - datadirpathlen = strlen(DataDir); - backup_started_in_recovery = RecoveryInProgress(); labelfile = makeStringInfo(); @@ -265,13 +262,9 @@ perform_base_backup(basebackup_options *opt) * Calculate the relative path of temporary statistics directory in * order to skip the files which are located in that directory later. */ - if (is_absolute_path(pgstat_stat_directory) && - strncmp(pgstat_stat_directory, DataDir, datadirpathlen) == 0) - statrelpath = psprintf("./%s", pgstat_stat_directory + datadirpathlen + 1); - else if (strncmp(pgstat_stat_directory, "./", 2) != 0) - statrelpath = psprintf("./%s", pgstat_stat_directory); - else - statrelpath = pgstat_stat_directory; + + Assert(strchr(PG_STAT_TMP_DIR, '/') == NULL); + statrelpath = psprintf("./%s", PG_STAT_TMP_DIR); /* Add a node for the base directory at the end */ ti = palloc0(sizeof(tablespaceinfo)); diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 92c4fee8f8..7feb44c39d 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -194,7 +194,6 @@ static bool check_max_wal_senders(int *newval, void **extra, GucSource source); static bool check_autovacuum_work_mem(int *newval, void **extra, GucSource source); static bool check_effective_io_concurrency(int *newval, void **extra, GucSource source); static void assign_effective_io_concurrency(int newval, void *extra); -static void assign_pgstat_temp_directory(const char *newval, void *extra); static bool check_application_name(char **newval, void **extra, GucSource source); static void assign_application_name(const char *newval, void *extra); static bool check_cluster_name(char **newval, void **extra, GucSource source); @@ -4085,17 +4084,6 @@ static struct config_string ConfigureNamesString[] = NULL, NULL, NULL }, - { - {"stats_temp_directory", PGC_SIGHUP, STATS_COLLECTOR, - gettext_noop("Writes temporary statistics files to the specified directory."), - NULL, - GUC_SUPERUSER_ONLY - }, - &pgstat_temp_directory, - PG_STAT_TMP_DIR, - check_canonical_path, assign_pgstat_temp_directory, NULL - }, - { {"synchronous_standby_names", PGC_SIGHUP, REPLICATION_MASTER, gettext_noop("Number of synchronous standbys and list of names of potential synchronous ones."), @@ -11368,35 +11356,6 @@ assign_effective_io_concurrency(int newval, void *extra) #endif /* USE_PREFETCH */ } -static void -assign_pgstat_temp_directory(const char *newval, void *extra) -{ - /* check_canonical_path already canonicalized newval for us */ - char *dname; - char *tname; - char *fname; - - /* directory */ - dname = guc_malloc(ERROR, strlen(newval) + 1); /* runtime dir */ - sprintf(dname, "%s", newval); - - /* global stats */ - tname = guc_malloc(ERROR, strlen(newval) + 12); /* /global.tmp */ - sprintf(tname, "%s/global.tmp", newval); - fname = guc_malloc(ERROR, strlen(newval) + 13); /* /global.stat */ - sprintf(fname, "%s/global.stat", newval); - - if (pgstat_stat_directory) - free(pgstat_stat_directory); - pgstat_stat_directory = dname; - if (pgstat_stat_tmpname) - free(pgstat_stat_tmpname); - pgstat_stat_tmpname = tname; - if (pgstat_stat_filename) - free(pgstat_stat_filename); - pgstat_stat_filename = fname; -} - static bool check_application_name(char **newval, void **extra, GucSource source) { diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 5ee5e09ddf..0ba984b074 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -562,7 +562,6 @@ #track_io_timing = off #track_functions = none # none, pl, all #track_activity_query_size = 1024 # (change requires restart) -#stats_temp_directory = 'pg_stat_tmp' # - Monitoring - diff --git a/src/include/pgstat.h b/src/include/pgstat.h index c0bbf8a7d5..2c7c799ca9 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -31,7 +31,10 @@ #define PGSTAT_STAT_PERMANENT_FILENAME "pg_stat/global.stat" #define PGSTAT_STAT_PERMANENT_TMPFILE "pg_stat/global.tmp" -/* Default directory to store temporary statistics data in */ +/* + * This used to be the directory to store temporary statistics data in but is + * no longer used. Defined here for backward compatibility. + */ #define PG_STAT_TMP_DIR "pg_stat_tmp" /* Values for track_functions GUC variable --- order is significant! */ diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 6019f37f91..a107683c0d 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -451,10 +451,6 @@ sub init print $conf TestLib::slurp_file($ENV{TEMP_CONFIG}) if defined $ENV{TEMP_CONFIG}; - # XXX Neutralize any stats_temp_directory in TEMP_CONFIG. Nodes running - # concurrently must not share a stats_temp_directory. - print $conf "stats_temp_directory = 'pg_stat_tmp'\n"; - if ($params{allows_streaming}) { if ($params{allows_streaming} eq "logical") -- 2.16.3 ----Next_Part(Thu_Jul_04_19_27_54_2019_888)---- ^ permalink raw reply [nested|flat] 3+ messages in thread
* [PATCH v29 2/2] In-place table persistence change @ 2023-09-04 08:23 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Kyotaro Horiguchi @ 2023-09-04 08:23 UTC (permalink / raw) Previously, the command caused a large amount of file I/O due to heap rewrites, even though ALTER TABLE SET UNLOGGED does not require any data rewrites. This patch eliminates the need for rewrites. Additionally, ALTER TABLE SET LOGGED is updated to emit XLOG_FPI records instead of numerous HEAP_INSERTs when wal_level > minimal, reducing resource consumption. --- src/backend/access/rmgrdesc/smgrdesc.c | 12 + src/backend/catalog/storage.c | 338 ++++++++++++++++++++++++- src/backend/commands/tablecmds.c | 268 +++++++++++++++++--- src/backend/storage/buffer/bufmgr.c | 84 ++++++ src/bin/pg_rewind/parsexlog.c | 6 + src/include/catalog/storage_xlog.h | 10 + src/include/storage/bufmgr.h | 3 + src/include/storage/reinit.h | 2 +- src/tools/pgindent/typedefs.list | 1 + 9 files changed, 683 insertions(+), 41 deletions(-) diff --git a/src/backend/access/rmgrdesc/smgrdesc.c b/src/backend/access/rmgrdesc/smgrdesc.c index bd841b96e8..620e02bc26 100644 --- a/src/backend/access/rmgrdesc/smgrdesc.c +++ b/src/backend/access/rmgrdesc/smgrdesc.c @@ -40,6 +40,15 @@ smgr_desc(StringInfo buf, XLogReaderState *record) xlrec->blkno, xlrec->flags); pfree(path); } + else if (info == XLOG_SMGR_BUFPERSISTENCE) + { + xl_smgr_bufpersistence *xlrec = (xl_smgr_bufpersistence *) rec; + char *path = relpathperm(xlrec->rlocator, MAIN_FORKNUM); + + appendStringInfoString(buf, path); + appendStringInfo(buf, " persistence %d", xlrec->persistence); + pfree(path); + } } const char * @@ -55,6 +64,9 @@ smgr_identify(uint8 info) case XLOG_SMGR_TRUNCATE: id = "TRUNCATE"; break; + case XLOG_SMGR_BUFPERSISTENCE: + id = "BUFPERSISTENCE"; + break; } return id; diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c index 1778801bbd..e7c917c50f 100644 --- a/src/backend/catalog/storage.c +++ b/src/backend/catalog/storage.c @@ -71,11 +71,13 @@ typedef struct PendingRelDelete } PendingRelDelete; #define PCOP_UNLINK_FORK (1 << 0) +#define PCOP_SET_PERSISTENCE (1 << 1) typedef struct PendingCleanup { RelFileLocator rlocator; /* relation that need a cleanup */ int op; /* operation mask */ + bool bufpersistence; /* buffer persistence to set */ ForkNumber unlink_forknum; /* forknum to unlink */ BackendId backend; /* InvalidBackendId if not a temp rel */ bool atCommit; /* T=delete at commit; F=delete at abort */ @@ -209,6 +211,208 @@ RelationCreateStorage(RelFileLocator rlocator, char relpersistence, return srel; } +/* + * RelationCreateInitFork + * Create physical storage for the init fork of a relation. + * + * Create the init fork for the relation. + * + * This function is transactional. The creation is WAL-logged, and if the + * transaction aborts later on, the init fork will be removed. + */ +void +RelationCreateInitFork(Relation rel) +{ + RelFileLocator rlocator = rel->rd_locator; + PendingCleanup *pending; + PendingCleanup *prev; + PendingCleanup *next; + SMgrRelation srel; + ul_uncommitted_storage ul_storage; + bool create = true; + + /* switch buffer persistence */ + SetRelationBuffersPersistence(RelationGetSmgr(rel), false, false); + + /* + * If a pending-unlink exists for this relation's init-fork, it indicates + * the init-fork's existed before the current transaction; this function + * reverts the pending-unlink by removing the entry. See + * RelationDropInitFork. + */ + prev = NULL; + for (pending = pendingCleanups; pending != NULL; pending = next) + { + next = pending->next; + + if (RelFileLocatorEquals(rlocator, pending->rlocator) && + pending->unlink_forknum == INIT_FORKNUM) + { + /* write cancel log for preceding undo log entry */ + ul_storage.rlocator = rlocator; + ul_storage.forknum = INIT_FORKNUM; + ul_storage.remove = false; + SimpleUndoLogWrite(RM_SMGR_ID, ULOG_SMGR_UNCOMMITED_STORAGE, + GetCurrentTransactionId(), + &ul_storage, sizeof(ul_storage)); + + if (prev) + prev->next = next; + else + pendingCleanups = next; + + pfree(pending); + /* prev does not change */ + + create = false; + } + else + prev = pending; + } + + if (!create) + return; + + /* create undo log entry, then the init fork */ + srel = smgropen(rlocator, InvalidBackendId); + + /* write undo log */ + ul_storage.rlocator = rlocator; + ul_storage.forknum = INIT_FORKNUM; + ul_storage.remove = true; + SimpleUndoLogWrite(RM_SMGR_ID, ULOG_SMGR_UNCOMMITED_STORAGE, + GetCurrentTransactionId(), + &ul_storage, sizeof(ul_storage)); + + /* We don't have existing init fork, create it. */ + smgrcreate(srel, INIT_FORKNUM, false); + + /* + * For index relations, WAL-logging and file sync are handled by + * ambuildempty. In contrast, for heap relations, these tasks are performed + * directly. + */ + if (rel->rd_rel->relkind == RELKIND_INDEX) + rel->rd_indam->ambuildempty(rel); + else + { + log_smgrcreate(&rlocator, INIT_FORKNUM); + smgrimmedsync(srel, INIT_FORKNUM); + } + + /* drop the init fork, mark file then revert persistence at abort */ + pending = (PendingCleanup *) + MemoryContextAlloc(TopMemoryContext, sizeof(PendingCleanup)); + pending->rlocator = rlocator; + pending->op = PCOP_UNLINK_FORK | PCOP_SET_PERSISTENCE; + pending->unlink_forknum = INIT_FORKNUM; + pending->bufpersistence = true; + pending->backend = InvalidBackendId; + pending->atCommit = false; + pending->nestLevel = GetCurrentTransactionNestLevel(); + pending->next = pendingCleanups; + pendingCleanups = pending; +} + +/* + * RelationDropInitFork + * Delete physical storage for the init fork of a relation. + */ +void +RelationDropInitFork(Relation rel) +{ + RelFileLocator rlocator = rel->rd_locator; + PendingCleanup *pending; + PendingCleanup *prev; + PendingCleanup *next; + bool inxact_created = false; + + /* switch buffer persistence */ + SetRelationBuffersPersistence(RelationGetSmgr(rel), true, false); + + /* + * Search for a pending-unlink associated with the init-fork of the + * relation. Its presence indicates that the init-fork was created within + * the current transaction. + */ + prev = NULL; + for (pending = pendingCleanups; pending != NULL; pending = next) + { + next = pending->next; + + if (RelFileLocatorEquals(rlocator, pending->rlocator) && + pending->unlink_forknum == INIT_FORKNUM) + { + ul_uncommitted_storage ul_storage; + + /* write cancel log for preceding undo log entry */ + ul_storage.rlocator = rlocator; + ul_storage.forknum = INIT_FORKNUM; + ul_storage.remove = false; + SimpleUndoLogWrite(RM_SMGR_ID, ULOG_SMGR_UNCOMMITED_STORAGE, + GetCurrentTransactionId(), + &ul_storage, sizeof(ul_storage)); + + /* unlink list entry */ + if (prev) + prev->next = next; + else + pendingCleanups = next; + + pfree(pending); + + /* prev does not change */ + + inxact_created = true; + } + else + prev = pending; + } + + /* + * If the init-fork was created in this transaction, remove the init-fork + * and cancel preceding undo log. Otherwise, register an at-commit + * pending-unlink for the existing init-fork. See RelationCreateInitFork. + */ + if (inxact_created) + { + SMgrRelation srel = smgropen(rlocator, InvalidBackendId); + ForkNumber forknum = INIT_FORKNUM; + BlockNumber firstblock = 0; + ul_uncommitted_storage ul_storage; + + /* + * Some AMs initialize init-fork via the buffer manager. To properly + * drop the init-fork, first drop all buffers for the init-fork, then + * unlink the init-fork and cancel preceding undo log. + */ + DropRelationBuffers(srel, &forknum, 1, &firstblock); + + /* cancel existing undo log */ + ul_storage.rlocator = rlocator; + ul_storage.forknum = INIT_FORKNUM; + ul_storage.remove = false; + SimpleUndoLogWrite(RM_SMGR_ID, ULOG_SMGR_UNCOMMITED_STORAGE, + GetCurrentTransactionId(), + &ul_storage, sizeof(ul_storage)); + log_smgrunlink(&rlocator, INIT_FORKNUM); + smgrunlink(srel, INIT_FORKNUM, false); + return; + } + + /* register drop of this init fork file at commit */ + pending = (PendingCleanup *) + MemoryContextAlloc(TopMemoryContext, sizeof(PendingCleanup)); + pending->rlocator = rlocator; + pending->op = PCOP_UNLINK_FORK; + pending->unlink_forknum = INIT_FORKNUM; + pending->backend = InvalidBackendId; + pending->atCommit = true; + pending->nestLevel = GetCurrentTransactionNestLevel(); + pending->next = pendingCleanups; + pendingCleanups = pending; +} + /* * Perform XLogInsert of an XLOG_SMGR_CREATE record to WAL. */ @@ -248,6 +452,25 @@ log_smgrunlink(const RelFileLocator *rlocator, ForkNumber forkNum) XLogInsert(RM_SMGR_ID, XLOG_SMGR_UNLINK | XLR_SPECIAL_REL_UPDATE); } +/* + * Perform XLogInsert of an XLOG_SMGR_BUFPERSISTENCE record to WAL. + */ +void +log_smgrbufpersistence(const RelFileLocator rlocator, bool persistence) +{ + xl_smgr_bufpersistence xlrec; + + /* + * Make an XLOG entry reporting the change of buffer persistence. + */ + xlrec.rlocator = rlocator; + xlrec.persistence = persistence; + + XLogBeginInsert(); + XLogRegisterData((char *) &xlrec, sizeof(xlrec)); + XLogInsert(RM_SMGR_ID, XLOG_SMGR_BUFPERSISTENCE | XLR_SPECIAL_REL_UPDATE); +} + /* * RelationDropStorage * Schedule unlinking of physical storage at transaction commit. @@ -800,7 +1023,14 @@ smgrDoPendingCleanups(bool isCommit) srel = smgropen(pending->rlocator, pending->backend); - Assert((pending->op & ~(PCOP_UNLINK_FORK)) == 0); + Assert((pending->op & + ~(PCOP_UNLINK_FORK | PCOP_SET_PERSISTENCE)) == 0); + + if (pending->op & PCOP_SET_PERSISTENCE) + { + SetRelationBuffersPersistence(srel, pending->bufpersistence, + InRecovery); + } if (pending->op & PCOP_UNLINK_FORK) { @@ -1200,6 +1430,112 @@ smgr_redo(XLogReaderState *record) FreeFakeRelcacheEntry(rel); } + else if (info == XLOG_SMGR_BUFPERSISTENCE) + { + xl_smgr_bufpersistence *xlrec = + (xl_smgr_bufpersistence *) XLogRecGetData(record); + SMgrRelation reln; + PendingCleanup *pending; + PendingCleanup *prev = NULL; + + reln = smgropen(xlrec->rlocator, InvalidBackendId); + SetRelationBuffersPersistence(reln, xlrec->persistence, true); + + /* + * Delete any pending action for persistence change, if present. There + * should be at most one entry for this action. + */ + for (pending = pendingCleanups; pending != NULL; + pending = pending->next) + { + if (RelFileLocatorEquals(xlrec->rlocator, pending->rlocator) && + (pending->op & PCOP_SET_PERSISTENCE) != 0) + { + Assert(pending->bufpersistence == xlrec->persistence); + + if (prev) + prev->next = pending->next; + else + pendingCleanups = pending->next; + + pfree(pending); + break; + } + + prev = pending; + } + + /* + * During abort, revert any changes to buffer persistence made made in + * this transaction. + */ + if (!pending) + { + pending = (PendingCleanup *) + MemoryContextAlloc(TopMemoryContext, sizeof(PendingCleanup)); + pending->rlocator = xlrec->rlocator; + pending->op = PCOP_SET_PERSISTENCE; + pending->bufpersistence = !xlrec->persistence; + pending->backend = InvalidBackendId; + pending->atCommit = false; + pending->nestLevel = GetCurrentTransactionNestLevel(); + pending->next = pendingCleanups; + pendingCleanups = pending; + } + } + else if (info == XLOG_SMGR_BUFPERSISTENCE) + { + xl_smgr_bufpersistence *xlrec = + (xl_smgr_bufpersistence *) XLogRecGetData(record); + SMgrRelation reln; + PendingCleanup *pending; + PendingCleanup *prev = NULL; + + reln = smgropen(xlrec->rlocator, InvalidBackendId); + SetRelationBuffersPersistence(reln, xlrec->persistence, true); + + /* + * Delete any pending action for persistence change, if present. There + * should be at most one entry for this action. + */ + for (pending = pendingCleanups; pending != NULL; + pending = pending->next) + { + if (RelFileLocatorEquals(xlrec->rlocator, pending->rlocator) && + (pending->op & PCOP_SET_PERSISTENCE) != 0) + { + Assert(pending->bufpersistence == xlrec->persistence); + + if (prev) + prev->next = pending->next; + else + pendingCleanups = pending->next; + + pfree(pending); + break; + } + + prev = pending; + } + + /* + * During abort, revert any changes to buffer persistence made made in + * this transaction. + */ + if (!pending) + { + pending = (PendingCleanup *) + MemoryContextAlloc(TopMemoryContext, sizeof(PendingCleanup)); + pending->rlocator = xlrec->rlocator; + pending->op = PCOP_SET_PERSISTENCE; + pending->bufpersistence = !xlrec->persistence; + pending->backend = InvalidBackendId; + pending->atCommit = false; + pending->nestLevel = GetCurrentTransactionNestLevel(); + pending->next = pendingCleanups; + pendingCleanups = pending; + } + } else elog(PANIC, "smgr_redo: unknown op code %u", info); } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 47c556669f..6c4cfbfa78 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -55,6 +55,7 @@ #include "commands/defrem.h" #include "commands/event_trigger.h" #include "commands/policy.h" +#include "commands/progress.h" #include "commands/sequence.h" #include "commands/tablecmds.h" #include "commands/tablespace.h" @@ -5571,6 +5572,188 @@ ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, return newcmd; } +/* + * RelationChangePersistence: perform in-place persistence change of a relation + */ +static void +RelationChangePersistence(AlteredTableInfo *tab, char persistence, + LOCKMODE lockmode) +{ + Relation rel; + Relation classRel; + HeapTuple tuple, + newtuple; + Datum new_val[Natts_pg_class]; + bool new_null[Natts_pg_class], + new_repl[Natts_pg_class]; + int i; + List *relids; + ListCell *lc_oid; + + Assert(tab->rewrite == AT_REWRITE_ALTER_PERSISTENCE); + Assert(lockmode == AccessExclusiveLock); + + /* + * Use ATRewriteTable instead of this function under the following + * condition. + */ + Assert(tab->constraints == NULL && tab->partition_constraint == NULL && + tab->newvals == NULL && !tab->verify_new_notnull); + + rel = table_open(tab->relid, lockmode); + + Assert(rel->rd_rel->relpersistence != persistence); + + elog(DEBUG1, "perform in-place persistence change"); + + /* + * Initially, gather all relations that require a persistence change. + */ + + /* Collect OIDs of indexes and toast relations */ + relids = RelationGetIndexList(rel); + relids = lcons_oid(rel->rd_id, relids); + + /* Add toast relation if any */ + if (OidIsValid(rel->rd_rel->reltoastrelid)) + { + List *toastidx; + Relation toastrel = table_open(rel->rd_rel->reltoastrelid, lockmode); + + relids = lappend_oid(relids, rel->rd_rel->reltoastrelid); + toastidx = RelationGetIndexList(toastrel); + relids = list_concat(relids, toastidx); + pfree(toastidx); + table_close(toastrel, NoLock); + } + + table_close(rel, NoLock); + + /* Make changes in storage */ + classRel = table_open(RelationRelationId, RowExclusiveLock); + + foreach(lc_oid, relids) + { + Oid reloid = lfirst_oid(lc_oid); + Relation r = relation_open(reloid, lockmode); + + /* + * XXXX: Some access methods don't support in-place persistence + * changes. GiST uses page LSNs to figure out whether a block has been + * modified. However, UNLOGGED GiST indexes use fake LSNs, which are + * incompatible with the real LSNs used for LOGGED indexes. + * + * Potentially, if gistGetFakeLSN behaved similarly for both permanent + * and unlogged indexes, we could avoid index rebuilds by emitting + * extra WAL records while the index is unlogged. + * + * Compare relam against a positive list to ensure the hard way is + * taken for unknown AMs. + */ + if (r->rd_rel->relkind == RELKIND_INDEX && + /* GiST is excluded */ + r->rd_rel->relam != BTREE_AM_OID && + r->rd_rel->relam != HASH_AM_OID && + r->rd_rel->relam != GIN_AM_OID && + r->rd_rel->relam != SPGIST_AM_OID && + r->rd_rel->relam != BRIN_AM_OID) + { + int reindex_flags; + ReindexParams params = {0}; + + /* reindex doesn't allow concurrent use of the index */ + table_close(r, NoLock); + + reindex_flags = + REINDEX_REL_SUPPRESS_INDEX_USE | + REINDEX_REL_CHECK_CONSTRAINTS; + + /* Set the same persistence with the parent relation. */ + if (persistence == RELPERSISTENCE_UNLOGGED) + reindex_flags |= REINDEX_REL_FORCE_INDEXES_UNLOGGED; + else + reindex_flags |= REINDEX_REL_FORCE_INDEXES_PERMANENT; + + reindex_index(reloid, reindex_flags, persistence, ¶ms); + + continue; + } + + /* Create or drop init fork */ + if (persistence == RELPERSISTENCE_UNLOGGED) + RelationCreateInitFork(r); + else + RelationDropInitFork(r); + + /* + * If this relation becomes WAL-logged, immediately sync all files + * except the init-fork to establish the initial state on storage. The + * buffers should have already been flushed out by + * RelationCreate(Drop)InitFork called just above. The init-fork should + * already be synchronized as required. + */ + if (persistence == RELPERSISTENCE_PERMANENT) + { + for (i = 0; i < INIT_FORKNUM; i++) + { + if (smgrexists(RelationGetSmgr(r), i)) + smgrimmedsync(RelationGetSmgr(r), i); + } + } + + /* Update catalog */ + tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for relation %u", reloid); + + memset(new_val, 0, sizeof(new_val)); + memset(new_null, false, sizeof(new_null)); + memset(new_repl, false, sizeof(new_repl)); + + new_val[Anum_pg_class_relpersistence - 1] = CharGetDatum(persistence); + new_null[Anum_pg_class_relpersistence - 1] = false; + new_repl[Anum_pg_class_relpersistence - 1] = true; + + newtuple = heap_modify_tuple(tuple, RelationGetDescr(classRel), + new_val, new_null, new_repl); + + CatalogTupleUpdate(classRel, &newtuple->t_self, newtuple); + heap_freetuple(newtuple); + + /* + * If wal_level >= replica, switching to LOGGED necessitates WAL-logging + * the relation content for later recovery. This is not emitted when + * wal_level = minimal. + */ + if (persistence == RELPERSISTENCE_PERMANENT && XLogIsNeeded()) + { + ForkNumber fork; + xl_smgr_truncate xlrec; + + xlrec.blkno = 0; + xlrec.rlocator = r->rd_locator; + xlrec.flags = SMGR_TRUNCATE_ALL; + + XLogBeginInsert(); + XLogRegisterData((char *) &xlrec, sizeof(xlrec)); + + XLogInsert(RM_SMGR_ID, XLOG_SMGR_TRUNCATE | XLR_SPECIAL_REL_UPDATE); + + for (fork = 0; fork < INIT_FORKNUM; fork++) + { + if (smgrexists(RelationGetSmgr(r), fork)) + log_newpage_range(r, fork, 0, + smgrnblocks(RelationGetSmgr(r), fork), + false); + } + } + + table_close(r, NoLock); + } + + table_close(classRel, NoLock); +} + /* * ATRewriteTables: ALTER TABLE phase 3 */ @@ -5701,48 +5884,55 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, tab->relid, tab->rewrite); - /* - * Create transient table that will receive the modified data. - * - * Ensure it is marked correctly as logged or unlogged. We have - * to do this here so that buffers for the new relfilenumber will - * have the right persistence set, and at the same time ensure - * that the original filenumbers's buffers will get read in with - * the correct setting (i.e. the original one). Otherwise a - * rollback after the rewrite would possibly result with buffers - * for the original filenumbers having the wrong persistence - * setting. - * - * NB: This relies on swap_relation_files() also swapping the - * persistence. That wouldn't work for pg_class, but that can't be - * unlogged anyway. - */ - OIDNewHeap = make_new_heap(tab->relid, NewTableSpace, NewAccessMethod, - persistence, lockmode); + if (tab->rewrite == AT_REWRITE_ALTER_PERSISTENCE) + RelationChangePersistence(tab, persistence, lockmode); + else + { + /* + * Create transient table that will receive the modified data. + * + * Ensure it is marked correctly as logged or unlogged. We + * have to do this here so that buffers for the new + * relfilenumber will have the right persistence set, and at + * the same time ensure that the original filenumbers's buffers + * will get read in with the correct setting (i.e. the original + * one). Otherwise a rollback after the rewrite would possibly + * result with buffers for the original filenumbers having the + * wrong persistence setting. + * + * NB: This relies on swap_relation_files() also swapping the + * persistence. That wouldn't work for pg_class, but that + * can't be unlogged anyway. + */ + OIDNewHeap = make_new_heap(tab->relid, NewTableSpace, + NewAccessMethod, + persistence, lockmode); - /* - * Copy the heap data into the new table with the desired - * modifications, and test the current data within the table - * against new constraints generated by ALTER TABLE commands. - */ - ATRewriteTable(tab, OIDNewHeap, lockmode); + /* + * Copy the heap data into the new table with the desired + * modifications, and test the current data within the table + * against new constraints generated by ALTER TABLE commands. + */ + ATRewriteTable(tab, OIDNewHeap, lockmode); - /* - * Swap the physical files of the old and new heaps, then rebuild - * indexes and discard the old heap. We can use RecentXmin for - * the table's new relfrozenxid because we rewrote all the tuples - * in ATRewriteTable, so no older Xid remains in the table. Also, - * we never try to swap toast tables by content, since we have no - * interest in letting this code work on system catalogs. - */ - finish_heap_swap(tab->relid, OIDNewHeap, - false, false, true, - !OidIsValid(tab->newTableSpace), - RecentXmin, - ReadNextMultiXactId(), - persistence); + /* + * Swap the physical files of the old and new heaps, then + * rebuild indexes and discard the old heap. We can use + * RecentXmin for the table's new relfrozenxid because we + * rewrote all the tuples in ATRewriteTable, so no older Xid + * remains in the table. Also, we never try to swap toast + * tables by content, since we have no interest in letting + * this code work on system catalogs. + */ + finish_heap_swap(tab->relid, OIDNewHeap, + false, false, true, + !OidIsValid(tab->newTableSpace), + RecentXmin, + ReadNextMultiXactId(), + persistence); - InvokeObjectPostAlterHook(RelationRelationId, tab->relid, 0); + InvokeObjectPostAlterHook(RelationRelationId, tab->relid, 0); + } } else if (tab->rewrite > 0 && tab->relkind == RELKIND_SEQUENCE) { diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 3bd82dbfca..04ab6ec8a7 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -3708,6 +3708,90 @@ DropRelationBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum, } } +/* --------------------------------------------------------------------- + * SetRelationBuffersPersistence + * + * This function changes the persistence of all buffer pages of a relation + * then writes all dirty pages to disk (or kernel disk buffers) when + * switching to PERMANENT, ensuring the kernel has an up-to-date view of + * the relation. + * + * The caller must be holding AccessExclusiveLock on the target relation + * to ensure no other backend is busy dirtying more blocks. + * + * XXX currently it sequentially searches the buffer pool; consider + * implementing more efficient search methods. This routine isn't used in + * performance-critical code paths, so it's not worth additional overhead + * to make it go faster; see also DropRelationBuffers. + * -------------------------------------------------------------------- + */ +void +SetRelationBuffersPersistence(SMgrRelation srel, bool permanent, bool isRedo) +{ + int i; + RelFileLocatorBackend rlocator = srel->smgr_rlocator; + + Assert(!RelFileLocatorBackendIsTemp(rlocator)); + + if (!isRedo) + log_smgrbufpersistence(srel->smgr_rlocator.locator, permanent); + + ResourceOwnerEnlargeBuffers(CurrentResourceOwner); + + for (i = 0; i < NBuffers; i++) + { + BufferDesc *bufHdr = GetBufferDescriptor(i); + uint32 buf_state; + + if (!RelFileLocatorEquals(BufTagGetRelFileLocator(&bufHdr->tag), + rlocator.locator)) + continue; + + ReservePrivateRefCountEntry(); + + buf_state = LockBufHdr(bufHdr); + + if (!RelFileLocatorEquals(BufTagGetRelFileLocator(&bufHdr->tag), + rlocator.locator)) + { + UnlockBufHdr(bufHdr, buf_state); + continue; + } + + if (permanent) + { + /* The init fork is being dropped, drop buffers for it. */ + if (BufTagGetForkNum(&bufHdr->tag) == INIT_FORKNUM) + { + InvalidateBuffer(bufHdr); + continue; + } + + buf_state |= BM_PERMANENT; + pg_atomic_write_u32(&bufHdr->state, buf_state); + + /* flush this buffer when switching to PERMANENT */ + if ((buf_state & (BM_VALID | BM_DIRTY)) == (BM_VALID | BM_DIRTY)) + { + PinBuffer_Locked(bufHdr); + LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), + LW_SHARED); + FlushBuffer(bufHdr, srel, IOOBJECT_RELATION, IOCONTEXT_NORMAL); + LWLockRelease(BufferDescriptorGetContentLock(bufHdr)); + UnpinBuffer(bufHdr); + } + else + UnlockBufHdr(bufHdr, buf_state); + } + else + { + /* There shouldn't be an init fork */ + Assert(BufTagGetForkNum(&bufHdr->tag) != INIT_FORKNUM); + UnlockBufHdr(bufHdr, buf_state); + } + } +} + /* --------------------------------------------------------------------- * DropRelationsAllBuffers * diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index 87b4659e27..db12f4f397 100644 --- a/src/bin/pg_rewind/parsexlog.c +++ b/src/bin/pg_rewind/parsexlog.c @@ -418,6 +418,12 @@ extractPageInfo(XLogReaderState *record) * source system. */ } + else if (rmid == RM_SMGR_ID && rminfo == XLOG_SMGR_BUFPERSISTENCE) + { + /* + * We can safely ignore these. These don't make any on-disk changes. + */ + } else if (rmid == RM_XACT_ID && ((rminfo & XLOG_XACT_OPMASK) == XLOG_XACT_COMMIT || (rminfo & XLOG_XACT_OPMASK) == XLOG_XACT_COMMIT_PREPARED || diff --git a/src/include/catalog/storage_xlog.h b/src/include/catalog/storage_xlog.h index 5122f5b61d..eaa162f0c7 100644 --- a/src/include/catalog/storage_xlog.h +++ b/src/include/catalog/storage_xlog.h @@ -14,6 +14,7 @@ #ifndef STORAGE_XLOG_H #define STORAGE_XLOG_H +#include "access/simpleundolog.h" #include "access/xlogreader.h" #include "lib/stringinfo.h" #include "storage/block.h" @@ -30,6 +31,7 @@ #define XLOG_SMGR_CREATE 0x10 #define XLOG_SMGR_TRUNCATE 0x20 #define XLOG_SMGR_UNLINK 0x30 +#define XLOG_SMGR_BUFPERSISTENCE 0x40 typedef struct xl_smgr_create { @@ -44,6 +46,12 @@ typedef struct xl_smgr_unlink ForkNumber forkNum; } xl_smgr_unlink; +typedef struct xl_smgr_bufpersistence +{ + RelFileLocator rlocator; + bool persistence; +} xl_smgr_bufpersistence; + /* flags for xl_smgr_truncate */ #define SMGR_TRUNCATE_HEAP 0x0001 #define SMGR_TRUNCATE_VM 0x0002 @@ -60,6 +68,8 @@ typedef struct xl_smgr_truncate extern void log_smgrcreate(const RelFileLocator *rlocator, ForkNumber forkNum); extern void log_smgrunlink(const RelFileLocator *rlocator, ForkNumber forkNum); +extern void log_smgrbufpersistence(const RelFileLocator rlocator, + bool persistence); extern void smgr_redo(XLogReaderState *record); extern void smgr_desc(StringInfo buf, XLogReaderState *record); diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h index b379c76e27..0e4e290392 100644 --- a/src/include/storage/bufmgr.h +++ b/src/include/storage/bufmgr.h @@ -222,6 +222,9 @@ extern void DropRelationBuffers(struct SMgrRelationData *smgr_reln, int nforks, BlockNumber *firstDelBlock); extern void DropRelationsAllBuffers(struct SMgrRelationData **smgr_reln, int nlocators); +extern void SetRelationBuffersPersistence(struct SMgrRelationData *srel, + bool permanent, bool isRedo); + extern void DropDatabaseBuffers(Oid dbid); #define RelationGetNumberOfBlocks(reln) \ diff --git a/src/include/storage/reinit.h b/src/include/storage/reinit.h index ccd182531d..e59fb7892e 100644 --- a/src/include/storage/reinit.h +++ b/src/include/storage/reinit.h @@ -20,10 +20,10 @@ extern void ResetUnloggedRelations(int op); -extern void ResetUnloggedRelationIgnore(RelFileLocator rloc); extern bool parse_filename_for_nontemp_relation(const char *name, int *relnumchars, ForkNumber *fork); +extern void ResetUnloggedRelationIgnore(RelFileLocator rloc); #define UNLOGGED_RELATION_CLEANUP 0x0001 #define UNLOGGED_RELATION_INIT 0x0002 diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index b9255e5e25..2c34434555 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3941,6 +3941,7 @@ xl_replorigin_set xl_restore_point xl_running_xacts xl_seq_rec +xl_smgr_bufpersistence xl_smgr_create xl_smgr_truncate xl_smgr_unlink -- 2.39.3 ----Next_Part(Mon_Sep__4_17_37_48_2023_852)---- ^ permalink raw reply [nested|flat] 3+ messages in thread
* [PATCH v1] aio: io_uring: Fix danger of completion getting reused before being read @ 2026-01-15 15:17 Andres Freund <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Andres Freund @ 2026-01-15 15:17 UTC (permalink / raw) We called io_uring_cqe_seen(..., cqe) before reading cqe->res. That allows the completion to be reused, which in turn could lead to cqe->res being overwritten. The window for that is very narrow and the likelihood of it happening is very low, as we should never actually utilize all CQEs, but the consequences would be bad. This bug was reported to me privately. Backpatch-through: 18 --- src/backend/storage/aio/method_io_uring.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/backend/storage/aio/method_io_uring.c b/src/backend/storage/aio/method_io_uring.c index af58c6118ac..f2527ae61dd 100644 --- a/src/backend/storage/aio/method_io_uring.c +++ b/src/backend/storage/aio/method_io_uring.c @@ -559,13 +559,14 @@ pgaio_uring_drain_locked(PgAioUringContext *context) for (int i = 0; i < ncqes; i++) { struct io_uring_cqe *cqe = cqes[i]; - PgAioHandle *ioh; + PgAioHandle *ioh = io_uring_cqe_get_data(cqe); + int result = cqe->res; - ioh = io_uring_cqe_get_data(cqe); errcallback.arg = ioh; + io_uring_cqe_seen(&context->io_uring_ring, cqe); - pgaio_io_process_completion(ioh, cqe->res); + pgaio_io_process_completion(ioh, result); errcallback.arg = NULL; } -- 2.48.1.76.g4e746b1a31.dirty --ppazo27qa47camgo-- ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-01-15 15:17 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2018-11-27 05:42 [PATCH 5/5] Remove the GUC stats_temp_directory Kyotaro Horiguchi <[email protected]> 2023-09-04 08:23 [PATCH v29 2/2] In-place table persistence change Kyotaro Horiguchi <[email protected]> 2026-01-15 15:17 [PATCH v1] aio: io_uring: Fix danger of completion getting reused before being read Andres Freund <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox