Skip to content

Commit

Permalink
Fix off-by-one error in an index shard test
Browse files Browse the repository at this point in the history
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 elastic#42325
  • Loading branch information
jasontedor committed May 21, 2019
1 parent a3bd569 commit bb2ec18
Showing 1 changed file with 10 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand All @@ -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<Runnable> assertions = new ArrayList<>(numberOfAcquisitions);
final int recoveryIndex = randomIntBetween(0, numberOfAcquisitions - 1);

for (int i = 0; i < numberOfAcquisitions; i++) {

final PlainActionFuture<Releasable> onLockAcquired;
final Runnable assertion;
if (i < recoveryIndex) {
final AtomicBoolean invoked = new AtomicBoolean();
onLockAcquired = new PlainActionFuture<>() {
Expand All @@ -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();
}

Expand Down

0 comments on commit bb2ec18

Please sign in to comment.