Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tasks submitted to shutting down fixed executors are sometimes neither executed nor rejected #104580

Closed
DaveCTurner opened this issue Jan 19, 2024 · 2 comments · Fixed by #104581
Labels
>bug :Core/Infra/Core Core issues without another label Team:Core/Infra Meta label for core/infra team

Comments

@DaveCTurner
Copy link
Contributor

DaveCTurner commented Jan 19, 2024

AFAICT it seems that fixed executors (with bounded queues) sometimes just drop tasks on the floor after shutdown, rather than rejecting them as one might expect. For a demonstration test case, see testFixedBoundedRejectOnShutdown added (albeit muted) in #104581 - this test fails for me whenever it submits a force-executed task.

@DaveCTurner DaveCTurner added >bug :Core/Infra/Core Core issues without another label labels Jan 19, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Jan 19, 2024
@DaveCTurner
Copy link
Contributor Author

Ah I think I see the problem, we're just blindly putting the task into the queue even if nobody is consuming it any more:

We should be doing something similar to how the ForceQueuePolicy handles it:

put(executor, task);
// we need to check again the executor state as it might have been concurrently shut down; in this case
// the executor's workers are shutting down and might have already picked up the task for execution.
if (executor.isShutdown() && executor.remove(task)) {
reject(executor, task);
}

DaveCTurner added a commit that referenced this issue Jan 22, 2024
Today if you submit a force-executing task to a fixed executor with an
unbounded queue after the executor has been shut down then the
`EsAbortPolicy` will incorrectly throw an `IllegalStateException` which
ends up tripping an assertion. This is a legitimate thing to happen, so
this commit introduces a different rejection handler to deal with it. It
also introduces a check for a shut-down bounded-queue executor on
rejection to make sure we don't force the task onto a queue that's never
going to be processed.

Closes #104580
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Core Core issues without another label Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants