public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c 46+ messages / 4 participants [nested] [flat]
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v3 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c54..55439db20b 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c..d8466385cf 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 0ec52cb032..5a7ab764db 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed67..67f90c2a38 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.18.4 ----Next_Part(Fri_Sep_18_16_41_50_2020_369)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v2_5-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------D93EDEBFB124D563B723F4BD Content-Type: text/x-patch; charset=UTF-8; name="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-08-19 12:34 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e445..c9b9e480c0f 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------BF34D0120055DC3839060F92 Content-Type: text/x-patch; charset=UTF-8; name="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* [PATCH v3 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c @ 2020-09-24 09:23 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Heikki Linnakangas @ 2020-09-24 09:23 UTC (permalink / raw) For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. Reviewed-by: Michael Paquier Discussion: https://www.postgresql.org/message-id/0c5b3783-af52-3ee5-f8fa-6e794061f70d%40iki.fi --- src/bin/pg_rewind/file_ops.c | 19 +++++++++++++++++++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +--------------------- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index b3bf091c546..55439db20ba 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -19,6 +19,7 @@ #include <unistd.h> #include "common/file_perm.h" +#include "common/file_utils.h" #include "file_ops.h" #include "filemap.h" #include "pg_rewind.h" @@ -266,6 +267,24 @@ remove_target_symlink(const char *path) dstpath); } +/* + * Sync target data directory to ensure that modifications are safely on disk. + * + * We do this once, for the whole data directory, for performance reasons. At + * the end of pg_rewind's run, the kernel is likely to already have flushed + * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass + * approach (only initiating writeback in the first pass), which often reduces + * the overall amount of IO noticeably. + */ +void +sync_target_dir(void) +{ + if (!do_sync || dry_run) + return; + + fsync_pgdata(datadir_target, PG_VERSION_NUM); +} + /* * Read a file into memory. The file to be read is <datadir>/<path>. diff --git a/src/bin/pg_rewind/file_ops.h b/src/bin/pg_rewind/file_ops.h index 025f24141c9..d8466385cf5 100644 --- a/src/bin/pg_rewind/file_ops.h +++ b/src/bin/pg_rewind/file_ops.h @@ -19,6 +19,7 @@ extern void remove_target_file(const char *path, bool missing_ok); extern void truncate_target_file(const char *path, off_t newsize); extern void create_target(file_entry_t *t); extern void remove_target(file_entry_t *t); +extern void sync_target_dir(void); extern char *slurpFile(const char *datadir, const char *path, size_t *filesize); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 0ec52cb0327..5a7ab764db4 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -20,7 +20,6 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" #include "common/file_perm.h" -#include "common/file_utils.h" #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/recovery_gen.h" @@ -38,7 +37,6 @@ static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, static void digestControlFile(ControlFileData *ControlFile, char *source, size_t size); -static void syncTargetDirectory(void); static void getRestoreCommand(const char *argv0); static void sanityChecks(void); static void findCommonAncestorTimeline(XLogRecPtr *recptr, int *tliIndex); @@ -455,7 +453,7 @@ main(int argc, char **argv) if (showprogress) pg_log_info("syncing target data directory"); - syncTargetDirectory(); + sync_target_dir(); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, @@ -803,24 +801,6 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) checkControlFile(ControlFile); } -/* - * Sync target data directory to ensure that modifications are safely on disk. - * - * We do this once, for the whole data directory, for performance reasons. At - * the end of pg_rewind's run, the kernel is likely to already have flushed - * most dirty buffers to disk. Additionally fsync_pgdata uses a two-pass - * approach (only initiating writeback in the first pass), which often reduces - * the overall amount of IO noticeably. - */ -static void -syncTargetDirectory(void) -{ - if (!do_sync || dry_run) - return; - - fsync_pgdata(datadir_target, PG_VERSION_NUM); -} - /* * Get value of GUC parameter restore_command from the target cluster. * diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 8a9319ed675..67f90c2a38c 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -24,6 +24,7 @@ extern char *datadir_source; extern char *connstr_source; extern bool showprogress; extern bool dry_run; +extern bool do_sync; extern int WalSegSz; /* Target history */ -- 2.20.1 --------------E5E97ECB089CAF17EDB11AD4 Content-Type: text/x-patch; charset=UTF-8; name="v3-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v3-0002-Refactor-pg_rewind-for-more-clear-decision-making.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: the s_lock_stuck on perform_spin_delay @ 2024-01-04 23:06 Andres Freund <[email protected]> 0 siblings, 2 replies; 46+ messages in thread From: Andres Freund @ 2024-01-04 23:06 UTC (permalink / raw) To: Andy Fan <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Thomas Munro <[email protected]> Hi, On 2024-01-04 14:59:06 +0800, Andy Fan wrote: > My question is if someone doesn't obey the rule by mistake (everyone > can make mistake), shall we PANIC on a production environment? IMO I > think it can be a WARNING on a production environment and be a stuck > when 'ifdef USE_ASSERT_CHECKING'. > > [...] > > I notice this issue actually because of the patch "Cache relation > sizes?" from Thomas Munro [1], where the latest patch[2] still have the > following code. > + sr = smgr_alloc_sr(); <-- HERE a spin lock is hold > + > + /* Upgrade to exclusive lock so we can create a mapping. */ > + LWLockAcquire(mapping_lock, LW_EXCLUSIVE); <-- HERE a complex > operation is needed. it may take a long time. > > Our internal testing system found more misuses on our own PG version. > I think a experienced engineer like Thomas can make this mistake and the > patch was reviewed by 3 peoples, the bug is still there. It is not easy > to say just don't do it. I don't follow this argument - just ignoring the problem, which emitting a WARNING basically is, doesn't reduce the impact of the bug, it *increases* the impact, because now the system will not recover from the bug without explicit operator intervention. During that time the system might be essentially unresponsive, because all backends end up contending for some spinlock, which makes investigating such issues very hard. I think we should add infrastructure to detect bugs like this during development, but not PANICing when this happens in production seems completely non-viable. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: the s_lock_stuck on perform_spin_delay @ 2024-01-05 02:20 Andy Fan <[email protected]> parent: Andres Freund <[email protected]> 1 sibling, 1 reply; 46+ messages in thread From: Andy Fan @ 2024-01-05 02:20 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: Thomas Munro <[email protected]>; [email protected] Hi, Andres Freund <[email protected]> writes: > > On 2024-01-04 14:59:06 +0800, Andy Fan wrote: >> My question is if someone doesn't obey the rule by mistake (everyone >> can make mistake), shall we PANIC on a production environment? IMO I >> think it can be a WARNING on a production environment and be a stuck >> when 'ifdef USE_ASSERT_CHECKING'. >> >> [...] >> >> I notice this issue actually because of the patch "Cache relation >> sizes?" from Thomas Munro [1], where the latest patch[2] still have the >> following code. >> + sr = smgr_alloc_sr(); <-- HERE a spin lock is hold >> + >> + /* Upgrade to exclusive lock so we can create a mapping. */ >> + LWLockAcquire(mapping_lock, LW_EXCLUSIVE); <-- HERE a complex >> operation is needed. it may take a long time. >> >> Our internal testing system found more misuses on our own PG version. > >> I think a experienced engineer like Thomas can make this mistake and the >> patch was reviewed by 3 peoples, the bug is still there. It is not easy >> to say just don't do it. > > I don't follow this argument - just ignoring the problem, I agree with you but I'm feeling you ignored my post at [1], where I said for the known issue, it should be fixed at the first chance. > which emitting a > WARNING basically is, doesn't reduce the impact of the bug, it *increases* the > impact, because now the system will not recover from the bug without explicit > operator intervention. During that time the system might be essentially > unresponsive, because all backends end up contending for some spinlock, which > makes investigating such issues very hard. Acutally they are doing pg_usleep at the most time. Besides what Robert said, one more reason to question PANIC is that: PAINC can't always make the system recovery faster because: a). In the most system, PANIC makes a core dump which take times and spaces. b). After the reboot, all the caches like relcache, plancache, fdcache need to be rebuit. c). Customer needs to handle failure better or else they will be hurt *more often*. All of such sense cause slowness as well. > > I think we should add infrastructure to detect bugs like this during > development, The commit 2 in [1] does something like this. for the details, I missed the check for memory allocation case as you suggested at [2], but checked heavyweight lock as well. others should be same IIUC. > but not PANICing when this happens in production seems completely > non-viable. > Not sure what does *this* exactly means. If it means the bug in Thomas's patch, I absoluately agree with you(since it is a known bug and it should be fixed). If it means the general *unknown* case, it's something we talked above. I'm also agree that some LLVM static checker should be pretty good ideally, it just requires more knowledge base and future maintain effort. I am willing to have a try shortly. [1] https://www.postgresql.org/message-id/871qaxp3ly.fsf%40163.com [2] https://www.postgresql.org/message-id/20240104225403.dgmbbfffmm3srpgq%40awork3.anarazel.de -- Best Regards Andy Fan ^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: the s_lock_stuck on perform_spin_delay @ 2024-01-05 13:51 Robert Haas <[email protected]> parent: Andres Freund <[email protected]> 1 sibling, 1 reply; 46+ messages in thread From: Robert Haas @ 2024-01-05 13:51 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: Andy Fan <[email protected]>; PostgreSQL Hackers <[email protected]>; Thomas Munro <[email protected]> On Thu, Jan 4, 2024 at 6:06 PM Andres Freund <[email protected]> wrote: > I think we should add infrastructure to detect bugs like this during > development, but not PANICing when this happens in production seems completely > non-viable. I mean +1 for the infrastructure, but "completely non-viable"? Why? I've only very rarely seen this PANIC occur, and in the few cases where I've seen it, it was entirely unclear that the problem was due to a bug where somebody failed to release a spinlock. It seemed more likely that the machine was just not really functioning, and the PANIC was a symptom of processes not getting scheduled rather than a PG bug. And every time I tell a user that they might need to use a debugger to, say, set VacuumCostActive = false, or to get a backtrace, or any other reason, I have to tell them to make sure to detach the debugger in under 60 seconds, because in the unlikely event that they attach while the process is holding a spinlock, failure to detach in under 60 seconds will take their production system down for no reason. Now, if you're about to say that people shouldn't need to use a debugger on their production instance, I entirely agree ... but in the world I inhabit, that's often the only way to solve a customer problem, and it probably will continue to be until we have much better ways of getting backtraces without using a debugger than is currently the case. Have you seen real cases where this PANIC prevents a hangup? If yes, that PANIC traced back to a bug in PostgreSQL? And why didn't the user just keep hitting the same bug over and PANICing in an endless loop? I feel like this is one of those things that has just been this way forever and we don't question it because it's become an article of faith that it's something we have to have. But I have a very hard time explaining why it's even a net positive, let alone the unquestionable good that you seem to think. -- Robert Haas EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: the s_lock_stuck on perform_spin_delay @ 2024-01-05 19:11 Andres Freund <[email protected]> parent: Robert Haas <[email protected]> 0 siblings, 1 reply; 46+ messages in thread From: Andres Freund @ 2024-01-05 19:11 UTC (permalink / raw) To: Robert Haas <[email protected]>; +Cc: Andy Fan <[email protected]>; PostgreSQL Hackers <[email protected]>; Thomas Munro <[email protected]> Hi, On 2024-01-05 08:51:53 -0500, Robert Haas wrote: > On Thu, Jan 4, 2024 at 6:06 PM Andres Freund <[email protected]> wrote: > > I think we should add infrastructure to detect bugs like this during > > development, but not PANICing when this happens in production seems completely > > non-viable. > > I mean +1 for the infrastructure, but "completely non-viable"? Why? > > I've only very rarely seen this PANIC occur, and in the few cases > where I've seen it, it was entirely unclear that the problem was due > to a bug where somebody failed to release a spinlock. I see it fairly regularly. Including finding several related bugs that lead to stuck systems last year (signal handlers are a menace). > It seemed more likely that the machine was just not really functioning, and > the PANIC was a symptom of processes not getting scheduled rather than a PG > bug. If processes don't get scheduled for that long a crash-restart doesn't seem that bad anymore :) > And every time I tell a user that they might need to use a debugger to, say, > set VacuumCostActive = false, or to get a backtrace, or any other reason, I > have to tell them to make sure to detach the debugger in under 60 seconds, > because in the unlikely event that they attach while the process is holding > a spinlock, failure to detach in under 60 seconds will take their production > system down for no reason. Hm - isn't the stuck lock timeout more like 900s (MAX_DELAY_USEC * NUM_DELAYS = 1000s, but we start at a lower delay)? One issue with the code as-is is that interrupted sleeps count towards to the timeout, despite possibly sleeping much shorter. We should probably fix that, and also report the time the lock was stuck for in s_lock_stuck(). > Now, if you're about to say that people shouldn't need to use a debugger on > their production instance, I entirely agree ... but in the world I inhabit, > that's often the only way to solve a customer problem, and it probably will > continue to be until we have much better ways of getting backtraces without > using a debugger than is currently the case. > > Have you seen real cases where this PANIC prevents a hangup? If yes, > that PANIC traced back to a bug in PostgreSQL? And why didn't the user > just keep hitting the same bug over and PANICing in an endless loop? Many, as hinted above. Some bugs in postgres, more bugs in extensions. IME these bugs aren't hit commonly, so a crash-restart at least allows to hobble along. The big issue with not crash-restarting is that often the system ends up inaccessible, which makes it very hard to investigate the issue. > I feel like this is one of those things that has just been this way > forever and we don't question it because it's become an article of > faith that it's something we have to have. But I have a very hard time > explaining why it's even a net positive, let alone the unquestionable > good that you seem to think. I don't think it's an unquestionable good, I just think the alternative of just endlessly spinning is way worse. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: the s_lock_stuck on perform_spin_delay @ 2024-01-05 19:19 Robert Haas <[email protected]> parent: Andres Freund <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Robert Haas @ 2024-01-05 19:19 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: Andy Fan <[email protected]>; PostgreSQL Hackers <[email protected]>; Thomas Munro <[email protected]> On Fri, Jan 5, 2024 at 2:11 PM Andres Freund <[email protected]> wrote: > I see it fairly regularly. Including finding several related bugs that lead to > stuck systems last year (signal handlers are a menace). In that case, I think this proposal is dead. I can't personally testify to this code being a force for good, but it sounds like you can. So be it! -- Robert Haas EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: the s_lock_stuck on perform_spin_delay @ 2024-01-05 19:27 Andres Freund <[email protected]> parent: Andy Fan <[email protected]> 0 siblings, 0 replies; 46+ messages in thread From: Andres Freund @ 2024-01-05 19:27 UTC (permalink / raw) To: Andy Fan <[email protected]>; +Cc: Thomas Munro <[email protected]>; [email protected] Hi, On 2024-01-05 10:20:39 +0800, Andy Fan wrote: > Andres Freund <[email protected]> writes: > > On 2024-01-04 14:59:06 +0800, Andy Fan wrote: > >> My question is if someone doesn't obey the rule by mistake (everyone > >> can make mistake), shall we PANIC on a production environment? IMO I > >> think it can be a WARNING on a production environment and be a stuck > >> when 'ifdef USE_ASSERT_CHECKING'. > >> > >> [...] > >> > >> I notice this issue actually because of the patch "Cache relation > >> sizes?" from Thomas Munro [1], where the latest patch[2] still have the > >> following code. > >> + sr = smgr_alloc_sr(); <-- HERE a spin lock is hold > >> + > >> + /* Upgrade to exclusive lock so we can create a mapping. */ > >> + LWLockAcquire(mapping_lock, LW_EXCLUSIVE); <-- HERE a complex > >> operation is needed. it may take a long time. > >> > >> Our internal testing system found more misuses on our own PG version. > > > >> I think a experienced engineer like Thomas can make this mistake and the > >> patch was reviewed by 3 peoples, the bug is still there. It is not easy > >> to say just don't do it. > > > > I don't follow this argument - just ignoring the problem, > > I agree with you but I'm feeling you ignored my post at [1], where I > said for the known issue, it should be fixed at the first chance. With "ignoring the problem" I was referencing emitting a WARNING instead of crash-restart. IME stuck spinlocks are caused by issues like not releasing a spinlock, possibly due to returning early due to an error or such, having lock-nesting issues leading to deadlocks, acquiring spinlocks or lwlocks in signal handlers, blocking in signal handlers while holding a spinlock outside of the signal handers and many variations of those. The common theme of these causes is that they don't resolve after some time. The only way out of the situation is to crash-restart, either "automatically" or by operator intervention. > > which emitting a > > WARNING basically is, doesn't reduce the impact of the bug, it *increases* the > > impact, because now the system will not recover from the bug without explicit > > operator intervention. During that time the system might be essentially > > unresponsive, because all backends end up contending for some spinlock, which > > makes investigating such issues very hard. > > Acutally they are doing pg_usleep at the most time. Sure - but that changes nothing about the problem. The concern isn't CPU usage, the concern is that there's often no possible forward progress. To take a recent-ish production issue I looked at, a buggy signal handler lead to a backend sleeping while holding a spinlock. Soon after the entire system got stuck, because they also acquired the spinlock. The person initially investigating the issue at first contacted me because they couldn't even log into the system, because connection establishment also acquired the spinlock (I'm not sure anymore which spinlock it was, possibly xlog.c's info_lck?). > > but not PANICing when this happens in production seems completely > > non-viable. > > > > Not sure what does *this* exactly means. If it means the bug in Thomas's > patch, I absoluately agree with you(since it is a known bug and it > should be fixed). If it means the general *unknown* case, it's something > we talked above. I mean that I think that not PANICing anymore would be a seriously bad idea and cause way more problems than the PANIC. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 46+ messages in thread
end of thread, other threads:[~2024-01-05 19:27 UTC | newest] Thread overview: 46+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v3 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH v2 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-08-19 12:34 [PATCH 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2020-09-24 09:23 [PATCH v3 1/5] pg_rewind: Move syncTargetDirectory() to file_ops.c Heikki Linnakangas <[email protected]> 2024-01-04 23:06 Re: the s_lock_stuck on perform_spin_delay Andres Freund <[email protected]> 2024-01-05 02:20 ` Re: the s_lock_stuck on perform_spin_delay Andy Fan <[email protected]> 2024-01-05 19:27 ` Re: the s_lock_stuck on perform_spin_delay Andres Freund <[email protected]> 2024-01-05 13:51 ` Re: the s_lock_stuck on perform_spin_delay Robert Haas <[email protected]> 2024-01-05 19:11 ` Re: the s_lock_stuck on perform_spin_delay Andres Freund <[email protected]> 2024-01-05 19:19 ` Re: the s_lock_stuck on perform_spin_delay Robert Haas <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox