Skip to content

Commit

Permalink
Ensure tasks with banned parents always get cancelled (#90188) (#90248)
Browse files Browse the repository at this point in the history
The check used to entirely skip parent lookup relies on
ConcurrentHashMap#isEmpty() which could return inconsistent results, and
potentially skip the cancellation of a task with a banned parent upon
registration, and it doesn't seem to have a benefit considering the hash
code computation.

Closes #88201
  • Loading branch information
pxsalehi authored Sep 22, 2022
1 parent 08aa7c0 commit 83c19ae
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@
import org.apache.http.client.methods.HttpGet;
import org.elasticsearch.action.admin.indices.segments.IndicesSegmentsAction;
import org.elasticsearch.client.Request;
import org.elasticsearch.test.junit.annotations.TestLogging;

@TestLogging(value = "org.elasticsearch.tasks.TaskManager:TRACE,org.elasticsearch.test.TaskAssertions:TRACE", reason = "debugging")
public class IndicesSegmentsRestCancellationIT extends BlockedSearcherRestCancellationTestCase {
public void testIndicesSegmentsRestCancellation() throws Exception {
runTest(new Request(HttpGet.METHOD_NAME, "/_segments"), IndicesSegmentsAction.NAME);
Expand Down
5 changes: 2 additions & 3 deletions server/src/main/java/org/elasticsearch/tasks/TaskManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,8 @@ private void registerCancellableTask(Task task) {
CancellableTask cancellableTask = (CancellableTask) task;
CancellableTaskHolder holder = new CancellableTaskHolder(cancellableTask);
cancellableTasks.put(task, holder);
// Check if this task was banned before we start it. The empty check is used to avoid
// computing the hash code of the parent taskId as most of the time bannedParents is empty.
if (task.getParentTaskId().isSet() && bannedParents.isEmpty() == false) {
// Check if this task was banned before we start it.
if (task.getParentTaskId().isSet()) {
final Ban ban = bannedParents.get(task.getParentTaskId());
if (ban != null) {
try {
Expand Down

0 comments on commit 83c19ae

Please sign in to comment.