Skip to content

Ensure tasks with banned parents always get cancelled#90188

Merged
pxsalehi merged 2 commits intoelastic:mainfrom
pxsalehi:ps220921-fix-testCatSegmentsRestCancellation
Sep 22, 2022
Merged

Ensure tasks with banned parents always get cancelled#90188
pxsalehi merged 2 commits intoelastic:mainfrom
pxsalehi:ps220921-fix-testCatSegmentsRestCancellation

Conversation

@pxsalehi
Copy link
Copy Markdown
Member

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, as brought up by @original-brownbear, and it doesn't seem to have a benefit considering the hash code computation.

Closes #88201

@pxsalehi pxsalehi added the :Distributed/Task Management Issues for anything around the Tasks API - both persistent and node level. label Sep 21, 2022
@elasticsearchmachine elasticsearchmachine added v8.5.0 Team:Distributed Meta label for distributed team. labels Sep 21, 2022
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@pxsalehi pxsalehi added v8.4.3 v7.17.7 auto-backport-and-merge >test Issues or PRs that are addressing/adding tests labels Sep 21, 2022
@pxsalehi
Copy link
Copy Markdown
Member Author

@elasticmachine update branch

@csoulios csoulios added v8.6.0 and removed v8.5.0 labels Sep 21, 2022
@pxsalehi pxsalehi added v8.5.1 and removed v8.4.3 labels Sep 22, 2022
Copy link
Copy Markdown
Contributor

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pxsalehi pxsalehi merged commit 9f5dee8 into elastic:main Sep 22, 2022
@pxsalehi
Copy link
Copy Markdown
Member Author

Thank, Armin.

pxsalehi added a commit to pxsalehi/elasticsearch that referenced this pull request Sep 22, 2022
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 elastic#88201
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

💔 Backport failed

Status Branch Result
7.17 Commit could not be cherrypicked due to conflicts
8.5

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 90188

pxsalehi added a commit to pxsalehi/elasticsearch that referenced this pull request Sep 22, 2022
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 elastic#88201
elasticsearchmachine pushed a commit that referenced this pull request Sep 22, 2022
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
elasticsearchmachine pushed a commit that referenced this pull request Sep 22, 2022
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

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@csoulios csoulios added v8.5.0 and removed v8.5.1 labels Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Task Management Issues for anything around the Tasks API - both persistent and node level. Team:Distributed Meta label for distributed team. >test Issues or PRs that are addressing/adding tests v7.17.7 v8.5.0 v8.6.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] IndicesSegmentsRestCancellationIT testCatSegmentsRestCancellation failing

5 participants