From 34daf017c9ace03326e4151b00200ab4a0c123f0 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Fri, 17 Dec 2021 12:05:14 -0600 Subject: [PATCH 2/2] fixes from justin --- src/backend/access/transam/README | 2 +- src/backend/catalog/storage.c | 10 +++++----- src/backend/commands/tablecmds.c | 15 +++++++-------- src/backend/storage/file/reinit.c | 8 ++++---- src/backend/storage/smgr/md.c | 8 ++++---- src/common/relpath.c | 4 ++-- src/include/storage/smgr.h | 6 +++--- 7 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/backend/access/transam/README b/src/backend/access/transam/README index 7cf77e4a02..5c0fd3f489 100644 --- a/src/backend/access/transam/README +++ b/src/backend/access/transam/README @@ -728,7 +728,7 @@ The Smgr MARK files -------------------------------- A smgr mark files is created when a new relation file is created to -mark the relfilenode needs to be cleaned up at recovery time. In +mark that the relfilenode needs to be cleaned up at recovery time. In contrast to 4 above, failure to remove smgr mark files will lead to data loss, in which case the server will shut down. diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c index a3e250515c..9ff1520946 100644 --- a/src/backend/catalog/storage.c +++ b/src/backend/catalog/storage.c @@ -174,7 +174,7 @@ RelationCreateStorage(RelFileNode rnode, char relpersistence) /* * We are going to create a new storage file. If server crashes before the - * current transaction ends the file needs to be cleaned up but there's no + * current transaction ends, the file needs to be cleaned up but there's no * clue to the orphan files. The SMGR_MARK_UNCOMMITED mark file works as * the signal of that situation. */ @@ -188,7 +188,7 @@ RelationCreateStorage(RelFileNode rnode, char relpersistence) /* * Add the relation to the list of stuff to delete at abort. We don't - * remove the mark file at commit. It needs to persiste until the main fork + * remove the mark file at commit. It needs to persist until the main fork * file is actually deleted. See SyncPostCheckpoint. */ pending = (PendingRelDelete *) @@ -280,7 +280,7 @@ RelationCreateInitFork(Relation rel) /* * We are going to create the init fork. If server crashes before the - * current transaction ends the init fork left alone corrupts data while + * current transaction ends, the init fork left alone corrupts data during * recovery. The cleanup fork works as the sentinel to identify that * situation. */ @@ -292,7 +292,7 @@ RelationCreateInitFork(Relation rel) smgrcreate(srel, INIT_FORKNUM, false); /* - * index-init fork needs further initialization. ambuildempty shoud do + * index-init fork needs further initialization. ambuildempty should do * WAL-log and file sync by itself but otherwise we do that by ourselves. */ if (rel->rd_rel->relkind == RELKIND_INDEX) @@ -357,7 +357,7 @@ RelationDropInitFork(Relation rel) * If we have entries for init-fork operations of this relation, that means * that we have created the init fork in the current transaction. We * remove the init fork and mark file immediately in that case. Otherwise - * just reister pending-delete for the existing init fork. + * just register pending-delete for the existing init fork. */ prev = NULL; for (pending = pendingDeletes; pending != NULL; pending = next) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index dbfbf12f64..a9bcf90960 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5362,7 +5362,7 @@ RelationChangePersistence(AlteredTableInfo *tab, char persistence, Assert(rel->rd_rel->relpersistence != persistence); - elog(DEBUG1, "perform im-place persistnce change"); + elog(DEBUG1, "perform in-place persistence change"); RelationGetSmgr(rel); @@ -5379,7 +5379,7 @@ RelationChangePersistence(AlteredTableInfo *tab, char persistence, { List *toastidx; Relation toastrel = table_open(rel->rd_rel->reltoastrelid, lockmode); - + RelationGetSmgr(toastrel); relids = lappend_oid(relids, rel->rd_rel->reltoastrelid); toastidx = RelationGetIndexList(toastrel); @@ -5397,8 +5397,8 @@ RelationChangePersistence(AlteredTableInfo *tab, char persistence, { Oid reloid = lfirst_oid(lc_oid); Relation r = relation_open(reloid, lockmode); - - /* + + /* * Some access methods do not accept in-place persistence change. For * example, GiST uses page LSNs to figure out whether a block has * changed, where UNLOGGED GiST indexes use fake LSNs that are @@ -5637,7 +5637,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, * persistence. That wouldn't work for pg_class, but that can't be * unlogged anyway. */ - + if (tab->rewrite == AT_REWRITE_ALTER_PERSISTENCE) RelationChangePersistence(tab, persistence, lockmode); else @@ -5660,7 +5660,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, */ 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 @@ -5685,7 +5685,6 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, InvokeObjectPostAlterHook(RelationRelationId, tab->relid, 0); } - } else { @@ -14510,7 +14509,7 @@ AlterTableMoveAll(AlterTableMoveAllStmt *stmt) * * Allows a user to change persistence of all objects in a given tablespace in * the current database. Objects can be chosen based on the owner of the - * object also, to allow users to change persistene only their objects. The + * object also, to allow users to change persistence only their objects. The * main permissions handling is done by the lower-level change persistence * function. * diff --git a/src/backend/storage/file/reinit.c b/src/backend/storage/file/reinit.c index a34aa8e9af..acbcf606ce 100644 --- a/src/backend/storage/file/reinit.c +++ b/src/backend/storage/file/reinit.c @@ -54,7 +54,7 @@ typedef struct * If SMGR_MARK_UNCOMMITTED mark file for main fork is present we remove the * whole relation along with the mark file. * - * Otherwise, if the "init" fork is found. we remove all forks of any relation + * Otherwise, if the "init" fork is found, we remove all forks of any relation * with the "init" fork, except for the "init" fork itself. * * If op includes UNLOGGED_RELATION_DROP_BUFFER, we drop all buffers for all @@ -284,7 +284,7 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, * When we come here after recovery, smgr object for this file might * have been created. In that case we need to drop all buffers then the * smgr object before initializing the unlogged relation. This is safe - * as far as no other backends have accessed the relation before + * as long as no other backends have accessed the relation before * starting archive recovery. */ HASH_SEQ_STATUS status; @@ -302,7 +302,7 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, RelFileNodeBackend rel; /* - * The relation is persistent and stays remain persistent. Don't + * The relation is persistent and stays persistent. Don't * drop the buffers for this relation. */ if (ent->has_init && ent->dirty_init) @@ -367,7 +367,7 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, if (ent->dirty_init) { /* - * The crashed trasaction did SET UNLOGGED. This relation + * The crashed transaction did SET UNLOGGED. This relation * is restored to a LOGGED relation. */ if (forkNum != INIT_FORKNUM) diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c index 27ca9c1ca2..492bd91c9e 100644 --- a/src/backend/storage/smgr/md.c +++ b/src/backend/storage/smgr/md.c @@ -193,15 +193,15 @@ mdcreatemark(SMgrRelation reln, ForkNumber forkNum, StorageMarks mark, (errcode_for_file_access(), errmsg("could not crete mark file \"%s\": %m", path))); - pfree(path); - pg_fsync(fd); - close(fd); - /* * To guarantee that the creation of the file is persistent, fsync its * parent directory. */ fsync_parent_path(path, ERROR); + + pfree(path); + pg_fsync(fd); + close(fd); } diff --git a/src/common/relpath.c b/src/common/relpath.c index 67f24890d6..4b1de9cf1e 100644 --- a/src/common/relpath.c +++ b/src/common/relpath.c @@ -142,12 +142,12 @@ GetRelationPath(Oid dbNode, Oid spcNode, Oid relNode, int backendId, ForkNumber forkNumber, char mark) { char *path; - char markstr[10]; + char markstr[3]; if (mark == 0) markstr[0] = 0; else - snprintf(markstr, 10, ".%c", mark); + snprintf(markstr, sizeof(markstr), ".%c", mark); if (spcNode == GLOBALTABLESPACE_OID) { diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h index 201ecace8a..c49b3142eb 100644 --- a/src/include/storage/smgr.h +++ b/src/include/storage/smgr.h @@ -19,9 +19,9 @@ #include "storage/relfilenode.h" /* - * Storage marks is a file of which existence suggests something about a - * file. The name of such files is ".", where the mark is one - * of the values of StorageMarks. Since "." means segment files so don't + * Storage marks is a file whose existence suggests something about a file. + * The name of such files is ".", where the mark is one + * of the values of StorageMarks. Since "." means segment files, don't * use digits for the mark character. */ typedef enum StorageMarks -- 2.17.0