Merge branch 'master' into issue_174

Author: gsmolk

doc/pgprobackup.xml

@@ -1634,13 +1634,14 @@ pg_probackup restore -B <replaceable>backup_dir</replaceable> --instance <replac
     <para>
      If you restore <link linkend="pbk-archive-mode">ARCHIVE</link> backups,
      perform <link linkend="pbk-performing-point-in-time-pitr-recovery">PITR</link>,
-     or specify the <literal>--restore-as-replica</literal> option with the
+     or specify the <literal>--restore-as-replica</literal> flag with the
      <literal>restore</literal> command to set up a standby server,
      <application>pg_probackup</application> creates a recovery configuration
      file once all data files are copied into the target directory. This file
      includes the minimal settings required for recovery, except for the password in the
      <ulink url="https://postgrespro.com/docs/postgresql/current/runtime-config-replication.html#GUC-PRIMARY-CONNINFO">primary_conninfo</ulink>
-     parameter; you have to add the password manually, if required.
+     parameter; you have to add the password manually or use
+     the <literal>--primary-conninfo</literal> option, if required.
      For <productname>PostgreSQL</productname> 11 or lower,
      recovery settings are written into the <filename>recovery.conf</filename>
      file. Starting from <productname>PostgreSQL</productname> 12,
@@ -2242,6 +2243,13 @@ BACKUP INSTANCE 'node'
                 <literal>MERGING</literal> — the backup is being merged.
               </para>
             </listitem>
+            <listitem>
+              <para>
+                <literal>MERGED</literal> — the backup data files were
+                successfully merged, but its metadata is in the process
+                of been updated. Only full backup can have this status.
+              </para>
+            </listitem>
             <listitem>
               <para>
                 <literal>DELETING</literal> — the backup files are being deleted.
@@ -3146,7 +3154,9 @@ pg_probackup merge -B <replaceable>backup_dir</replaceable> --instance <replacea
     <para>
       This command merges the specified incremental backup to its
       parent full backup, together with all incremental backups
-      between them. Once the merge is complete, the incremental
+      between them. If the specified backup ID belong to the full backup,
+      then it will be merged with the closest incremental backup.
+      Once the merge is complete, the incremental
       backups are removed as redundant. Thus, the merge operation is
       virtually equivalent to retaking a full backup and removing all
       the outdated backups, but it allows to save much time,
@@ -3165,7 +3175,8 @@ pg_probackup show -B <replaceable>backup_dir</replaceable> --instance <replaceab
 </programlisting>
     <para>
       If the merge is still in progress, the backup status is
-      displayed as <literal>MERGING</literal>. The merge is idempotent, so you can
+      displayed as <literal>MERGING</literal> or, at the final stage,
+      <literal>MERGED</literal>. The merge is idempotent, so you can
       restart the merge if it was interrupted.
     </para>
   </refsect2>
@@ -3404,6 +3415,7 @@ pg_probackup backup -B <replaceable>backup_dir</replaceable> -b <replaceable>bac
 [--no-validate] [--skip-block-validation]
 [-w --no-password] [-W --password]
 [--archive-timeout=<replaceable>timeout</replaceable>] [--external-dirs=<replaceable>external_directory_path</replaceable>]
+[--no-sync]
 [<replaceable>connection_options</replaceable>] [<replaceable>compression_options</replaceable>] [<replaceable>remote_options</replaceable>]
 [<replaceable>retention_options</replaceable>] [<replaceable>pinning_options</replaceable>] [<replaceable>logging_options</replaceable>]
 </programlisting>
@@ -3556,6 +3568,18 @@ pg_probackup backup -B <replaceable>backup_dir</replaceable> -b <replaceable>bac
       </para>
       </listitem>
       </varlistentry>
+
+      <varlistentry>
+<term><option>--no-sync</option></term>
+      <listitem>
+      <para>
+        Do not sync backed up files to disk. You can use this flag to speed
+        up backup process. Using this flag can result in data
+        corruption in case of operating system or hardware crash.
+        Corruption can be detected by backup validation.
+      </para>
+      </listitem>
+      </varlistentry>
       </variablelist>
       </para>
 
