Skip to content

Commit

Permalink
Stop NodeTests from timing out in certain cases
Browse files Browse the repository at this point in the history
The NodeTests class contains tests that check behavior when shutting
down a node. This involves starting a node, performing some operation,
stopping the node, and then awaiting the close of the node. Part of
closing a node is the termination of the node's ThreadPool. ThreadPool
termination semantics can be deceiving. The ThreadPool#terminate method
takes a timeout value and the first oddity is that the terminate method
can take two times the timeout value before returning. Internally this
method acts on the ExecutorService instances that are held by the
ThreadPool. First, an orderly shutdown is attempted and pending tasks
are allowed to execute while waiting for the timeout value. If any of
the ExecutorService instances have not terminated, a call is made to
attempt to stop all active tasks (usually using interrupts) and then
waits for up to the timeout value a second time for the termination of
the ExecutorService instances. This means that if use a large value
when waiting for a node to close, we may not attempt to interrupt any
threads that are in a blocking call before the test times out.

In order to avoid causing these tests to time out, this change reduces
the timeout passed to Node#awaitClose to 10 seconds from 1 day. This
will allow blocked threads to be interrupted before the test suite
fails due to the timeout.

Closes elastic#44256
Closes elastic#42350
  • Loading branch information
jaymode committed Nov 15, 2019
1 parent 730faa8 commit c145c95
Showing 1 changed file with 6 additions and 8 deletions.
14 changes: 6 additions & 8 deletions server/src/test/java/org/elasticsearch/node/NodeTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
*/
package org.elasticsearch.node;

import org.apache.lucene.util.Constants;
import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.bootstrap.BootstrapCheck;
import org.elasticsearch.bootstrap.BootstrapContext;
Expand Down Expand Up @@ -150,7 +149,6 @@ private static Settings.Builder baseSettings() {
}

public void testCloseOnOutstandingTask() throws Exception {
assumeFalse("https://github.com/elastic/elasticsearch/issues/44256", Constants.WINDOWS);
Node node = new MockNode(baseSettings().build(), basePlugins());
node.start();
ThreadPool threadpool = node.injector().getInstance(ThreadPool.class);
Expand All @@ -163,7 +161,7 @@ public void testCloseOnOutstandingTask() throws Exception {
threadRunning.await();
node.close();
shouldRun.set(false);
assertTrue(node.awaitClose(1, TimeUnit.DAYS));
assertTrue(node.awaitClose(10L, TimeUnit.SECONDS));
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/42577")
Expand Down Expand Up @@ -206,7 +204,7 @@ public void testCloseRaceWithTaskExecution() throws Exception {
closeThread.join();

shouldRun.set(false);
assertTrue(node.awaitClose(1, TimeUnit.DAYS));
assertTrue(node.awaitClose(10L, TimeUnit.SECONDS));
}

public void testAwaitCloseTimeoutsOnNonInterruptibleTask() throws Exception {
Expand All @@ -223,7 +221,7 @@ public void testAwaitCloseTimeoutsOnNonInterruptibleTask() throws Exception {
node.close();
assertFalse(node.awaitClose(0, TimeUnit.MILLISECONDS));
shouldRun.set(false);
assertTrue(node.awaitClose(1, TimeUnit.DAYS));
assertTrue(node.awaitClose(10L, TimeUnit.SECONDS));
}

public void testCloseOnInterruptibleTask() throws Exception {
Expand All @@ -247,7 +245,7 @@ public void testCloseOnInterruptibleTask() throws Exception {
});
threadRunning.await();
node.close();
// close should not interrput ongoing tasks
// close should not interrupt ongoing tasks
assertFalse(interrupted.get());
// but awaitClose should
node.awaitClose(0, TimeUnit.SECONDS);
Expand All @@ -266,7 +264,7 @@ public void testCloseOnLeakedIndexReaderReference() throws Exception {
Searcher searcher = shard.acquireSearcher("test");
node.close();

IllegalStateException e = expectThrows(IllegalStateException.class, () -> node.awaitClose(1, TimeUnit.DAYS));
IllegalStateException e = expectThrows(IllegalStateException.class, () -> node.awaitClose(10L, TimeUnit.SECONDS));
searcher.close();
assertThat(e.getMessage(), Matchers.containsString("Something is leaking index readers or store references"));
}
Expand All @@ -282,7 +280,7 @@ public void testCloseOnLeakedStoreReference() throws Exception {
shard.store().incRef();
node.close();

IllegalStateException e = expectThrows(IllegalStateException.class, () -> node.awaitClose(1, TimeUnit.DAYS));
IllegalStateException e = expectThrows(IllegalStateException.class, () -> node.awaitClose(10L, TimeUnit.SECONDS));
shard.store().decRef();
assertThat(e.getMessage(), Matchers.containsString("Something is leaking index readers or store references"));
}
Expand Down

0 comments on commit c145c95

Please sign in to comment.