On Mon, Sep 02, 2019 at 04:42:55AM +0000, r.takahash...@fujitsu.com wrote: > I think fsync() for each tablespace is not necessary. > Like pg_basebackup -F p, I think fsync() is necessary only once at the end.
Yes, I agree that we overlooked that part when introducing tcp_user_timeout. It is possible to sync all the contents of pg_basebackup's tar format once at the end with fsync_dir_recurse(). Looking at the original discussion that brought bc34223b, the proposed patches did what we have now on HEAD but we did not really exchange about doing a fsync() just at the end with all the result base directory contents: https://www.postgresql.org/message-id/cab7npqql0fcp0edcvd6+3+je24xeapu14vkz_pbpna0stpw...@mail.gmail.com Attached is a patch to do that, which should go down to v12 where tcp_user_timeout has been introduced. Takahashi-san, what do you think? -- Michael
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 9207109ba3..f9f27fbf76 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1346,9 +1346,10 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum)
if (copybuf != NULL)
PQfreemem(copybuf);
- /* sync the resulting tar file, errors are not considered fatal */
- if (do_sync && strcmp(basedir, "-") != 0)
- (void) fsync_fname(filename, false);
+ /*
+ * Do not sync the resulting tar file yet, all files are synced once
+ * at the end.
+ */
}
@@ -2138,8 +2139,8 @@ BaseBackup(void)
/*
* Make data persistent on disk once backup is completed. For tar format
- * once syncing the parent directory is fine, each tar file created per
- * tablespace has been already synced. In plain format, all the data of
+ * sync the parent directory and all its contents as each tar file was
+ * not synced after being completed. In plain format, all the data of
* the base directory is synced, taking into account all the tablespaces.
* Errors are not considered fatal.
*/
@@ -2150,7 +2151,7 @@ BaseBackup(void)
if (format == 't')
{
if (strcmp(basedir, "-") != 0)
- (void) fsync_fname(basedir, true);
+ (void) fsync_dir_recurse(basedir);
}
else
{
signature.asc
Description: PGP signature
