opendev/gerrit
Code Issues Proposed changes

Fix retrying of index queries

Browse Source 
Since change I9b577a827 we provide an InternalQuery instance to the
retryable action that executes an index query. Unfortunately for all
retries we always provide the same InternalQuery instance to the action,
but InternalQuery instances can be used only once. Fix this by creating
a new InternalQuery instance for each retry.

Example stacktrace:
java.lang.IllegalStateException: AccountQueryProcessor has already been used
        at com.google.common.base.Preconditions.checkState(Preconditions.java:589)
        at com.google.gerrit.index.query.QueryProcessor.query(QueryProcessor.java:206)
        at com.google.gerrit.index.query.QueryProcessor.query(QueryProcessor.java:194)
        at com.google.gerrit.index.query.QueryProcessor.query(QueryProcessor.java:178)
        at com.google.gerrit.index.query.InternalQuery.queryResults(InternalQuery.java:97)
        at com.google.gerrit.index.query.InternalQuery.query(InternalQuery.java:92)
        at com.google.gerrit.server.query.account.InternalAccountQuery.byPreferredEmail(InternalAccountQuery.java:97)
        at com.google.gerrit.server.account.Emails.lambda$getAccountFor$0(Emails.java:78)
        at com.google.gerrit.server.update.RetryableIndexQueryAction.lambda$new$0(RetryableIndexQueryAction.java:48)
        at com.github.rholder.retry.AttemptTimeLimiters$NoAttemptTimeLimit.call(AttemptTimeLimiters.java:78)
        at com.github.rholder.retry.Retryer.call(Retryer.java:160)
        at com.google.gerrit.server.update.RetryHelper.executeWithTimeoutCount(RetryHelper.java:555)
        at com.google.gerrit.server.update.RetryHelper.execute(RetryHelper.java:504)
        at com.google.gerrit.server.update.RetryableAction.call(RetryableAction.java:171)
        at com.google.gerrit.server.update.RetryableIndexQueryAction.call(RetryableIndexQueryAction.java:84)
        at com.google.gerrit.server.account.Emails.getAccountFor(Emails.java:78)
        at com.google.gerrit.server.account.AccountResolver$ByEmail.search(AccountResolver.java:371)
        at com.google.gerrit.server.account.AccountResolver$ByEmail.search(AccountResolver.java:363)
        at com.google.gerrit.server.account.AccountResolver$Searcher.trySearch(AccountResolver.java:227)
        at com.google.gerrit.server.account.AccountResolver.searchImpl(AccountResolver.java:595)
        at com.google.gerrit.server.account.AccountResolver.resolve(AccountResolver.java:519)
        at com.google.gerrit.server.restapi.account.AccountsCollection.parse(AccountsCollection.java:53)
        at com.google.gerrit.server.restapi.account.AccountsCollection.parse(AccountsCollection.java:33)
        at com.google.gerrit.httpd.restapi.RestApiServlet.lambda$parseResourceWithRetry$2(RestApiServlet.java:727)
        at com.github.rholder.retry.AttemptTimeLimiters$NoAttemptTimeLimit.call(AttemptTimeLimiters.java:78)
        at com.github.rholder.retry.Retryer.call(Retryer.java:160)
        at com.google.gerrit.server.update.RetryHelper.executeWithTimeoutCount(RetryHelper.java:555)
        at com.google.gerrit.server.update.RetryHelper.execute(RetryHelper.java:504)
        at com.google.gerrit.server.update.RetryableAction.call(RetryableAction.java:171)
        at com.google.gerrit.httpd.restapi.RestApiServlet.invokeRestEndpointWithRetry(RestApiServlet.java:833)
        at com.google.gerrit.httpd.restapi.RestApiServlet.parseResourceWithRetry(RestApiServlet.java:722)
        at com.google.gerrit.httpd.restapi.RestApiServlet.service(RestApiServlet.java:387)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:717)
        ...

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ib9e07e7c097115ea2cedbbf0c012ee20cfe0a5eb
This commit is contained in:
Edwin Kempin
2019-12-18 09:43:34 +01:00
parent 96c56f5180
commit 03a77c48be
2 changed files with 9 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
java/com/google/gerrit/server/update/RetryHelper.java
View File

@@ -363,7 +363,7 @@ public class RetryHelper {
public <T> RetryableIndexQueryAction<InternalAccountQuery, T> accountIndexQuery(
String actionName, IndexQueryAction<T, InternalAccountQuery> indexQueryAction) {
return new RetryableIndexQueryAction<>(
this, internalAccountQuery.get(), actionName, indexQueryAction);
this, internalAccountQuery, actionName, indexQueryAction);
}
/**
@@ -379,8 +379,7 @@ public class RetryHelper {
*/
public <T> RetryableIndexQueryAction<InternalChangeQuery, T> changeIndexQuery(
String actionName, IndexQueryAction<T, InternalChangeQuery> indexQueryAction) {
return new RetryableIndexQueryAction<>(
this, internalChangeQuery.get(), actionName, indexQueryAction);
return new RetryableIndexQueryAction<>(this, internalChangeQuery, actionName, indexQueryAction);
}
/**

9
java/com/google/gerrit/server/update/RetryableIndexQueryAction.java
View File

@@ -18,6 +18,7 @@ import com.github.rholder.retry.RetryListener;
import com.google.common.base.Throwables;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.query.InternalQuery;
import com.google.inject.Provider;
import java.util.function.Consumer;
import java.util.function.Predicate;
@@ -42,10 +43,14 @@ public class RetryableIndexQueryAction<Q extends InternalQuery<?, Q>, T>
RetryableIndexQueryAction(
RetryHelper retryHelper,
Q internalQuery,
Provider<Q> internalQuery,
String actionName,
IndexQueryAction<T, Q> indexQuery) {
super(retryHelper, ActionType.INDEX_QUERY, actionName, () -> indexQuery.call(internalQuery));
super(
retryHelper,
ActionType.INDEX_QUERY,
actionName,
() -> indexQuery.call(internalQuery.get()));
}
@Override