diff --git a/src/backend/utils/cache/inval.c b/src/backend/utils/cache/inval.c index 5fac924207..29b9c19f0e 100644 --- a/src/backend/utils/cache/inval.c +++ b/src/backend/utils/cache/inval.c @@ -840,10 +840,10 @@ inval_twophase_postcommit(TransactionId xid, uint16 info, SendSharedInvalidMessages(msg, 1); break; case TWOPHASE_INFO_FILE_BEFORE: - RelationCacheInitFileInvalidate(true); + RelationCacheInitFilePreInvalidate(); break; case TWOPHASE_INFO_FILE_AFTER: - RelationCacheInitFileInvalidate(false); + RelationCacheInitFilePostInvalidate(); break; default: Assert(false); @@ -890,7 +890,7 @@ AtEOXact_Inval(bool isCommit) * unless we committed. */ if (transInvalInfo->RelcacheInitFileInval) - RelationCacheInitFileInvalidate(true); + RelationCacheInitFilePreInvalidate(); AppendInvalidationMessages(&transInvalInfo->PriorCmdInvalidMsgs, &transInvalInfo->CurrentCmdInvalidMsgs); @@ -899,7 +899,7 @@ AtEOXact_Inval(bool isCommit) SendSharedInvalidMessages); if (transInvalInfo->RelcacheInitFileInval) - RelationCacheInitFileInvalidate(false); + RelationCacheInitFilePostInvalidate(); } else if (transInvalInfo != NULL) { diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index ea8d458547..76dfa66e34 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -3957,8 +3957,8 @@ write_relcache_init_file(void) * updated by SI message processing, but we can't be sure whether what we * wrote out was up-to-date.) * - * This mustn't run concurrently with RelationCacheInitFileInvalidate, so - * grab a serialization lock for the duration. + * This mustn't run concurrently with the code that unlinks an init file + * and sends SI messages, so grab a serialization lock for the duration. */ LWLockAcquire(RelCacheInitLock, LW_EXCLUSIVE); @@ -4022,49 +4022,55 @@ RelationIdIsInInitFile(Oid relationId) * changed one or more of the relation cache entries that are kept in the * init file. * - * We actually need to remove the init file twice: once just before sending - * the SI messages that include relcache inval for such relations, and once - * just after sending them. The unlink before ensures that a backend that's - * currently starting cannot read the now-obsolete init file and then miss - * the SI messages that will force it to update its relcache entries. (This - * works because the backend startup sequence gets into the PGPROC array before - * trying to load the init file.) The unlink after is to synchronize with a - * backend that may currently be trying to write an init file based on data - * that we've just rendered invalid. Such a backend will see the SI messages, - * but we can't leave the init file sitting around to fool later backends. + * To be safe against concurrent inspection or rewriting of the init file, + * we must take RelCacheInitLock, then remove the old init file, then send + * the SI messages that include relcache inval for such relations, and then + * release RelCacheInitLock. This serializes the whole affair against + * write_relcache_init_file, so that we can be sure that any other process + * that's concurrently trying to create a new init file won't move an + * already-stale version into place after we unlink. Also, because we unlink + * before sending the SI messages, a backend that's currently starting cannot + * read the now-obsolete init file and then miss the SI messages that will + * force it to update its relcache entries. (This works because the backend + * startup sequence gets into the sinval array before trying to load the init + * file.) * - * Ignore any failure to unlink the file, since it might not be there if - * no backend has been started since the last removal. + * We take the lock and do the unlink in RelationCacheInitFilePreInvalidate, + * then release the lock in RelationCacheInitFilePostInvalidate. Caller must + * send any pending SI messages between those calls. */ void -RelationCacheInitFileInvalidate(bool beforeSend) +RelationCacheInitFilePreInvalidate(void) { char initfilename[MAXPGPATH]; snprintf(initfilename, sizeof(initfilename), "%s/%s", DatabasePath, RELCACHE_INIT_FILENAME); - if (beforeSend) - { - /* no interlock needed here */ - unlink(initfilename); - } - else + LWLockAcquire(RelCacheInitLock, LW_EXCLUSIVE); + + if (unlink(initfilename) < 0) { /* - * We need to interlock this against write_relcache_init_file, to - * guard against possibility that someone renames a new-but- - * already-obsolete init file into place just after we unlink. With - * the interlock, it's certain that write_relcache_init_file will - * notice our SI inval message before renaming into place, or else - * that we will execute second and successfully unlink the file. + * The file might not be there if no backend has been started since + * the last removal. But complain about failures other than ENOENT. + * Fortunately, it's not too late to abort the transaction if we + * can't get rid of the would-be-obsolete init file. */ - LWLockAcquire(RelCacheInitLock, LW_EXCLUSIVE); - unlink(initfilename); - LWLockRelease(RelCacheInitLock); + if (errno != ENOENT) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove cache file \"%s\": %m", + initfilename))); } } +void +RelationCacheInitFilePostInvalidate(void) +{ + LWLockRelease(RelCacheInitLock); +} + /* * Remove the init file for a given database during postmaster startup. * diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h index 3c5b868058..42e7fa3d90 100644 --- a/src/include/utils/relcache.h +++ b/src/include/utils/relcache.h @@ -84,7 +84,8 @@ extern void RelationCacheMarkNewRelfilenode(Relation rel); * Routines to help manage rebuilding of relcache init file */ extern bool RelationIdIsInInitFile(Oid relationId); -extern void RelationCacheInitFileInvalidate(bool beforeSend); +extern void RelationCacheInitFilePreInvalidate(void); +extern void RelationCacheInitFilePostInvalidate(void); extern void RelationCacheInitFileRemove(const char *dbPath); /* should be used only by relcache.c and catcache.c */