postgres.git / summary / log / commit / refs

commit    64eb426afc344600d7d4b9944ed7c3be7ed1812d
Author:   Michael Paquier <michael@paquier.xyz>
Date:     Wed Sep 04 04:23:41 2019 +0000
First released in: 13.0, 10.11, 11.6, 12.0

    Delay fsyncs of pg_basebackup until the end of backup
    
    Since the addition of fsync requests in bc34223 to make base backup data
    consistent on disk once pg_basebackup finishes, each tablespace tar file
    is individually flushed once completed, with an additional flush of the
    parent directory when the base backup finishes.  While holding a
    connection to the server, a fsync request taking a long time may cause a
    failure of the base backup, which is annoying for any integration.  A
    recent example of breakage can involve tcp_user_timeout, but
    wal_sender_timeout can cause similar problems.
    
    While reviewing the code, there was a second issue causing too many
    fsync requests to be done for the same WAL data.  As recursive fsyncs
    are done at the end of the backup for both the plain and tar formats
    from the base target directory where everything is written, it is fine
    to disable fsyncs when fetching or streaming WAL.
    
    Reported-by: Ryohei Takahashi
    Author: Michael Paquier
    Reviewed-by: Ryohei Takahashi
    Discussion: https://postgr.es/m/OSBPR01MB4550DAE2F8C9502894A45AAB82BE0@OSBPR01MB4550.jpnprd01.prod.outlook.com
    Backpatch-through: 10


 src/bin/pg_basebackup/pg_basebackup.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 068404887d0..5fdc6cfd19b 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -486,15 +486,18 @@ LogStreamerMain(logstreamer_param *param)
 #endif
 	stream.standby_message_timeout = standby_message_timeout;
 	stream.synchronous = false;
-	stream.do_sync = do_sync;
+	/* fsync happens at the end of pg_basebackup for all data */
+	stream.do_sync = false;
 	stream.mark_done = true;
 	stream.partial_suffix = NULL;
 	stream.replication_slot = replication_slot;
 
 	if (format == 'p')
-		stream.walmethod = CreateWalDirectoryMethod(param->xlog, 0, do_sync);
+		stream.walmethod = CreateWalDirectoryMethod(param->xlog, 0,
+													stream.do_sync);
 	else
-		stream.walmethod = CreateWalTarMethod(param->xlog, compresslevel, do_sync);
+		stream.walmethod = CreateWalTarMethod(param->xlog, compresslevel,
+											  stream.do_sync);
 
 	if (!ReceiveXlogStream(param->bgconn, &stream))
 
@@ -1346,9 +1349,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.
+	 */
 }
 
 
@@ -2134,9 +2138,9 @@ 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
-	 * the base directory is synced, taking into account all the tablespaces.
+	 * 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.
 	 */
 	if (do_sync)
@@ -2146,7 +2150,7 @@ BaseBackup(void)
 		if (format == 't')
 		{
 			if (strcmp(basedir, "-") != 0)
-				(void) fsync_fname(basedir, true);
+				(void) fsync_dir_recurse(basedir);
 		}
 		else
 		{


[parent: 73687190d1e7]