From c145c9577e5049d64c2302e345495ff7131f2132 Mon Sep 17 00:00:00 2001 From: jaymode Date: Fri, 15 Nov 2019 14:07:04 -0700 Subject: [PATCH] Stop NodeTests from timing out in certain cases 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 #44256 Closes #42350 --- .../java/org/elasticsearch/node/NodeTests.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/node/NodeTests.java b/server/src/test/java/org/elasticsearch/node/NodeTests.java index 5e06e557975ea..bd8b5edfb0b5e 100644 --- a/server/src/test/java/org/elasticsearch/node/NodeTests.java +++ b/server/src/test/java/org/elasticsearch/node/NodeTests.java @@ -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; @@ -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); @@ -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") @@ -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 { @@ -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 { @@ -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); @@ -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")); } @@ -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")); }