Improve robustness of cluster version bookkeeping

Summary:
Ref T4292. Small fixes:

  - There was a bug with the //first// write, where we'd write 1 but expect 0. Fix this.
  - Narrow the window where we hold the `isWriting` lock: we don't need to wait for the client to finish.
  - Release the lock even if something throws.
  - Use a more useful variable name.

Test Plan:
  - Made new writes to a fresh cluster repository.
  - Made sequential writes.
  - Made concurrent writes.
  - Made good writes and bad writes.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4292

Differential Revision: https://secure.phabricator.com/D15747

src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php

@@ -21,22 +21,30 @@ final class DiffusionGitReceivePackSSHWorkflow extends DiffusionGitSSHWorkflow {
 
     if ($this->shouldProxy()) {
       $command = $this->getProxyCommand();
-      $is_proxy = true;
+      $did_synchronize = false;
     } else {
       $command = csprintf('git-receive-pack %s', $repository->getLocalPath());
-      $is_proxy = false;
 
+      $did_synchronize = true;
       $repository->synchronizeWorkingCopyBeforeWrite();
     }
-    $command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
 
-    $future = id(new ExecFuture('%C', $command))
-      ->setEnv($this->getEnvironment());
+    $caught = null;
+    try {
+      $err = $this->executeRepositoryCommand($command);
+    } catch (Exception $ex) {
+      $caught = $ex;
+    }
 
-    $err = $this->newPassthruCommand()
-      ->setIOChannel($this->getIOChannel())
-      ->setCommandChannelFromExecFuture($future)
-      ->execute();
+    // We've committed the write (or rejected it), so we can release the lock
+    // without waiting for the client to receive the acknowledgement.
+    if ($did_synchronize) {
+      $repository->synchronizeWorkingCopyAfterWrite();
+    }
+
+    if ($caught) {
+      throw $caught;
+    }
 
     if (!$err) {
       $repository->writeStatusMessage(
@@ -45,11 +53,20 @@ final class DiffusionGitReceivePackSSHWorkflow extends DiffusionGitSSHWorkflow {
       $this->waitForGitClient();
     }
 
-    if (!$is_proxy) {
-      $repository->synchronizeWorkingCopyAfterWrite();
-    }
-
     return $err;
   }
 
+  private function executeRepositoryCommand($command) {
+    $repository = $this->getRepository();
+    $command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
+
+    $future = id(new ExecFuture('%C', $command))
+      ->setEnv($this->getEnvironment());
+
+    return $this->newPassthruCommand()
+      ->setIOChannel($this->getIOChannel())
+      ->setCommandChannelFromExecFuture($future)
+      ->execute();
+  }
+
 }

src/applications/repository/storage/PhabricatorRepository.php

@@ -2410,9 +2410,6 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
 
 
   private function shouldEnableSynchronization() {
-    // TODO: This mostly works, but isn't stable enough for production yet.
-    return false;
-
     $device = AlmanacKeys::getLiveDevice();
     if (!$device) {
       return false;

src/applications/repository/storage/PhabricatorRepositoryWorkingCopyVersion.php

@@ -82,7 +82,7 @@ final class PhabricatorRepositoryWorkingCopyVersion
       $table,
       $repository_phid,
       $device_phid,
-      1,
+      0,
       1);
   }