From 90dbeafda334018677c4bea2831e4311950442e6 Mon Sep 17 00:00:00 2001 From: Dilip Kumar Date: Fri, 11 Mar 2022 09:03:09 +0530 Subject: [PATCH v14 1/6] Refactor relmap load and relmap write functions Currently, relmap reading and writing interfaces are tightly coupled with shared_map and local_map of the database it is connected to. But as higher level patch set we need interfaces where we can read relmap into any input memory and while writing also we should be able to pass the map. So as part of this patch, we are doing refactoring of the existing code such that we can expose the read and write interfaces that are independent of the shared_map and the local_map, without changing any logic. Author: Robert Haas --- src/backend/utils/cache/relmapper.c | 147 ++++++++++++++++++------------------ 1 file changed, 74 insertions(+), 73 deletions(-) diff --git a/src/backend/utils/cache/relmapper.c b/src/backend/utils/cache/relmapper.c index 4f6811f..f172f61 100644 --- a/src/backend/utils/cache/relmapper.c +++ b/src/backend/utils/cache/relmapper.c @@ -136,9 +136,11 @@ static void apply_map_update(RelMapFile *map, Oid relationId, Oid fileNode, bool add_okay); static void merge_map_updates(RelMapFile *map, const RelMapFile *updates, bool add_okay); -static void load_relmap_file(bool shared, bool lock_held); -static void write_relmap_file(bool shared, RelMapFile *newmap, - bool write_wal, bool send_sinval, bool preserve_files, +static void read_relmap_file(bool shared, bool lock_held); +static void load_relmap_file(RelMapFile *map, char *dbpath, bool lock_held, + int elevel); +static void write_relmap_file(RelMapFile *newmap, bool write_wal, + bool send_sinval, bool preserve_files, Oid dbid, Oid tsid, const char *dbpath); static void perform_relmap_update(bool shared, const RelMapFile *updates); @@ -405,12 +407,12 @@ RelationMapInvalidate(bool shared) if (shared) { if (shared_map.magic == RELMAPPER_FILEMAGIC) - load_relmap_file(true, false); + read_relmap_file(true, false); } else { if (local_map.magic == RELMAPPER_FILEMAGIC) - load_relmap_file(false, false); + read_relmap_file(false, false); } } @@ -425,9 +427,9 @@ void RelationMapInvalidateAll(void) { if (shared_map.magic == RELMAPPER_FILEMAGIC) - load_relmap_file(true, false); + read_relmap_file(true, false); if (local_map.magic == RELMAPPER_FILEMAGIC) - load_relmap_file(false, false); + read_relmap_file(false, false); } /* @@ -568,9 +570,9 @@ RelationMapFinishBootstrap(void) Assert(pending_local_updates.num_mappings == 0); /* Write the files; no WAL or sinval needed */ - write_relmap_file(true, &shared_map, false, false, false, - InvalidOid, GLOBALTABLESPACE_OID, NULL); - write_relmap_file(false, &local_map, false, false, false, + write_relmap_file(&shared_map, false, false, false, + InvalidOid, GLOBALTABLESPACE_OID, "global"); + write_relmap_file(&local_map, false, false, false, MyDatabaseId, MyDatabaseTableSpace, DatabasePath); } @@ -612,7 +614,7 @@ RelationMapInitializePhase2(void) /* * Load the shared map file, die on error. */ - load_relmap_file(true, false); + read_relmap_file(true, false); } /* @@ -633,7 +635,7 @@ RelationMapInitializePhase3(void) /* * Load the local map file, die on error. */ - load_relmap_file(false, false); + read_relmap_file(false, false); } /* @@ -687,39 +689,48 @@ RestoreRelationMap(char *startAddress) } /* - * load_relmap_file -- load data from the shared or local map file + * read_relmap_file -- load the shared or local map file * - * Because the map file is essential for access to core system catalogs, - * failure to read it is a fatal error. + * Because these files are essential for access to core system catalogs, + * failure to load either of them is a fatal error. * * Note that the local case requires DatabasePath to be set up. */ static void -load_relmap_file(bool shared, bool lock_held) +read_relmap_file(bool shared, bool lock_held) +{ + if (shared) + load_relmap_file(&shared_map, "global", lock_held, FATAL); + else + load_relmap_file(&local_map, DatabasePath, lock_held, FATAL); +} + +/* + * load_relmap_file -- load data from any relation mapper file + * + * dbpath must be the relevant database path, or "global" for shared relations. + * + * RelationMappingLock will be acquired released unless lock_held = true. + * + * Errors will be reported at the indicated elevel, which should be at least + * ERROR. + */ +static void +load_relmap_file(RelMapFile *map, char *dbpath, bool lock_held, int elevel) { - RelMapFile *map; char mapfilename[MAXPGPATH]; pg_crc32c crc; int fd; int r; - if (shared) - { - snprintf(mapfilename, sizeof(mapfilename), "global/%s", - RELMAPPER_FILENAME); - map = &shared_map; - } - else - { - snprintf(mapfilename, sizeof(mapfilename), "%s/%s", - DatabasePath, RELMAPPER_FILENAME); - map = &local_map; - } + Assert(elevel >= ERROR); - /* Read data ... */ + /* Open the target file. */ + snprintf(mapfilename, sizeof(mapfilename), "%s/%s", dbpath, + RELMAPPER_FILENAME); fd = OpenTransientFile(mapfilename, O_RDONLY | PG_BINARY); if (fd < 0) - ereport(FATAL, + ereport(elevel, (errcode_for_file_access(), errmsg("could not open file \"%s\": %m", mapfilename))); @@ -734,16 +745,17 @@ load_relmap_file(bool shared, bool lock_held) if (!lock_held) LWLockAcquire(RelationMappingLock, LW_SHARED); + /* Now read the data. */ pgstat_report_wait_start(WAIT_EVENT_RELATION_MAP_READ); r = read(fd, map, sizeof(RelMapFile)); if (r != sizeof(RelMapFile)) { if (r < 0) - ereport(FATAL, + ereport(elevel, (errcode_for_file_access(), errmsg("could not read file \"%s\": %m", mapfilename))); else - ereport(FATAL, + ereport(elevel, (errcode(ERRCODE_DATA_CORRUPTED), errmsg("could not read file \"%s\": read %d of %zu", mapfilename, r, sizeof(RelMapFile)))); @@ -754,7 +766,7 @@ load_relmap_file(bool shared, bool lock_held) LWLockRelease(RelationMappingLock); if (CloseTransientFile(fd) != 0) - ereport(FATAL, + ereport(elevel, (errcode_for_file_access(), errmsg("could not close file \"%s\": %m", mapfilename))); @@ -763,7 +775,7 @@ load_relmap_file(bool shared, bool lock_held) if (map->magic != RELMAPPER_FILEMAGIC || map->num_mappings < 0 || map->num_mappings > MAX_MAPPINGS) - ereport(FATAL, + ereport(elevel, (errmsg("relation mapping file \"%s\" contains invalid data", mapfilename))); @@ -773,7 +785,7 @@ load_relmap_file(bool shared, bool lock_held) FIN_CRC32C(crc); if (!EQ_CRC32C(crc, map->crc)) - ereport(FATAL, + ereport(elevel, (errmsg("relation mapping file \"%s\" contains incorrect checksum", mapfilename))); } @@ -795,16 +807,16 @@ load_relmap_file(bool shared, bool lock_held) * * Because this may be called during WAL replay when MyDatabaseId, * DatabasePath, etc aren't valid, we require the caller to pass in suitable - * values. The caller is also responsible for being sure no concurrent - * map update could be happening. + * values. Pass dbpath as "global" for the shared map. + * + * The caller is also responsible for being sure no concurrent map update + * could be happening. */ static void -write_relmap_file(bool shared, RelMapFile *newmap, - bool write_wal, bool send_sinval, bool preserve_files, - Oid dbid, Oid tsid, const char *dbpath) +write_relmap_file(RelMapFile *newmap, bool write_wal, bool send_sinval, + bool preserve_files, Oid dbid, Oid tsid, const char *dbpath) { int fd; - RelMapFile *realmap; char mapfilename[MAXPGPATH]; /* @@ -822,19 +834,8 @@ write_relmap_file(bool shared, RelMapFile *newmap, * Open the target file. We prefer to do this before entering the * critical section, so that an open() failure need not force PANIC. */ - if (shared) - { - snprintf(mapfilename, sizeof(mapfilename), "global/%s", - RELMAPPER_FILENAME); - realmap = &shared_map; - } - else - { - snprintf(mapfilename, sizeof(mapfilename), "%s/%s", - dbpath, RELMAPPER_FILENAME); - realmap = &local_map; - } - + snprintf(mapfilename, sizeof(mapfilename), "%s/%s", + dbpath, RELMAPPER_FILENAME); fd = OpenTransientFile(mapfilename, O_WRONLY | O_CREAT | PG_BINARY); if (fd < 0) ereport(ERROR, @@ -934,16 +935,6 @@ write_relmap_file(bool shared, RelMapFile *newmap, } } - /* - * Success, update permanent copy. During bootstrap, we might be working - * on the permanent copy itself, in which case skip the memcpy() to avoid - * invoking nominally-undefined behavior. - */ - if (realmap != newmap) - memcpy(realmap, newmap, sizeof(RelMapFile)); - else - Assert(!send_sinval); /* must be bootstrapping */ - /* Critical section done */ if (write_wal) END_CRIT_SECTION(); @@ -975,7 +966,7 @@ perform_relmap_update(bool shared, const RelMapFile *updates) LWLockAcquire(RelationMappingLock, LW_EXCLUSIVE); /* Be certain we see any other updates just made */ - load_relmap_file(shared, true); + read_relmap_file(shared, true); /* Prepare updated data in a local variable */ if (shared) @@ -990,10 +981,19 @@ perform_relmap_update(bool shared, const RelMapFile *updates) merge_map_updates(&newmap, updates, allowSystemTableMods); /* Write out the updated map and do other necessary tasks */ - write_relmap_file(shared, &newmap, true, true, true, + write_relmap_file(&newmap, true, true, true, (shared ? InvalidOid : MyDatabaseId), (shared ? GLOBALTABLESPACE_OID : MyDatabaseTableSpace), - DatabasePath); + (shared ? "global" : DatabasePath)); + + /* + * We succesfully wrote the updated file, so it's now safe to rely on the + * new values in this process, too. + */ + if (shared) + memcpy(&shared_map, &newmap, sizeof(RelMapFile)); + else + memcpy(&local_map, &newmap, sizeof(RelMapFile)); /* Now we can release the lock */ LWLockRelease(RelationMappingLock); @@ -1021,8 +1021,10 @@ relmap_redo(XLogReaderState *record) xlrec->nbytes); memcpy(&newmap, xlrec->data, sizeof(newmap)); - /* We need to construct the pathname for this database */ - dbpath = GetDatabasePath(xlrec->dbid, xlrec->tsid); + if (xlrec->dbid != InvalidOid) + dbpath = GetDatabasePath(xlrec->dbid, xlrec->tsid); + else + dbpath = pstrdup("global"); /* * Write out the new map and send sinval, but of course don't write a @@ -1030,11 +1032,10 @@ relmap_redo(XLogReaderState *record) * preserve files, either. * * There shouldn't be anyone else updating relmaps during WAL replay, - * but grab the lock to interlock against load_relmap_file(). + * but grab the lock to interlock against read_relmap_file(). */ LWLockAcquire(RelationMappingLock, LW_EXCLUSIVE); - write_relmap_file((xlrec->dbid == InvalidOid), &newmap, - false, true, false, + write_relmap_file(&newmap, false, true, false, xlrec->dbid, xlrec->tsid, dbpath); LWLockRelease(RelationMappingLock); -- 1.8.3.1