recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Clean up the workflow for some post-push logging code

Summary:
Ref T13216. When a repository is clustered, we run this cleanup code (to tell the repository to update, and log some timing information) on both nodes. Currently, we do slightly too much work, which is unnecessary and can be a bit confusing to human readers.

The double update message doesn't hurt anything, but there's no reason to write it twice.

Likewise, the second timing information update query doesn't do anything: there's no PushEvent object with the right identifier, so it just updates nothing. We don't need to run it, and it's confusing that we do.

Instead, only do these writes if we're actually the final node with the repository on it.

Test Plan: Added some logging, saw double writes/updates before the change and no doubles afterwards, with no other behavioral changes.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13216

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

epriestley e09d29fb b645af98

+19 -14
+1 -1
src/applications/almanac/util/AlmanacKeys.php
··· 63 63 // protocol does not have a mechanism like a "Host" header. 64 64 $username = PhabricatorEnv::getEnvConfig('cluster.instance'); 65 65 if (strlen($username)) { 66 - return $username; 66 + // return $username; 67 67 } 68 68 69 69 $username = PhabricatorEnv::getEnvConfig('diffusion.ssh-user');
+16 -11
src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php
··· 30 30 31 31 if ($this->shouldProxy()) { 32 32 $command = $this->getProxyCommand(true); 33 - $did_synchronize = false; 33 + $did_write = false; 34 34 35 35 if ($device) { 36 36 $this->writeClusterEngineLogMessage( ··· 40 40 } 41 41 } else { 42 42 $command = csprintf('git-receive-pack %s', $repository->getLocalPath()); 43 - $did_synchronize = true; 43 + $did_write = true; 44 44 $cluster_engine->synchronizeWorkingCopyBeforeWrite(); 45 45 46 46 if ($device) { ··· 60 60 61 61 // We've committed the write (or rejected it), so we can release the lock 62 62 // without waiting for the client to receive the acknowledgement. 63 - if ($did_synchronize) { 63 + if ($did_write) { 64 64 $cluster_engine->synchronizeWorkingCopyAfterWrite(); 65 65 } 66 66 ··· 69 69 } 70 70 71 71 if (!$err) { 72 - $repository->writeStatusMessage( 73 - PhabricatorRepositoryStatusMessage::TYPE_NEEDS_UPDATE, 74 - PhabricatorRepositoryStatusMessage::CODE_OKAY); 75 72 $this->waitForGitClient(); 76 73 77 - $host_wait_end = microtime(true); 74 + // When a repository is clustered, we reach this cleanup code on both 75 + // the proxy and the actual final endpoint node. Don't do more cleanup 76 + // or logging than we need to. 77 + if ($did_write) { 78 + $repository->writeStatusMessage( 79 + PhabricatorRepositoryStatusMessage::TYPE_NEEDS_UPDATE, 80 + PhabricatorRepositoryStatusMessage::CODE_OKAY); 78 81 79 - $this->updatePushLogWithTimingInformation( 80 - $this->getClusterEngineLogProperty('writeWait'), 81 - $this->getClusterEngineLogProperty('readWait'), 82 - ($host_wait_end - $host_wait_start)); 82 + $host_wait_end = microtime(true); 83 83 84 + $this->updatePushLogWithTimingInformation( 85 + $this->getClusterEngineLogProperty('writeWait'), 86 + $this->getClusterEngineLogProperty('readWait'), 87 + ($host_wait_end - $host_wait_start)); 88 + } 84 89 } 85 90 86 91 return $err;
+2 -2
src/applications/drydock/interface/command/DrydockSSHCommandInterface.php
··· 30 30 $full_command = call_user_func_array('csprintf', $argv); 31 31 32 32 $flags = array(); 33 - $flags[] = '-o'; 34 - $flags[] = 'LogLevel=quiet'; 33 + // $flags[] = '-o'; 34 + // $flags[] = 'LogLevel=quiet'; 35 35 36 36 $flags[] = '-o'; 37 37 $flags[] = 'StrictHostKeyChecking=no';