Bonjour Michaƫl-san,
Yes, that would be nice, for now I have focused. For pg_resetwal yes we could do it easily. Would you like to send a patch?
Here is a proposal for "pg_resetwal".The implementation basically removes a lot of copy paste and calls the new update_controlfile function instead. I like removing useless code:-)
The reserwal implementation was doing a rm/create cycle, which was leaving a small window for losing the controlfile. Not neat.
I do not see the value of *not* fsyncing the control file when writing it, as it is by definition very precious, so I added a fsync. The server side branch uses the backend available "pg_fsync", which complies with server settings there and can do nothing if fsync is disabled.
Maybe the two changes could be committed separately. -- Fabien.
diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c index 2af8713216..dd085e16ab 100644 --- a/src/bin/pg_resetwal/pg_resetwal.c +++ b/src/bin/pg_resetwal/pg_resetwal.c @@ -918,18 +918,6 @@ PrintNewControlValues(void) static void RewriteControlFile(void) { - int fd; - char buffer[PG_CONTROL_FILE_SIZE]; /* need not be aligned */ - - /* - * For good luck, apply the same static assertions as in backend's - * WriteControlFile(). - */ - StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_MAX_SAFE_SIZE, - "pg_control is too large for atomic disk writes"); - StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_FILE_SIZE, - "sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE"); - /* * Adjust fields as needed to force an empty XLOG starting at * newXlogSegNo. @@ -961,53 +949,7 @@ RewriteControlFile(void) ControlFile.max_prepared_xacts = 0; ControlFile.max_locks_per_xact = 64; - /* Contents are protected with a CRC */ - INIT_CRC32C(ControlFile.crc); - COMP_CRC32C(ControlFile.crc, - (char *) &ControlFile, - offsetof(ControlFileData, crc)); - FIN_CRC32C(ControlFile.crc); - - /* - * We write out PG_CONTROL_FILE_SIZE bytes into pg_control, zero-padding - * the excess over sizeof(ControlFileData). This reduces the odds of - * premature-EOF errors when reading pg_control. We'll still fail when we - * check the contents of the file, but hopefully with a more specific - * error than "couldn't read pg_control". - */ - memset(buffer, 0, PG_CONTROL_FILE_SIZE); - memcpy(buffer, &ControlFile, sizeof(ControlFileData)); - - unlink(XLOG_CONTROL_FILE); - - fd = open(XLOG_CONTROL_FILE, - O_RDWR | O_CREAT | O_EXCL | PG_BINARY, - pg_file_create_mode); - if (fd < 0) - { - fprintf(stderr, _("%s: could not create pg_control file: %s\n"), - progname, strerror(errno)); - exit(1); - } - - errno = 0; - if (write(fd, buffer, PG_CONTROL_FILE_SIZE) != PG_CONTROL_FILE_SIZE) - { - /* if write didn't set errno, assume problem is no disk space */ - if (errno == 0) - errno = ENOSPC; - fprintf(stderr, _("%s: could not write pg_control file: %s\n"), - progname, strerror(errno)); - exit(1); - } - - if (fsync(fd) != 0) - { - fprintf(stderr, _("%s: fsync error: %s\n"), progname, strerror(errno)); - exit(1); - } - - close(fd); + update_controlfile(XLOG_CONTROL_FILE, progname, &ControlFile); } diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c index 71e67a2eda..78ce8b020f 100644 --- a/src/common/controldata_utils.c +++ b/src/common/controldata_utils.c @@ -144,9 +144,9 @@ get_controlfile(const char *DataDir, const char *progname, bool *crc_ok_p) * update_controlfile() * * Update controlfile values with the contents given by caller. The - * contents to write are included in "ControlFile". Note that it is up - * to the caller to fsync the updated file, and to properly lock - * ControlFileLock when calling this routine in the backend. + * contents to write are included in "ControlFile". Not that it is + * to the caller to properly lock ControlFileLock when calling this + * routine in the backend. */ void update_controlfile(const char *DataDir, const char *progname, @@ -216,6 +216,21 @@ update_controlfile(const char *DataDir, const char *progname, #endif } +#ifndef FRONTEND + if (pg_fsync(fd) != 0) + ereport(PANIC, + (errcode_for_file_access(), + errmsg("could not fsync file \"%s\": %m", + ControlFilePath))); +#else + if (fsync(fd) != 0) + { + fprintf(stderr, _("%s: could not fsync file \"%s\": %s\n"), + progname, ControlFilePath, strerror(errno)); + exit(EXIT_FAILURE); + } +#endif + #ifndef FRONTEND if (CloseTransientFile(fd)) ereport(PANIC,