From 203e77f9d08d7bef1add7aed63ba042cf88697fe Mon Sep 17 00:00:00 2001 From: Dilip Kumar Date: Wed, 1 Sep 2021 14:06:29 +0530 Subject: [PATCH v9 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. XXX For the code simplicity in write_relmap_file we are updating the permanent memory copy outside the critical section but we have already done the disk changes and it is just a memory change so there is no reason for this to be in the critical section. --- 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 4f6811f..56495f0 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(); } /* -- 1.8.3.1