Fix archive write lock being taken for the synchronous archive-get co…

…mmand. Reported by Uspen.
pgbackrest · May 21, 2018 · ada5c5c · ada5c5c
1 parent e69d5fb
commit ada5c5c
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 4 deletions.
diff --git a/doc/xml/release.xml b/doc/xml/release.xml
@@ -22,6 +22,14 @@
 
                         <p>Fix potential buffer overrun in error message handling.</p>
                     </release-item>
+
+                    <release-item>
+                        <release-item-contributor-list>
+                            <release-item-ideator id="uspen"/>
+                        </release-item-contributor-list>
+
+                        <p>Fix archive write lock being taken for the synchronous <cmd>archive-get</cmd> command.</p>
+                    </release-item>
                 </release-bug-list>
 
                 <release-improvement-list>

diff --git a/lib/pgBackRest/Protocol/Base/Minion.pm b/lib/pgBackRest/Protocol/Base/Minion.pm
@@ -149,8 +149,8 @@ sub process
     # Loop until the exit command is received
     eval
     {
-        # Aquire a lock if required (this will be determined by lockAcquire()).  This is done here so any errors will be transmitted
-        # through the protocol layer and cause a graceful shutdown rather than a remote abort.
+        # Acquire a lock if required (this will be determined by lockAcquire()).  This is done here so any errors will be
+        # transmitted through the protocol layer and cause a graceful shutdown rather than a remote abort.
         if (defined($strLockPath) && defined($strLockStanza))
         {
             eval

diff --git a/libc/xs/common/lock.xs b/libc/xs/common/lock.xs
@@ -21,7 +21,7 @@ CODE:
         cfgCommandSet(cfgCommandId(command));
 
         // Attempt to acquire the lock
-        if (cfgLockType() != lockTypeNone)
+        if (cfgLockRequired())
             RETVAL = lockAcquire(strNew(lockPath), strNew(stanza), cfgLockType(), lockTimeout, failOnNoLock);
     }
     MEM_CONTEXT_XS_TEMP_END();

diff --git a/test/lib/pgBackRestTest/Module/Protocol/ProtocolCommonMinionPerlTest.pm b/test/lib/pgBackRestTest/Module/Protocol/ProtocolCommonMinionPerlTest.pm
@@ -19,6 +19,7 @@ use pgBackRest::Common::Exception;
 use pgBackRest::Common::Io::Buffered;
 use pgBackRest::Common::Log;
 use pgBackRest::Common::Wait;
+use pgBackRest::LibC qw(:config);
 use pgBackRest::Protocol::Base::Minion;
 use pgBackRest::Version;
 
@@ -103,7 +104,9 @@ sub run
             my $oIoHandle = shift;
 
             my $oMinion = new pgBackRest::Protocol::Base::Minion('test', new pgBackRest::Common::Io::Buffered($oIoHandle, 5, 4096));
-            $oMinion->process();
+
+            # Use bogus lock path to ensure a lock is not taken for the archive-get command
+            $oMinion->process($self->testPath(), cfgCommandName(CFGCMD_ARCHIVE_GET), "test");
         });
 
         #---------------------------------------------------------------------------------------------------------------------------