From d4aeb3e17d7fb053af0ca94f5f59ef1ea0c34f98 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Mon, 21 Feb 2022 16:42:16 -0800 Subject: [PATCH v1 3/4] WIP: fix old-fd issues using global barriers everywhere. --- src/include/pg_config_manual.h | 9 --------- src/include/storage/smgr.h | 1 + src/backend/commands/tablespace.c | 10 +++++----- src/backend/storage/buffer/bufmgr.c | 11 ++++++++--- src/backend/storage/smgr/smgr.c | 20 ++++++++++++++++++++ 5 files changed, 34 insertions(+), 17 deletions(-) diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h index 84ce5a4a5d7..1eb39884144 100644 --- a/src/include/pg_config_manual.h +++ b/src/include/pg_config_manual.h @@ -152,16 +152,7 @@ #define EXEC_BACKEND #endif -/* - * If USE_BARRIER_SMGRRELEASE is defined, certain code paths that unlink - * directories will ask other backends to close all smgr file descriptors. - * This is enabled on Windows, because otherwise unlinked but still open files - * can prevent rmdir(containing_directory) from succeeding. On other - * platforms, it can be defined to exercise those code paths. - */ -#if defined(WIN32) #define USE_BARRIER_SMGRRELEASE -#endif /* * Define this if your operating system supports link() diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h index 8e3ef92cda1..d774fc98cf8 100644 --- a/src/include/storage/smgr.h +++ b/src/include/storage/smgr.h @@ -79,6 +79,7 @@ typedef SMgrRelationData *SMgrRelation; extern void smgrinit(void); extern SMgrRelation smgropen(RelFileNode rnode, BackendId backend); +extern SMgrRelation smgropen_cond(RelFileNode rnode, BackendId backend); extern bool smgrexists(SMgrRelation reln, ForkNumber forknum); extern void smgrsetowner(SMgrRelation *owner, SMgrRelation reln); extern void smgrclearowner(SMgrRelation *owner, SMgrRelation reln); diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c index 40514ab550f..0c433360b63 100644 --- a/src/backend/commands/tablespace.c +++ b/src/backend/commands/tablespace.c @@ -1574,6 +1574,11 @@ tblspc_redo(XLogReaderState *record) { xl_tblspc_drop_rec *xlrec = (xl_tblspc_drop_rec *) XLogRecGetData(record); +#if defined(USE_BARRIER_SMGRRELEASE) + /* Close all smgr fds in all backends. */ + WaitForProcSignalBarrier(EmitProcSignalBarrier(PROCSIGNAL_BARRIER_SMGRRELEASE)); +#endif + /* * 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 @@ -1591,11 +1596,6 @@ tblspc_redo(XLogReaderState *record) */ if (!destroy_tablespace_directories(xlrec->ts_id, true)) { -#if defined(USE_BARRIER_SMGRRELEASE) - /* Close all smgr fds in all backends. */ - WaitForProcSignalBarrier(EmitProcSignalBarrier(PROCSIGNAL_BARRIER_SMGRRELEASE)); -#endif - ResolveRecoveryConflictWithTablespace(xlrec->ts_id); /* diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index f5459c68f89..e9fd512df7c 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -4819,9 +4819,14 @@ IssuePendingWritebacks(WritebackContext *context) i += ahead; - /* and finally tell the kernel to write the data to storage */ - reln = smgropen(tag.rnode, InvalidBackendId); - smgrwriteback(reln, tag.forkNum, tag.blockNum, nblocks); + /* + * Finally tell the kernel to write the data to storage. Don't smgr if + * previously closed, otherwise we could end up evading fd-reuse + * protection. + */ + reln = smgropen_cond(tag.rnode, InvalidBackendId); + if (reln) + smgrwriteback(reln, tag.forkNum, tag.blockNum, nblocks); } context->nr_pending = 0; diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c index d71a557a352..efa83ae3942 100644 --- a/src/backend/storage/smgr/smgr.c +++ b/src/backend/storage/smgr/smgr.c @@ -190,6 +190,26 @@ smgropen(RelFileNode rnode, BackendId backend) return reln; } +/* + * Like smgropen(), but returns NULL if relation is not already open. + */ +SMgrRelation +smgropen_cond(RelFileNode rnode, BackendId backend) +{ + RelFileNodeBackend brnode; + + if (SMgrRelationHash == NULL) + return NULL; + + /* Look up or create an entry */ + brnode.node = rnode; + brnode.backend = backend; + + return (SMgrRelation) hash_search(SMgrRelationHash, + (void *) &brnode, + HASH_FIND, NULL); +} + /* * smgrsetowner() -- Establish a long-lived reference to an SMgrRelation object * -- 2.34.0