Skip to content

Commit 88d05d1

Browse files
authored
Fix race condition when deleting an async search (#53513)
Deleting an async search id can throw a ResourceNotFoundException even if the query was successfully cancelled. We delete the stored response automatically if the query is cancelled so that creates a race with the delete action that also ensures that the task is removed. This change ensures that we ignore missing async search ids in the async search index if they were successfully cancelled. Relates #53360 Relates #49931
1 parent 9ad0597 commit 88d05d1

File tree

3 files changed

+8
-5
lines changed

3 files changed

+8
-5
lines changed

x-pack/plugin/async-search/src/main/java/org/elasticsearch/xpack/search/AsyncSearchIndexService.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -201,13 +201,14 @@ void updateExpirationTime(String docId,
201201
* Deletes the provided <code>searchId</code> from the index if present.
202202
*/
203203
void deleteResponse(AsyncSearchId searchId,
204+
boolean failIfNotFound,
204205
ActionListener<AcknowledgedResponse> listener) {
205206
DeleteRequest request = new DeleteRequest(INDEX).id(searchId.getDocId());
206207
createIndexIfNecessary(
207208
ActionListener.wrap(v -> client.delete(request,
208209
ActionListener.wrap(
209210
resp -> {
210-
if (resp.status() == RestStatus.NOT_FOUND) {
211+
if (resp.status() == RestStatus.NOT_FOUND && failIfNotFound) {
211212
listener.onFailure(new ResourceNotFoundException(searchId.getEncoded()));
212213
} else {
213214
listener.onResponse(new AcknowledgedResponse(true));

x-pack/plugin/async-search/src/main/java/org/elasticsearch/xpack/search/TransportDeleteAsyncSearchAction.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,12 @@ protected void doExecute(Task task, DeleteAsyncSearchAction.Request request, Act
6161
private void cancelTaskAndDeleteResult(AsyncSearchId searchId, ActionListener<AcknowledgedResponse> listener) throws IOException {
6262
AsyncSearchTask task = store.getTask(taskManager, searchId);
6363
if (task != null) {
64-
task.cancelTask(() -> store.deleteResponse(searchId, listener));
64+
task.cancelTask(() -> store.deleteResponse(searchId, false, listener));
6565
} else {
66-
// check if the response can be retrieved by the user (handle security) and then delete.
67-
store.getResponse(searchId, ActionListener.wrap(res -> store.deleteResponse(searchId, listener), listener::onFailure));
66+
// the task is not running anymore so we throw a not found exception if
67+
// the search id is also not present in the index (already deleted) or if the user
68+
// is not allowed to access it.
69+
store.getResponse(searchId, ActionListener.wrap(res -> store.deleteResponse(searchId, true, listener), listener::onFailure));
6870
}
6971
}
7072
}

x-pack/plugin/async-search/src/main/java/org/elasticsearch/xpack/search/TransportSubmitAsyncSearchAction.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ private void onFinalResponse(CancellableTask submitTask,
167167
Runnable nextAction) {
168168
if (submitTask.isCancelled() || searchTask.isCancelled()) {
169169
// the user cancelled the submit so we ensure that there is nothing stored in the response index.
170-
store.deleteResponse(searchTask.getSearchId(), ActionListener.wrap(() -> {
170+
store.deleteResponse(searchTask.getSearchId(), false, ActionListener.wrap(() -> {
171171
taskManager.unregister(searchTask);
172172
nextAction.run();
173173
}));

0 commit comments

Comments
 (0)