Remove index.waitTimeout configuration property
Lock acquisition timeout is set to 5 milliseconds. Increasing the
timeout causes healthcheck plugin to report temporary deadlock and
mark the entire node as unhealthy.
For that reason there is no point to increase timeout to more than
a few milliseconds.
Use `http.socketTimeout` and `http.maxTries` to control for how long
the plugin will keep retrying to acquire the lock.
Feature: Issue 13561
Change-Id: I01d6c42a0f9ac801a52b9e5915502d0eea54f134
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java
index 3bd58a3..4743bee 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/Configuration.java
@@ -455,7 +455,6 @@
private final int threadPoolSize;
private final int retryInterval;
private final int maxTries;
- private final int waitTimeout;
private final int numStripedLocks;
private final boolean synchronizeForced;
@@ -465,7 +464,6 @@
numStripedLocks = getInt(cfg, INDEX_SECTION, NUM_STRIPED_LOCKS, DEFAULT_NUM_STRIPED_LOCKS);
retryInterval = getInt(cfg, INDEX_SECTION, RETRY_INTERVAL_KEY, DEFAULT_INDEX_RETRY_INTERVAL);
maxTries = getInt(cfg, INDEX_SECTION, MAX_TRIES_KEY, DEFAULT_INDEX_MAX_TRIES);
- waitTimeout = getInt(cfg, INDEX_SECTION, WAIT_TIMEOUT_KEY, DEFAULT_TIMEOUT_MS);
synchronizeForced =
cfg.getBoolean(INDEX_SECTION, SYNCHRONIZE_FORCED_KEY, DEFAULT_SYNCHRONIZE_FORCED);
}
@@ -486,10 +484,6 @@
return maxTries;
}
- public int waitTimeout() {
- return waitTimeout;
- }
-
public boolean synchronizeForced() {
return synchronizeForced;
}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventLocks.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventLocks.java
index 7dad07d..3fd26db 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventLocks.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventLocks.java
@@ -28,15 +28,14 @@
private static final FluentLogger log = FluentLogger.forEnclosingClass();
private static final int NUMBER_OF_INDEX_TASK_TYPES = 4;
+ private static final int WAIT_TIMEOUT_MS = 5;
private final Striped<Semaphore> semaphores;
- private final long waitTimeout;
@Inject
public IndexEventLocks(Configuration cfg) {
this.semaphores =
Striped.semaphore(NUMBER_OF_INDEX_TASK_TYPES * cfg.index().numStripedLocks(), 1);
- this.waitTimeout = cfg.index().waitTimeout();
}
public CompletableFuture<?> withLock(
@@ -45,7 +44,7 @@
Semaphore idSemaphore = getSemaphore(indexId);
try {
log.atFine().log("Trying to acquire %s", id);
- if (idSemaphore.tryAcquire(waitTimeout, TimeUnit.MILLISECONDS)) {
+ if (idSemaphore.tryAcquire(WAIT_TIMEOUT_MS, TimeUnit.MILLISECONDS)) {
log.atFine().log("Acquired %s", id);
return function
.invoke()
@@ -65,7 +64,7 @@
String timeoutMessage =
String.format(
"Acquisition of the locking of %s timed out after %d msec: consider increasing the number of shards",
- indexId, waitTimeout);
+ indexId, WAIT_TIMEOUT_MS);
log.atWarning().log(timeoutMessage);
lockAcquireTimeoutCallback.invoke();
CompletableFuture<?> failureFuture = new CompletableFuture<>();
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index f5c786f..489c9f4 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -207,13 +207,8 @@
: The interval of time in milliseconds between the subsequent auto-retries.
Defaults to 30000 (30 seconds).
-```index.waitTimeout```
-: Maximum interval of time in milliseconds the plugin waits to acquire
- the lock for an indexing call. When not specified, the default value
- is set to 5000ms.
-
-NOTE: the default settings for `http.socketTimeout`, `http.maxTries` and `index.waitTimeout`
-ensure that the plugin will keep retrying to forward a message for one hour.
+NOTE: the default settings for `http.socketTimeout` and `http.maxTries` ensure
+that the plugin will keep retrying to forward a message for one hour.
```websession.synchronize```
: Whether to synchronize web sessions.
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventHandlerTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventHandlerTest.java
index 17013cc..0f21ff7 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventHandlerTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/index/IndexEventHandlerTest.java
@@ -110,7 +110,6 @@
Configuration.Index cfgIndex = mock(Configuration.Index.class);
when(configuration.index()).thenReturn(cfgIndex);
when(cfgIndex.numStripedLocks()).thenReturn(Configuration.DEFAULT_NUM_STRIPED_LOCKS);
- when(cfgIndex.waitTimeout()).thenReturn(INDEX_WAIT_TIMEOUT_MS);
Configuration.Http http = mock(Configuration.Http.class);
when(configuration.http()).thenReturn(http);
