Skip to content

Commit 86544aa

Browse files
Safely ensure search contexts are release in ESSingleNodeTestCase (#111153)
The transport action for freeing contexts is forking to GENERIC nowadays and executed in a fire-and-forget manner in production code. When it was running on the direct executor, no busy wait was needed for a single node since the freeing was fully synchronous inside the searches. With forking, the waiting on free tasks does not help because the task is created inside the forked execution for a local node execution, so we need to busy wait like we do for multi-node clusters here too. -> dry up the code and use the same method for both.
1 parent 101775b commit 86544aa

File tree

3 files changed

+15
-12
lines changed

3 files changed

+15
-12
lines changed

test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,7 @@ public void tearDown() throws Exception {
133133
ensureNoInitializingShards();
134134
ensureAllFreeContextActionsAreConsumed();
135135

136-
SearchService searchService = getInstanceFromNode(SearchService.class);
137-
assertThat(searchService.getActiveContexts(), equalTo(0));
138-
assertThat(searchService.getOpenScrollContexts(), equalTo(0));
136+
ensureAllContextsReleased(getInstanceFromNode(SearchService.class));
139137
super.tearDown();
140138
var deleteDataStreamsRequest = new DeleteDataStreamAction.Request("*");
141139
deleteDataStreamsRequest.indicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN);

test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@
115115
import org.elasticsearch.script.Script;
116116
import org.elasticsearch.script.ScriptType;
117117
import org.elasticsearch.search.MockSearchService;
118+
import org.elasticsearch.search.SearchService;
118119
import org.elasticsearch.test.junit.listeners.LoggingListener;
119120
import org.elasticsearch.test.junit.listeners.ReproduceInfoPrinter;
120121
import org.elasticsearch.threadpool.ExecutorBuilder;
@@ -208,6 +209,7 @@
208209
import static org.hamcrest.Matchers.equalTo;
209210
import static org.hamcrest.Matchers.hasItem;
210211
import static org.hamcrest.Matchers.startsWith;
212+
import static org.junit.Assert.assertThat;
211213

212214
/**
213215
* Base testcase for randomized unit testing with Elasticsearch
@@ -2515,4 +2517,15 @@ public static void runInParallel(int numberOfTasks, IntConsumer taskFactory) thr
25152517
throw new AssertionError(e);
25162518
}
25172519
}
2520+
2521+
public static void ensureAllContextsReleased(SearchService searchService) {
2522+
try {
2523+
assertBusy(() -> {
2524+
assertThat(searchService.getActiveContexts(), equalTo(0));
2525+
assertThat(searchService.getOpenScrollContexts(), equalTo(0));
2526+
});
2527+
} catch (Exception e) {
2528+
throw new AssertionError("Failed to verify search contexts", e);
2529+
}
2530+
}
25182531
}

test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2545,15 +2545,7 @@ public void assertRequestsFinished() {
25452545

25462546
private void assertSearchContextsReleased() {
25472547
for (NodeAndClient nodeAndClient : nodes.values()) {
2548-
SearchService searchService = getInstance(SearchService.class, nodeAndClient.name);
2549-
try {
2550-
assertBusy(() -> {
2551-
assertThat(searchService.getActiveContexts(), equalTo(0));
2552-
assertThat(searchService.getOpenScrollContexts(), equalTo(0));
2553-
});
2554-
} catch (Exception e) {
2555-
throw new AssertionError("Failed to verify search contexts", e);
2556-
}
2548+
ESTestCase.ensureAllContextsReleased(getInstance(SearchService.class, nodeAndClient.name));
25572549
}
25582550
}
25592551

0 commit comments

Comments
 (0)