From bb2ec18f672d850c043633f154cc196196995b29 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 21 May 2019 19:13:39 -0400 Subject: [PATCH] Fix off-by-one error in an index shard test There is an off-by-one error in this test. It leads to the recovery thread never being started, and that means joining on it will wait indefinitely. This commit addresses that by fixing the off-by-one error. Closes #42325 --- .../index/shard/IndexShardTests.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java index 64b0c0db1dc8c..64886af18332a 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java @@ -1736,7 +1736,6 @@ public void testLockingBeforeAndAfterRelocated() throws Exception { closeShards(shard); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/42325") public void testDelayedOperationsBeforeAndAfterRelocated() throws Exception { final IndexShard shard = newStartedShard(true); IndexShardTestCase.updateRoutingEntry(shard, ShardRoutingHelper.relocate(shard.routingEntry(), "other_node")); @@ -1754,12 +1753,11 @@ public void testDelayedOperationsBeforeAndAfterRelocated() throws Exception { recoveryThread.start(); final int numberOfAcquisitions = randomIntBetween(1, 10); - final int recoveryIndex = randomIntBetween(1, numberOfAcquisitions); + final List assertions = new ArrayList<>(numberOfAcquisitions); + final int recoveryIndex = randomIntBetween(0, numberOfAcquisitions - 1); for (int i = 0; i < numberOfAcquisitions; i++) { - final PlainActionFuture onLockAcquired; - final Runnable assertion; if (i < recoveryIndex) { final AtomicBoolean invoked = new AtomicBoolean(); onLockAcquired = new PlainActionFuture<>() { @@ -1777,26 +1775,29 @@ public void onFailure(Exception e) { } }; - assertion = () -> assertTrue(invoked.get()); + assertions.add(() -> assertTrue(invoked.get())); } else if (recoveryIndex == i) { startRecovery.countDown(); relocationStarted.await(); onLockAcquired = new PlainActionFuture<>(); - assertion = () -> { + assertions.add(() -> { final ExecutionException e = expectThrows(ExecutionException.class, () -> onLockAcquired.get(30, TimeUnit.SECONDS)); assertThat(e.getCause(), instanceOf(ShardNotInPrimaryModeException.class)); assertThat(e.getCause(), hasToString(containsString("shard is not in primary mode"))); - }; + }); } else { onLockAcquired = new PlainActionFuture<>(); - assertion = () -> { + assertions.add(() -> { final ExecutionException e = expectThrows(ExecutionException.class, () -> onLockAcquired.get(30, TimeUnit.SECONDS)); assertThat(e.getCause(), instanceOf(ShardNotInPrimaryModeException.class)); assertThat(e.getCause(), hasToString(containsString("shard is not in primary mode"))); - }; + }); } shard.acquirePrimaryOperationPermit(onLockAcquired, ThreadPool.Names.WRITE, "i_" + i); + } + + for (final Runnable assertion : assertions) { assertion.run(); }