@@ -3582,8 +3606,11 @@ pg_probackup restore -B <replaceable>backup_dir</replaceable> --instance <replac
 [--help] [-D <replaceable>data_dir</replaceable>] [-i <replaceable>backup_id</replaceable>]
 [-j <replaceable>num_threads</replaceable>] [--progress]
 [-T <replaceable>OLDDIR</replaceable>=<replaceable>NEWDIR</replaceable>] [--external-mapping=<replaceable>OLDDIR</replaceable>=<replaceable>NEWDIR</replaceable>] [--skip-external-dirs]
-[-R | --restore-as-replica] [--no-validate] [--skip-block-validation] [--force]
+[-R | --restore-as-replica] [--no-validate] [--skip-block-validation]
+[--force] [--no-sync]
+[--restore-command=<replaceable>cmdline</replaceable>]
 [--restore-command=<replaceable>cmdline</replaceable>]
+[--primary-conninfo=<replaceable>primary_conninfo</replaceable>]
 [<replaceable>recovery_target_options</replaceable>] [<replaceable>logging_options</replaceable>] [<replaceable>remote_options</replaceable>]
 [<replaceable>partial_restore_options</replaceable>] [<replaceable>remote_wal_archive_options</replaceable>]
 </programlisting>
@@ -3621,6 +3648,21 @@ pg_probackup restore -B <replaceable>backup_dir</replaceable> --instance <replac
       </listitem>
       </varlistentry>
 
+      <varlistentry>
+<term><option>--primary-conninfo=<replaceable>primary_conninfo</replaceable></option></term>
+      <listitem>
+      <para>
+        Sets the
+        <ulink url="https://postgrespro.com/docs/postgresql/current/runtime-config-replication.html#GUC-PRIMARY-CONNINFO">primary_conninfo</ulink>
+        parameter to the specified value.
+        This option will be ignored unless the <option>-R</option> flag if specified.
+      </para>
+      <para>
+        Example: <literal>--primary-conninfo='host=192.168.1.50 port=5432 user=foo password=foopass'</literal>
+      </para>
+      </listitem>
+      </varlistentry>
+
       <varlistentry>
 <term><option>-T <replaceable>OLDDIR</replaceable>=<replaceable>NEWDIR</replaceable></option></term>
 <term><option>--tablespace-mapping=<replaceable>OLDDIR</replaceable>=<replaceable>NEWDIR</replaceable></option></term>
@@ -3705,6 +3747,17 @@ pg_probackup restore -B <replaceable>backup_dir</replaceable> --instance <replac
       </para>
       </listitem>
       </varlistentry>
+
+      <varlistentry>
+<term><option>--no-sync</option></term>
+      <listitem>
+      <para>
+        Do not sync restored files to disk. You can use this flag to speed
+        up restore process. Using this flag can result in data
+        corruption in case of operating system or hardware crash.
+      </para>
+      </listitem>
+      </varlistentry>
     </variablelist>
     </para>
       <para>
@@ -3831,7 +3884,9 @@ pg_probackup merge -B <replaceable>backup_dir</replaceable> --instance <replacea
       <para>
         Merges the specified incremental backup to its parent full
         backup, together with all incremental backups between them, if
-        any. As a result, the full backup takes in all the merged
+        any. If the specified backup ID belong to the full backup,
+        then it will be merged with the closest incremental backup.
+        As a result, the full backup takes in all the merged
         data, and the incremental backups are removed as redundant.
       </para>
       <para>

src/merge.c

@@ -106,6 +106,27 @@ do_merge(time_t backup_id)
 		}
 	}
 
+	/*
+	 * Handle the case of crash right after deletion of the target
+	 * incremental backup. We still can recover from this.
+	 * Iterate over backups and look for the FULL backup with
+	 * MERGED status, that has merge-target-id eqial to backup_id.
+	 */
+	if (dest_backup == NULL)
+	{
+		for (i = 0; i < parray_num(backups); i++)
+		{
+			pgBackup   *backup = (pgBackup *) parray_get(backups, i);
+
+			if (backup->status == BACKUP_STATUS_MERGED &&
+				backup->merge_dest_backup == backup_id)
+			{
+				dest_backup = backup;
+				break;
+			}
+		}
+	}
+
 	if (dest_backup == NULL)
 		elog(ERROR, "Target backup %s was not found", base36enc(backup_id));
 
