opendev/gerrit
Code Issues Proposed changes

ReceiveCommits: Retry scanning for changes to auto-close

Browse Source 
Accessing the secondary index may sometimes fail due to a temporary
issue with the index. In this case we want to retry scanning for changes
to auto-close.

This is especially important because the push succeeds even if
auto-closing changes fails. If this happens the changes are in an
inconsistent state, they are still open but the changes were actually
already merged into the target branch. Since the push succeeded the user
cannot retry the operation to get the auto-closing done.

Temporary issues with accessing the index are not common with Lucene but
at Google where we have a custom index implementation we have such
short-term failures frequently.

Using RetryHelper to retry scanning for changes to auto-close is not
optimal because RetryHelper is designed for retrying updates to NoteDb
(e.g. the RetryHelper configuration and the exception handling is
specific for NoteDb updates).

Change-Id: I8c16487780f40d99b9f843ac5bac4c35776e726f
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2018-01-04 13:48:23 +01:00
parent 9c6f12c752
commit ced85fc9d2
2 changed files with 23 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
java/com/google/gerrit/server/git/receive/ReceiveCommits.java
View File

@@ -148,6 +148,7 @@ import com.google.gerrit.server.update.Context;
import com.google.gerrit.server.update.RepoContext;
import com.google.gerrit.server.update.RepoOnlyOp;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryHelper.ActionType;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.LabelVote;
import com.google.gerrit.server.util.MagicBranch;
@@ -2868,7 +2869,11 @@ class ReceiveCommits {
for (Ref ref : byCommit.get(c.copy())) {
PatchSet.Id psId = PatchSet.Id.fromRef(ref.getName());
Optional<ChangeData> cd = byLegacyId(psId.getParentKey());
Optional<ChangeData> cd =
retryHelper.execute(
ActionType.CHANGE_QUERY,
() -> byLegacyId(psId.getParentKey()),
t -> t instanceof OrmException);
if (cd.isPresent() && cd.get().change().getDest().equals(branch)) {
existingPatchSets++;
bu.addOp(
@@ -2880,7 +2885,11 @@ class ReceiveCommits {
for (String changeId : c.getFooterLines(CHANGE_ID)) {
if (byKey == null) {
byKey = openChangesByKeyByBranch(branch);
byKey =
retryHelper.execute(
ActionType.CHANGE_QUERY,
() -> openChangesByKeyByBranch(branch),
t -> t instanceof OrmException);
}
ChangeNotes onto = byKey.get(new Change.Key(changeId.trim()));
@@ -2926,7 +2935,10 @@ class ReceiveCommits {
logError("Failed to auto-close changes", e);
}
return null;
});
},
// Use a multiple of the default timeout to account for inner retries that may otherwise
// eat up the whole timeout so that no time is left to retry this outer action.
RetryHelper.options().timeout(retryHelper.getDefaultTimeout().multipliedBy(5)).build());
} catch (RestApiException e) {
logError("Can't insert patchset", e);
} catch (UpdateException e) {