From f5f8d51366faae6ac31f355d021eaff67bef8f7f Mon Sep 17 00:00:00 2001
From: David Steele <david@pgmasters.net>
Date: Wed, 12 Apr 2017 09:53:07 -0400
Subject: [PATCH] Improved error message when the restore command detects the
 presence of postmaster.pid.

Suggested by Yogesh Sharma.
---
 doc/xml/release.xml                               | 8 ++++++++
 lib/pgBackRest/Restore.pm                         | 6 +++++-
 test/expect/full-synthetic-001.log                | 7 +++++++
 test/lib/pgBackRestTest/Full/FullSyntheticTest.pm | 9 +++++++++
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/doc/xml/release.xml b/doc/xml/release.xml
index f5d6d71bb..313d9f1fe 100644
--- a/doc/xml/release.xml
+++ b/doc/xml/release.xml
@@ -196,6 +196,14 @@
                         <p>Do not set timestamps of files in the backup directories to match timestamps in the cluster directory.  This was originally done to enable backup resume, but that process is now implemented with checksums.</p>
                     </release-item>
 
+                    <release-item>
+                        <release-item-contributor-list>
+                            <release-item-ideator id="sharma.yogesh"/>
+                        </release-item-contributor-list>
+
+                        <p>Improved error message when the <cmd>restore</cmd> command detects the presence of <file>postmaster.pid</file>.</p>
+                    </release-item>
+
                     <release-item>
                         <p>The <cmd>backup</cmd> and <cmd>restore</cmd> commands no longer copy via temp files.  In both cases the files are checksummed on resume so there's no danger of partial copies.</p>
                     </release-item>
diff --git a/lib/pgBackRest/Restore.pm b/lib/pgBackRest/Restore.pm
index 36bea8df4..a435e219f 100644
--- a/lib/pgBackRest/Restore.pm
+++ b/lib/pgBackRest/Restore.pm
@@ -1048,7 +1048,11 @@ sub process
     # Make sure that Postgres is not running
     if ($self->{oFile}->exists(PATH_DB_ABSOLUTE, $self->{strDbClusterPath} . '/' . DB_FILE_POSTMASTERPID))
     {
-        confess &log(ERROR, 'unable to restore while Postgres is running', ERROR_POSTMASTER_RUNNING);
+        confess &log(ERROR,
+            "unable to restore while PostgreSQL is running\n" .
+            "HINT: presence of '" . DB_FILE_POSTMASTERPID . "' in '$self->{strDbClusterPath}' indicates PostgreSQL is running.\n" .
+            "HINT: remove '" . DB_FILE_POSTMASTERPID . "' only if PostgreSQL is not running.",
+            ERROR_POSTMASTER_RUNNING);
     }
 
     # If the restore will be destructive attempt to verify that $PGDATA is valid
diff --git a/test/expect/full-synthetic-001.log b/test/expect/full-synthetic-001.log
index 1601bc1f7..8559a7a55 100644
--- a/test/expect/full-synthetic-001.log
+++ b/test/expect/full-synthetic-001.log
@@ -1151,6 +1151,13 @@ P00   WARN: backup group for pg_data/base/16384/PG_VERSION was not mapped to a n
 P00   WARN: backup user for pg_data/base/1/PG_VERSION was not mapped to a name, set to [USER-1]
 P00  ERROR: [115]: cannot restore file '[TEST_PATH]/db-master/db/pg_config/postgresql.conf' that already exists - try using --delta if this is what you intended
 
+restore, backup '[BACKUP-FULL-2]', expect exit 113 - error on postmaster.pid exists (db-master host)
+> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --set=[BACKUP-FULL-2] --log-level-console=warn  --stanza=db restore
+------------------------------------------------------------------------------------------------------------------------------------
+P00  ERROR: [113]: unable to restore while PostgreSQL is running
+            HINT: presence of 'postmaster.pid' in '[TEST_PATH]/db-master/db/base' indicates PostgreSQL is running.
+            HINT: remove 'postmaster.pid' only if PostgreSQL is not running.
+
 restore delta, backup '[BACKUP-FULL-2]' - restore all links --link-all and mapping (db-master host)
 > [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --delta --set=[BACKUP-FULL-2] --log-level-console=detail  --link-map=pg_stat=../pg_stat  --link-all  --stanza=db restore
 ------------------------------------------------------------------------------------------------------------------------------------
diff --git a/test/lib/pgBackRestTest/Full/FullSyntheticTest.pm b/test/lib/pgBackRestTest/Full/FullSyntheticTest.pm
index d8cab2b0a..68dbbadf3 100644
--- a/test/lib/pgBackRestTest/Full/FullSyntheticTest.pm
+++ b/test/lib/pgBackRestTest/Full/FullSyntheticTest.pm
@@ -543,6 +543,15 @@ sub run
                 $strFullBackup, \%oManifest, undef, $bDelta, $bForce, undef, undef, undef, undef, undef, undef,
                 'error on existing linked file', ERROR_PATH_NOT_EMPTY, '--log-level-console=warn --link-all');
 
+            # Error when postmaster.pid is present
+            executeTest('touch ' . $oHostDbMaster->dbBasePath() . qw(/) . DB_FILE_POSTMASTERPID);
+
+            $oHostDbMaster->restore(
+                $strFullBackup, \%oManifest, undef, $bDelta, $bForce, undef, undef, undef, undef, undef, undef,
+                'error on postmaster.pid exists', ERROR_POSTMASTER_RUNNING, '--log-level-console=warn');
+
+            executeTest('rm ' . $oHostDbMaster->dbBasePath() . qw(/) . DB_FILE_POSTMASTERPID);
+
             # Now a combination of remapping
             $bDelta = true;