From fce1a87e25d20bf4b1a85c6cc535db42b5bdfc73 Mon Sep 17 00:00:00 2001 From: Dilip Kumar Date: Wed, 1 Sep 2021 14:06:29 +0530 Subject: [PATCH v5 1/7] Refactor relmap load and relmap write functions Currently, write_relmap_file and load_relmap_file are tightly coupled with shared_map and local_map. As part of the higher level patch set we need remap read/write interfaces that are not dependent upon shared_map and local_map, and we should be able to pass map memory as an external parameter instead. --- src/backend/utils/cache/relmapper.c | 163 +++++++++++++++++----------- 1 file changed, 99 insertions(+), 64 deletions(-) diff --git a/src/backend/utils/cache/relmapper.c b/src/backend/utils/cache/relmapper.c index a6e38adce3..bb39632080 100644 --- a/src/backend/utils/cache/relmapper.c +++ b/src/backend/utils/cache/relmapper.c @@ -136,6 +136,12 @@ 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 read_relmap_file(char *mapfilename, RelMapFile *map, + bool lock_held); +static void write_relmap_file_internal(char *mapfilename, RelMapFile *newmap, + bool write_wal, bool send_sinval, + bool preserve_files, Oid dbid, Oid tsid, + const char *dbpath); 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, @@ -687,36 +693,19 @@ RestoreRelationMap(char *startAddress) } /* - * load_relmap_file -- load data from the shared or local map file + * read_relmap_file -- read data from given mapfilename file. * * Because the map file is essential for access to core system catalogs, * failure to read it 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(char *mapfilename, RelMapFile *map, bool lock_held) { - 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; - } - - /* Read data ... */ + /* Open the relmap file for reading. */ fd = OpenTransientFile(mapfilename, O_RDONLY | PG_BINARY); if (fd < 0) ereport(FATAL, @@ -779,62 +768,50 @@ load_relmap_file(bool shared, bool lock_held) } /* - * Write out a new shared or local map file with the given contents. - * - * The magic number and CRC are automatically updated in *newmap. On - * success, we copy the data to the appropriate permanent static variable. - * - * If write_wal is true then an appropriate WAL message is emitted. - * (It will be false for bootstrap and WAL replay cases.) - * - * If send_sinval is true then a SI invalidation message is sent. - * (This should be true except in bootstrap case.) - * - * If preserve_files is true then the storage manager is warned not to - * delete the files listed in the map. + * load_relmap_file -- load data from the shared or local map file * - * 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. + * Note that the local case requires DatabasePath to be set up. */ 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) +load_relmap_file(bool shared, bool lock_held) { - int fd; - RelMapFile *realmap; + RelMapFile *map; char mapfilename[MAXPGPATH]; - /* - * Fill in the overhead fields and update CRC. - */ - newmap->magic = RELMAPPER_FILEMAGIC; - if (newmap->num_mappings < 0 || newmap->num_mappings > MAX_MAPPINGS) - elog(ERROR, "attempt to write bogus relation mapping"); - - INIT_CRC32C(newmap->crc); - COMP_CRC32C(newmap->crc, (char *) newmap, offsetof(RelMapFile, crc)); - FIN_CRC32C(newmap->crc); - - /* - * 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; + map = &shared_map; } else { snprintf(mapfilename, sizeof(mapfilename), "%s/%s", - dbpath, RELMAPPER_FILENAME); - realmap = &local_map; + DatabasePath, RELMAPPER_FILENAME); + map = &local_map; } + /* Read data ... */ + read_relmap_file(mapfilename, map, lock_held); +} + +/* + * Helper function for write_relmap_file, Read comments atop write_relmap_file + * for more details. The CRC should be computed by the caller and stored in + * the newmap. + */ +static void +write_relmap_file_internal(char *mapfilename, RelMapFile *newmap, + bool write_wal, bool send_sinval, + bool preserve_files, Oid dbid, Oid tsid, + const char *dbpath) +{ + int fd; + + /* + * Open the target file. We prefer to do this before entering the + * critical section, so that an open() failure need not force PANIC. + */ fd = OpenTransientFile(mapfilename, O_WRONLY | O_CREAT | PG_BINARY); if (fd < 0) ereport(ERROR, @@ -934,6 +911,68 @@ write_relmap_file(bool shared, RelMapFile *newmap, } } + /* Critical section done */ + if (write_wal) + END_CRIT_SECTION(); +} + +/* + * Write out a new shared or local map file with the given contents. + * + * The magic number and CRC are automatically updated in *newmap. On + * success, we copy the data to the appropriate permanent static variable. + * + * If write_wal is true then an appropriate WAL message is emitted. + * (It will be false for bootstrap and WAL replay cases.) + * + * If send_sinval is true then a SI invalidation message is sent. + * (This should be true except in bootstrap case.) + * + * If preserve_files is true then the storage manager is warned not to + * delete the files listed in the map. + * + * 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. + */ +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) +{ + RelMapFile *realmap; + char mapfilename[MAXPGPATH]; + + /* + * Fill in the overhead fields and update CRC. + */ + newmap->magic = RELMAPPER_FILEMAGIC; + if (newmap->num_mappings < 0 || newmap->num_mappings > MAX_MAPPINGS) + elog(ERROR, "attempt to write bogus relation mapping"); + + INIT_CRC32C(newmap->crc); + COMP_CRC32C(newmap->crc, (char *) newmap, offsetof(RelMapFile, crc)); + FIN_CRC32C(newmap->crc); + + 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; + } + + /* Write the map to the relmap file. */ + write_relmap_file_internal(mapfilename, newmap, write_wal, + send_sinval, preserve_files, dbid, tsid, + dbpath); + /* * Success, update permanent copy. During bootstrap, we might be working * on the permanent copy itself, in which case skip the memcpy() to avoid @@ -943,10 +982,6 @@ write_relmap_file(bool shared, RelMapFile *newmap, memcpy(realmap, newmap, sizeof(RelMapFile)); else Assert(!send_sinval); /* must be bootstrapping */ - - /* Critical section done */ - if (write_wal) - END_CRIT_SECTION(); } /* -- 2.23.0