ReceiveCommits: Call end() on progress monitor only at the very end
After end() has been called on the progress monitor the ReceiveCommits
task has 500ms to terminate and return, otherwise MultiProgressMonitor
will get a TimeoutException while waiting on the future to return. The
500ms timeout is hard-coded in MultiProgressMonitor. At Google we had
some reports of users that got an error on push due to this timeout. At
this point the push was already successful, so it's confusing for the
user to get an error. Calling end() in ReceiveCommits only at the very
end may safe some milliseconds in between calling end() and returning so
that hitting this timeout gets less likely. On first sight moving the
end() call a few lines down doesn't look like it's safing a lot, but now
it's only done after the AutoCloseables from the try-block have been
closed. One of the AutoCloseables that is being closed here is the
PerformanceLogContext. When it is closed the performance log entries
which have been collected during the request are forwarded to the
registered PerformanceLoggers which usually persist the performance log
entries. Doing this can take a while, so it's better to call end() on
the progress monitor only afterwards.
Issue: Google b/202848945
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ic466d07c23d33325b6ab4ee6b0dc6d14a82010b8
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index c1cd30c..66e7d80 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -709,10 +709,10 @@
sendErrorMessages();
commandProgress.end();
- progress.end();
loggingTags = traceContext.getTags();
logger.atFine().log("Processing commands done.");
}
+ progress.end();
return result.build();
}
