Expose the logic to cancel task when the rest channel is closed #51423
Conversation
This commit moves the logic that cancels search requests when the rest channel is closed to a generic client that can be used by other APIs. This will be useful for any rest action that wants to cancel the execution of a task if the underlying rest channel is closed by the client before completion. Relates elastic#49931 Relates elastic#50990 Relates elastic#50990
jimczi
added
>non-issue
:Distributed Coordination/Task Management
|
Pinging @elastic/es-distributed (:Distributed/Task Management) |
|
@elasticmachine run elasticsearch-ci/2 |
There was a problem hiding this comment.
This looks good. I think my main concern is that if the assertion in EsIntegTestCase fails, it'll be very hard to dig as it doesn't give any information about what leaked. But this looks like a phard problem and is also a pre-existing issue so let's not let it be in the way.
| } | ||
| toCancel.stream().forEach(taskId -> cancelTask(taskId)); |
There was a problem hiding this comment.
nit: this might be abusive streams a bit, let's do a for loop?
| } | ||
| public void onResponse(Void aVoid) { | ||
| final List<TaskId> toCancel; | ||
| synchronized (this) { |
There was a problem hiding this comment.
we don't need to protect httpChannels so we could move the synchronized keyword two lines below?
|
|
||
| CloseListener(Client client) { | ||
| this.client = client; | ||
| CloseListener() { | ||
| } | ||
|
|
||
| int getNumTasks() { |
There was a problem hiding this comment.
I think it needs to be synchronized?
| final List<TaskId> toCancel; | ||
| synchronized (this) { | ||
| // when the channel gets closed it won't be reused: we can remove it from the map and forget about it. | ||
| CloseListener closeListener = httpChannels.remove(channel.get()); |
There was a problem hiding this comment.
should we assert that channel.get() is not null?
|
Thanks @jpountz , I pushed a commit to address your comments.
I left this out of scope for now but I'll think about a way to add more informations in a follow up. |
This commit moves the logic that cancels search requests when the rest channel is closed to a generic client that can be used by other APIs. This will be useful for any rest action that wants to cancel the execution of a task if the underlying rest channel is closed by the client before completion. Relates #49931 Relates #50990 Relates #50990
This commit moves the logic that cancels search requests when the rest channel is closed
to a generic client that can be used by other APIs. This will be useful for any rest action
that wants to cancel the execution of a task if the underlying rest channel is closed by the
client before completion.
Relates #49931
Relates #50990
Relates #49581