diff --git a/src/backend/storage/file/reinit.c b/src/backend/storage/file/reinit.c index 99c443c7536..00678cb1828 100644 --- a/src/backend/storage/file/reinit.c +++ b/src/backend/storage/file/reinit.c @@ -98,7 +98,9 @@ ResetUnloggedRelations(int op) MemoryContextDelete(tmpctx); } -/* Process one tablespace directory for ResetUnloggedRelations */ +/* + * Process one tablespace directory for ResetUnloggedRelations + */ static void ResetUnloggedRelationsInTablespaceDir(const char *tsdirname, int op) { @@ -107,29 +109,32 @@ ResetUnloggedRelationsInTablespaceDir(const char *tsdirname, int op) char dbspace_path[MAXPGPATH * 2]; ts_dir = AllocateDir(tsdirname); - if (ts_dir == NULL) + + /* + * If we get ENOENT on a tablespace directory, log it and return. This + * can happen if a previous DROP TABLESPACE crashed between removing the + * tablespace directory and removing the symlink in pg_tblspc. We don't + * really want to prevent database startup in that scenario, so let it + * pass instead. Any other type of error will be reported by ReadDir + * (causing a startup failure). + */ + if (ts_dir == NULL && errno == ENOENT) { - /* anything except ENOENT is fishy */ - if (errno != ENOENT) - ereport(LOG, - (errcode_for_file_access(), - errmsg("could not open directory \"%s\": %m", - tsdirname))); + ereport(LOG, + (errcode_for_file_access(), + errmsg("could not open directory \"%s\": %m", + tsdirname))); return; } while ((de = ReadDir(ts_dir, tsdirname)) != NULL) { - int i = 0; - /* * We're only interested in the per-database directories, which have * numeric names. Note that this code will also (properly) ignore "." * and "..". */ - while (isdigit((unsigned char) de->d_name[i])) - ++i; - if (de->d_name[i] != '\0' || i == 0) + if (strspn(de->d_name, "0123456789") != strlen(de->d_name)) continue; snprintf(dbspace_path, sizeof(dbspace_path), "%s/%s", @@ -140,7 +145,9 @@ ResetUnloggedRelationsInTablespaceDir(const char *tsdirname, int op) FreeDir(ts_dir); } -/* Process one per-dbspace directory for ResetUnloggedRelations */ +/* + * Process one per-dbspace directory for ResetUnloggedRelations + */ static void ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op) { @@ -158,20 +165,9 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op) */ if ((op & UNLOGGED_RELATION_CLEANUP) != 0) { - HTAB *hash = NULL; + HTAB *hash; HASHCTL ctl; - /* Open the directory. */ - dbspace_dir = AllocateDir(dbspacedirname); - if (dbspace_dir == NULL) - { - ereport(LOG, - (errcode_for_file_access(), - errmsg("could not open directory \"%s\": %m", - dbspacedirname))); - return; - } - /* * It's possible that someone could create a ton of unlogged relations * in the same database & tablespace, so we'd better use a hash table @@ -179,11 +175,13 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op) * need to be reset. Otherwise, this cleanup operation would be * O(n^2). */ + memset(&ctl, 0, sizeof(ctl)); ctl.keysize = sizeof(unlogged_relation_entry); ctl.entrysize = sizeof(unlogged_relation_entry); hash = hash_create("unlogged hash", 32, &ctl, HASH_ELEM); /* Scan the directory. */ + dbspace_dir = AllocateDir(dbspacedirname); while ((de = ReadDir(dbspace_dir, dbspacedirname)) != NULL) { ForkNumber forkNum; @@ -222,21 +220,9 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op) } /* - * Now, make a second pass and remove anything that matches. First, - * reopen the directory. + * Now, make a second pass and remove anything that matches. */ dbspace_dir = AllocateDir(dbspacedirname); - if (dbspace_dir == NULL) - { - ereport(LOG, - (errcode_for_file_access(), - errmsg("could not open directory \"%s\": %m", - dbspacedirname))); - hash_destroy(hash); - return; - } - - /* Scan the directory. */ while ((de = ReadDir(dbspace_dir, dbspacedirname)) != NULL) { ForkNumber forkNum; @@ -266,15 +252,11 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op) { snprintf(rm_path, sizeof(rm_path), "%s/%s", dbspacedirname, de->d_name); - - /* - * It's tempting to actually throw an error here, but since - * this code gets run during database startup, that could - * result in the database failing to start. (XXX Should we do - * it anyway?) - */ - if (unlink(rm_path)) - elog(LOG, "could not unlink file \"%s\": %m", rm_path); + if (unlink(rm_path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", + rm_path))); else elog(DEBUG2, "unlinked file \"%s\"", rm_path); } @@ -294,19 +276,8 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op) */ if ((op & UNLOGGED_RELATION_INIT) != 0) { - /* Open the directory. */ - dbspace_dir = AllocateDir(dbspacedirname); - if (dbspace_dir == NULL) - { - /* we just saw this directory, so it really ought to be there */ - ereport(LOG, - (errcode_for_file_access(), - errmsg("could not open directory \"%s\": %m", - dbspacedirname))); - return; - } - /* Scan the directory. */ + dbspace_dir = AllocateDir(dbspacedirname); while ((de = ReadDir(dbspace_dir, dbspacedirname)) != NULL) { ForkNumber forkNum; @@ -350,16 +321,6 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op) * (especially the metadata ones) at once. */ dbspace_dir = AllocateDir(dbspacedirname); - if (dbspace_dir == NULL) - { - /* we just saw this directory, so it really ought to be there */ - ereport(LOG, - (errcode_for_file_access(), - errmsg("could not open directory \"%s\": %m", - dbspacedirname))); - return; - } - while ((de = ReadDir(dbspace_dir, dbspacedirname)) != NULL) { ForkNumber forkNum; @@ -388,6 +349,14 @@ ResetUnloggedRelationsInDbspaceDir(const char *dbspacedirname, int op) FreeDir(dbspace_dir); + /* + * Lastly, fsync the database directory itself, ensuring the + * filesystem remembers the file creations and deletions we've done. + * We don't bother with this during a call that does only + * UNLOGGED_RELATION_CLEANUP, because if recovery crashes before we + * get to doing UNLOGGED_RELATION_INIT, we'll redo the cleanup step + * too at the next startup attempt. + */ fsync_fname(dbspacedirname, true); } }