Skip to content

Commit 638e2c4

Browse files
committed
Fix race in clear scroll (#31259)
Here is the problem: if two threads are racing and one hits a failure freeing a context and the other succeeded, we can expose the value of the has failure marker to the succeeding thread before the failing thread has had a chance to set the failure marker. This is a problem if the failing thread counted down the expected number of operations, then be put to sleep by a gentle lullaby from the OS, and then the other thread could count down to zero. Since the failing thread did not get to set the failure marker, the succeeding thread would respond that the clear scroll succeeded and that makes that thread a liar. This commit addresses by first setting the failure marker before we potentially expose its value to another thread.
1 parent 19f5284 commit 638e2c4

File tree

1 file changed

+5
-2
lines changed

1 file changed

+5
-2
lines changed

core/src/main/java/org/elasticsearch/action/search/ClearScrollController.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,13 @@ private void onFreedContext(boolean freed) {
134134

135135
private void onFailedFreedContext(Throwable e, DiscoveryNode node) {
136136
logger.warn((Supplier<?>) () -> new ParameterizedMessage("Clear SC failed on node[{}]", node), e);
137+
/*
138+
* We have to set the failure marker before we count down otherwise we can expose the failure marker before we have set it to a
139+
* racing thread successfully freeing a context. This would lead to that thread responding that the clear scroll succeeded.
140+
*/
141+
hasFailed.set(true);
137142
if (expectedOps.countDown()) {
138143
listener.onResponse(new ClearScrollResponse(false, freedSearchContexts.get()));
139-
} else {
140-
hasFailed.set(true);
141144
}
142145
}
143146
}

0 commit comments

Comments
 (0)