@@ -776,6 +797,11 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup)
 				 full_backup->root_dir, destination_path, strerror(errno));
 	}
 
+	/* If we crash here, it will produce full backup in MERGED
+	 * status, located in directory with wrong backup id.
+	 * It should not be a problem.
+	 */
+
 	/*
 	 * Merging finished, now we can safely update ID of the FULL backup
 	 */

src/pg_probackup.c

@@ -90,6 +90,8 @@ static bool			target_immediate;
 static char		   *target_name = NULL;
 static char		   *target_action = NULL;
 
+static char *primary_conninfo = NULL;
+
 static pgRecoveryTarget *recovery_target_options = NULL;
 static pgRestoreParams *restore_params = NULL;
 
@@ -194,6 +196,7 @@ static ConfigOption cmd_options[] =
 	{ 'b', 156, "skip-external-dirs", &skip_external_dirs,	SOURCE_CMD_STRICT },
 	{ 'f', 158, "db-include", 		opt_datname_include_list, SOURCE_CMD_STRICT },
 	{ 'f', 159, "db-exclude", 		opt_datname_exclude_list, SOURCE_CMD_STRICT },
+	{ 's', 160, "primary-conninfo",	&primary_conninfo,	SOURCE_CMD_STRICT },
 	/* checkdb options */
 	{ 'b', 195, "amcheck",			&need_amcheck,		SOURCE_CMD_STRICT },
 	{ 'b', 196, "heapallindexed",	&heapallindexed,	SOURCE_CMD_STRICT },
@@ -690,6 +693,7 @@ main(int argc, char *argv[])
 		restore_params->skip_external_dirs = skip_external_dirs;
 		restore_params->partial_db_list = NULL;
 		restore_params->partial_restore_type = NONE;
+		restore_params->primary_conninfo = primary_conninfo;
 
 		/* handle partial restore parameters */
 		if (datname_exclude_list && datname_include_list)

src/pg_probackup.h

@@ -422,6 +422,7 @@ typedef struct pgRestoreParams
 	/* options for partial restore */
 	PartialRestoreType partial_restore_type;
 	parray *partial_db_list;
+	const char *primary_conninfo;
 } pgRestoreParams;
 
 /* Options needed for set-backup command */

src/restore.c

@@ -1059,7 +1059,9 @@ create_recovery_conf(time_t backup_id,
 		fio_fprintf(fp, "standby_mode = 'on'\n");
 #endif
 
-		if (backup->primary_conninfo)
+		if (params->primary_conninfo)
+			fio_fprintf(fp, "primary_conninfo = '%s'\n", params->primary_conninfo);
+		else if (backup->primary_conninfo)
 			fio_fprintf(fp, "primary_conninfo = '%s'\n", backup->primary_conninfo);
 	}
 

tests/merge.py

@@ -2308,17 +2308,81 @@ def test_smart_merge(self):
         # Clean after yourself
         self.del_test_dir(module_name, fname)
 
