From a98ff6154a79f23d71ccc8884e3a8f31184e3e01 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Fri, 6 Sep 2019 13:20:42 +0300 Subject: [PATCH] [Issue #115]: some improvements of sanity and comments. Also do not wait for NullOffset LSN when looking for previous record --- src/backup.c | 20 ++++++++++++-------- tests/replica.py | 14 +++++++------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/backup.c b/src/backup.c index 60f3dc1e..481735de 100644 --- a/src/backup.c +++ b/src/backup.c @@ -1528,14 +1528,18 @@ wait_wal_lsn(XLogRecPtr target_lsn, bool is_start_lsn, TimeLineID tli, * If we failed to get target LSN in a reasonable time, try * to get LSN of last valid record prior to the target LSN. But only * in case of a backup from a replica. - + * Note, that with NullOffset target_lsn we do not wait + * for 'timeout / 2' seconds before going for previous record, + * because such LSN cannot be delivered at all. + * * There are two cases for this: * 1. Replica returned readpoint LSN which just do not exists. We want to look * for previous record in the same(!) WAL segment which endpoint points to this LSN. * 2. Replica returened endpoint LSN with 0 offset. We want to look * for previous record which endpoint points greater or equal LSN in previous WAL segment. */ - if (!exclusive_backup && current.from_replica && try_count > timeout / 2) + if (current.from_replica && + (XRecOffIsNull(target_lsn) || try_count > timeout / 2)) { XLogRecPtr res; @@ -1641,7 +1645,7 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn, * Only if backup is from master. * For PG 9.5 create restore point only if pguser is superuser. */ - if (backup != NULL && !current.from_replica && + if (backup != NULL && !backup->from_replica && !(nodeInfo->server_version < 90600 && !nodeInfo->is_superuser)) { @@ -1677,7 +1681,7 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn, * In case of backup from replica >= 9.6 we do not trust minRecPoint * and stop_backup LSN, so we use latest replayed LSN as STOP LSN. */ - if (current.from_replica) + if (backup->from_replica) stop_backup_query = "SELECT" " pg_catalog.txid_snapshot_xmax(pg_catalog.txid_current_snapshot())," " current_timestamp(0)::timestamptz," @@ -1799,8 +1803,8 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn, if (!XRecOffIsValid(stop_backup_lsn_tmp)) { - /* Replica returned STOP LSN with null offset */ - if (XRecOffIsNull(stop_backup_lsn_tmp)) + /* It is ok for replica to return STOP LSN with null offset */ + if (backup->from_replica && XRecOffIsNull(stop_backup_lsn_tmp)) { char *xlog_path, stream_xlog_path[MAXPGPATH]; @@ -1888,7 +1892,7 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn, stop_backup_lsn = lsn_tmp; stop_lsn_exists = true; } - /* Replica returned something very illegal, error out */ + /* PostgreSQL returned something very illegal as STOP_LSN, error out */ else elog(ERROR, "Invalid stop_backup_lsn value %X/%X", (uint32) (stop_backup_lsn_tmp >> 32), (uint32) (stop_backup_lsn_tmp)); @@ -1898,7 +1902,7 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn, if (!exclusive_backup) { Assert(PQnfields(res) >= 4); - pgBackupGetPath(¤t, path, lengthof(path), DATABASE_DIR); + pgBackupGetPath(backup, path, lengthof(path), DATABASE_DIR); /* Write backup_label */ join_path_components(backup_label, path, PG_BACKUP_LABEL_FILE); diff --git a/tests/replica.py b/tests/replica.py index 0c367206..f0368749 100644 --- a/tests/replica.py +++ b/tests/replica.py @@ -586,7 +586,7 @@ class ReplicaTest(ProbackupTest, unittest.TestCase): return_id=False) self.assertIn( - 'WARNING: Invalid stop_backup_lsn value 0/3000000', + 'LOG: Null offset in stop_backup_lsn value 0/3000000', output) self.assertIn( @@ -601,10 +601,6 @@ class ReplicaTest(ProbackupTest, unittest.TestCase): 'LOG: Looking for LSN 0/3000000 in segment: 000000010000000000000002', output) - self.assertIn( - 'INFO: Wait for LSN 0/3000000 in streamed WAL segment', - output) - self.assertIn( 'LOG: Record 0/2000160 has endpoint 0/3000000 which is ' 'equal or greater than requested LSN 0/3000000', @@ -697,7 +693,7 @@ class ReplicaTest(ProbackupTest, unittest.TestCase): log_content = f.read() self.assertIn( - 'WARNING: Invalid stop_backup_lsn value 0/3000000', + 'LOG: Null offset in stop_backup_lsn value 0/3000000', log_content) self.assertIn( @@ -766,7 +762,7 @@ class ReplicaTest(ProbackupTest, unittest.TestCase): return_id=False) self.assertIn( - 'WARNING: Invalid stop_backup_lsn value 0/3000000', + 'LOG: Null offset in stop_backup_lsn value 0/3000000', output) self.assertIn( @@ -790,6 +786,8 @@ class ReplicaTest(ProbackupTest, unittest.TestCase): 'LOG: Found prior LSN: 0/2000160', output) + print(output) + # Clean after yourself self.del_test_dir(module_name, fname) @@ -949,6 +947,8 @@ class ReplicaTest(ProbackupTest, unittest.TestCase): 'LOG: Found prior LSN:', output) + print(output) + replica.cleanup() self.restore_node(backup_dir, 'replica', replica)