diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt index 47e2505cdc..229c463f11 100644 --- a/Documentation/metrics.txt +++ b/Documentation/metrics.txt @@ -13,6 +13,13 @@ The following metrics are reported. * `build/label`: Version of Gerrit server software. * `events`: Triggered events. +=== Actions + +* `action/retry_attempt_counts`: Distribution of number of attempts made +by RetryHelper to execute an action (1 == single attempt, no retry) +* `action/retry_timeout_count`: Number of action executions of RetryHelper +that ultimately timed out + === Process * `proc/birth_timestamp`: Time at which the Gerrit process started. @@ -87,10 +94,6 @@ topic submissions that concluded successfully. * `batch_update/execute_change_ops`: BatchUpdate change update latency, excluding reindexing -* `batch_update/retry_attempt_counts`: Distribution of number of attempts made -by RetryHelper (1 == single attempt, no retry) -* `batch_update/retry_timeout_count`: Number of executions of RetryHelper that -ultimately timed out === NoteDb diff --git a/java/com/google/gerrit/server/account/AccountsUpdate.java b/java/com/google/gerrit/server/account/AccountsUpdate.java index f88f7e25cd..7f9825f383 100644 --- a/java/com/google/gerrit/server/account/AccountsUpdate.java +++ b/java/com/google/gerrit/server/account/AccountsUpdate.java @@ -37,6 +37,7 @@ import com.google.gerrit.server.index.change.ReindexAfterRefUpdate; import com.google.gerrit.server.mail.send.OutgoingEmailValidator; import com.google.gerrit.server.update.RefUpdateUtil; import com.google.gerrit.server.update.RetryHelper; +import com.google.gerrit.server.update.RetryHelper.ActionType; import com.google.gwtorm.server.OrmDuplicateKeyException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -416,6 +417,7 @@ public class AccountsUpdate { private Account deleteAccount(Account.Id accountId) throws IOException, OrmException, ConfigInvalidException { return retryHelper.execute( + ActionType.ACCOUNT_UPDATE, () -> { deleteUserBranch(accountId); return null; @@ -468,6 +470,7 @@ public class AccountsUpdate { private Account updateAccount(AccountUpdate accountUpdate) throws IOException, ConfigInvalidException, OrmException { return retryHelper.execute( + ActionType.ACCOUNT_UPDATE, () -> { try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) { UpdatedAccount updatedAccount = accountUpdate.update(allUsersRepo); diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java b/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java index 8a05a6c95b..028fd8d146 100644 --- a/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java +++ b/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java @@ -28,6 +28,7 @@ import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.update.RetryHelper; +import com.google.gerrit.server.update.RetryHelper.ActionType; import com.google.gwtorm.server.OrmDuplicateKeyException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -427,6 +428,7 @@ public class ExternalIdsUpdate { private void updateNoteMap(ExternalIdUpdater updater) throws IOException, ConfigInvalidException, OrmException { retryHelper.execute( + ActionType.ACCOUNT_UPDATE, () -> { try (Repository repo = repoManager.openRepository(allUsersName)) { ExternalIdNotes extIdNotes = diff --git a/java/com/google/gerrit/server/update/RetryHelper.java b/java/com/google/gerrit/server/update/RetryHelper.java index 54726fe43a..161169fc3d 100644 --- a/java/com/google/gerrit/server/update/RetryHelper.java +++ b/java/com/google/gerrit/server/update/RetryHelper.java @@ -32,9 +32,10 @@ import com.google.common.base.Predicate; import com.google.common.base.Throwables; import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.restapi.RestApiException; -import com.google.gerrit.metrics.Counter0; +import com.google.gerrit.metrics.Counter1; import com.google.gerrit.metrics.Description; -import com.google.gerrit.metrics.Histogram0; +import com.google.gerrit.metrics.Field; +import com.google.gerrit.metrics.Histogram1; import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.git.LockFailureException; @@ -61,6 +62,11 @@ public class RetryHelper { T call() throws Exception; } + public enum ActionType { + ACCOUNT_UPDATE, + CHANGE_UPDATE + } + /** * Options for retrying a single operation. * @@ -96,25 +102,29 @@ public class RetryHelper { @VisibleForTesting @Singleton public static class Metrics { - final Histogram0 attemptCounts; - final Counter0 timeoutCount; + final Histogram1 attemptCounts; + final Counter1 timeoutCount; @Inject Metrics(MetricMaker metricMaker) { + Field view = Field.ofEnum(ActionType.class, "action_type"); attemptCounts = metricMaker.newHistogram( - "batch_update/retry_attempt_counts", + "action/retry_attempt_counts", new Description( - "Distribution of number of attempts made by RetryHelper" + "Distribution of number of attempts made by RetryHelper to execute an action" + " (1 == single attempt, no retry)") .setCumulative() - .setUnit("attempts")); + .setUnit("attempts"), + view); timeoutCount = metricMaker.newCounter( - "batch_update/retry_timeout_count", - new Description("Number of executions of RetryHelper that ultimately timed out") + "action/retry_timeout_count", + new Description( + "Number of action executions of RetryHelper that ultimately timed out") .setCumulative() - .setUnit("timeouts")); + .setUnit("timeouts"), + view); } } @@ -171,9 +181,10 @@ public class RetryHelper { return defaultTimeout; } - public T execute(Action action) throws IOException, ConfigInvalidException, OrmException { + public T execute(ActionType actionType, Action action) + throws IOException, ConfigInvalidException, OrmException { try { - return execute(action, defaults(), t -> t instanceof LockFailureException); + return execute(actionType, action, defaults(), t -> t instanceof LockFailureException); } catch (Throwable t) { Throwables.throwIfUnchecked(t); Throwables.throwIfInstanceOf(t, IOException.class); @@ -195,10 +206,13 @@ public class RetryHelper { // transactions. Either way, retrying a partially-failed operation is not idempotent, so // don't do it automatically. Let the end user decide whether they want to retry. return execute( - () -> changeAction.call(updateFactory), RetryerBuilder.newBuilder().build()); + ActionType.CHANGE_UPDATE, + () -> changeAction.call(updateFactory), + RetryerBuilder.newBuilder().build()); } return execute( + ActionType.CHANGE_UPDATE, () -> changeAction.call(updateFactory), opts, t -> { @@ -218,6 +232,7 @@ public class RetryHelper { /** * Executes an action with a given retryer. * + * @param actionType the type of the action * @param action the action which should be executed and retried on failure * @param opts options for retrying the action on failure * @param exceptionPredicate predicate to control on which exception the action should be retried @@ -225,33 +240,39 @@ public class RetryHelper { * @throws Throwable any error or exception that made the action fail, callers are expected to * catch and inspect this Throwable to decide carefully whether it should be re-thrown */ - private T execute(Action action, Options opts, Predicate exceptionPredicate) + private T execute( + ActionType actionType, + Action action, + Options opts, + Predicate exceptionPredicate) throws Throwable { MetricListener listener = new MetricListener(); try { RetryerBuilder retryerBuilder = createRetryerBuilder(opts, exceptionPredicate); retryerBuilder.withRetryListener(listener); - return execute(action, retryerBuilder.build()); + return execute(actionType, action, retryerBuilder.build()); } finally { - metrics.attemptCounts.record(listener.getAttemptCount()); + metrics.attemptCounts.record(actionType, listener.getAttemptCount()); } } /** * Executes an action with a given retryer. * + * @param actionType the type of the action * @param action the action which should be executed and retried on failure * @param retryer the retryer * @return the result of executing the action * @throws Throwable any error or exception that made the action fail, callers are expected to * catch and inspect this Throwable to decide carefully whether it should be re-thrown */ - private T execute(Action action, Retryer retryer) throws Throwable { + private T execute(ActionType actionType, Action action, Retryer retryer) + throws Throwable { try { return retryer.call(() -> action.call()); } catch (ExecutionException | RetryException e) { if (e instanceof RetryException) { - metrics.timeoutCount.increment(); + metrics.timeoutCount.increment(actionType); } if (e.getCause() != null) { throw e.getCause();