+    def test_idempotent_merge(self):
+        """
+        """
+        fname = self.id().split('.')[3]
+        backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
+        node = self.make_simple_node(
+            base_dir=os.path.join(module_name, fname, 'node'),
+            set_replication=True,
+            initdb_params=['--data-checksums'],
+            pg_options={'autovacuum': 'off'})
+
+        self.init_pb(backup_dir)
+        self.add_instance(backup_dir, 'node', node)
+        self.set_archiving(backup_dir, 'node', node)
+        node.slow_start()
+
+        # add database
+        node.safe_psql(
+            'postgres',
+            'CREATE DATABASE testdb')
+
+        # take FULL backup
+        full_id = self.backup_node(
+            backup_dir, 'node', node, options=['--stream'])
+
+        # create database
+        node.safe_psql(
+            'postgres',
+            'create DATABASE testdb1')
+
+        # take PAGE backup
+        page_id = self.backup_node(
+            backup_dir, 'node', node, backup_type='page')
+
+        # create database
+        node.safe_psql(
+            'postgres',
+            'create DATABASE testdb2')
+
+        page_id_2 = self.backup_node(
+            backup_dir, 'node', node, backup_type='page')
+
+        gdb = self.merge_backup(
+            backup_dir, 'node', page_id_2,
+            gdb=True, options=['--log-level-console=verbose'])
+
+        gdb.set_breakpoint('delete_backup_files')
+        gdb.run_until_break()
+        gdb.remove_all_breakpoints()
+
+        gdb.set_breakpoint('rename')
+        gdb.continue_execution_until_break()
+        gdb.continue_execution_until_break(2)
+
+        gdb._execute('signal SIGKILL')
+
+        show_backups = self.show_pb(backup_dir, "node")
+        self.assertEqual(len(show_backups), 1)
+
+        self.assertEqual(
+            'MERGED', self.show_pb(backup_dir, 'node')[0]['status'])
+
+        self.assertEqual(
+            full_id, self.show_pb(backup_dir, 'node')[0]['id'])
+
+        self.merge_backup(backup_dir, 'node', page_id_2)
+
+        self.assertEqual(
+            'OK', self.show_pb(backup_dir, 'node')[0]['status'])
 
-# 1. always use parent link when merging (intermediates may be from different chain)
-# 2. page backup we are merging with may disappear after failed merge,
-# it should not be possible to continue merge after that
-#   PAGE_A  MERGING (disappear)
-#   FULL    MERGING
+        self.assertEqual(
+            page_id_2, self.show_pb(backup_dir, 'node')[0]['id'])
 
-#   FULL    MERGING
 
-#   PAGE_B  OK      (new backup)
-#   FULL    MERGING
+        self.del_test_dir(module_name, fname)
 
-# 3. Need new test with corrupted FULL backup
-# 4. different compression levels
+# 1. Need new test with corrupted FULL backup
+# 2. different compression levels

tests/restore.py

@@ -3376,4 +3376,54 @@ def test_stream_restore_command_option(self):
         self.assertEqual('2', timeline_id)
 
         # Clean after yourself
-        self.del_test_dir(module_name, fname)
+        self.del_test_dir(module_name, fname)
+
+    # @unittest.skip("skip")
+    def test_restore_primary_conninfo(self):
+        """
+        """
+        fname = self.id().split('.')[3]
+        node = self.make_simple_node(
+            base_dir=os.path.join(module_name, fname, 'node'),
+            set_replication=True,
+            initdb_params=['--data-checksums'])
+
+        backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
+        self.init_pb(backup_dir)
+        self.add_instance(backup_dir, 'node', node)
+        node.slow_start()
+
+        # Take FULL
+        self.backup_node(backup_dir, 'node', node, options=['--stream'])
+
+        node.pgbench_init(scale=1)
+
+        #primary_conninfo = 'host=192.168.1.50 port=5432 user=foo password=foopass'
+
+        replica = self.make_simple_node(
+            base_dir=os.path.join(module_name, fname, 'replica'))
+        replica.cleanup()
+        str_conninfo='host=192.168.1.50 port=5432 user=foo password=foopass'
+
+        self.restore_node(
+            backup_dir, 'node', replica,
+            options=['-R', '--primary-conninfo={0}'.format(str_conninfo)])
+
+        if self.get_version(node) >= self.version_to_num('12.0'):
+            standby_signal = os.path.join(replica.data_dir, 'standby.signal')
+            self.assertTrue(
+                os.path.isfile(standby_signal),
+                "File '{0}' do not exists".format(standby_signal))
+
+        if self.get_version(node) >= self.version_to_num('12.0'):
+            recovery_conf = os.path.join(replica.data_dir, 'probackup_recovery.conf')
+        else:
+            recovery_conf = os.path.join(replica.data_dir, 'recovery.conf')
+
+        with open(os.path.join(replica.data_dir, recovery_conf), 'r') as f:
+            recovery_conf_content = f.read()
+
+        self.assertIn(str_conninfo, recovery_conf_content)
+
+        # Clean after yourself
+        self.del_test_dir(module_name, fname)