You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ho...@apache.org on 2025/03/19 19:21:14 UTC
(solr) branch branch_9x updated: SOLR-17709: Fix race condition when checking distrib async cmd status (#3268)
This is an automated email from the ASF dual-hosted git repository.
houston pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new e51dd47d884 SOLR-17709: Fix race condition when checking distrib async cmd status (#3268)
e51dd47d884 is described below
commit e51dd47d88445259d57fc63dc655aecaafecf265
Author: Houston Putman <ho...@apache.org>
AuthorDate: Wed Mar 19 14:16:32 2025 -0500
SOLR-17709: Fix race condition when checking distrib async cmd status (#3268)
(cherry picked from commit d0d4f280b6410d8996fa998620d9b6661848d1f0)
---
solr/CHANGES.txt | 2 ++
.../solr/cloud/DistributedApiAsyncTracker.java | 28 +++++++++++++++-------
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 942196f4982..239325e1a96 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -59,6 +59,8 @@ Bug Fixes
* SOLR-17699: Fixed performance regression since 9.0 with "frange" queries embedded in other queries.
May also affect some numeric range queries when the minimum is negative. (David Smiley)
+* SOLR-17709: Fix race condition when checking distrib async cmd status (Houston Putman)
+
Dependency Upgrades
---------------------
* SOLR-17471: Upgrade Lucene to 9.12.1. (Pierre Salagnac, Christine Poerschke)
diff --git a/solr/core/src/java/org/apache/solr/cloud/DistributedApiAsyncTracker.java b/solr/core/src/java/org/apache/solr/cloud/DistributedApiAsyncTracker.java
index 8ebdb2ae287..c41244aace9 100644
--- a/solr/core/src/java/org/apache/solr/cloud/DistributedApiAsyncTracker.java
+++ b/solr/core/src/java/org/apache/solr/cloud/DistributedApiAsyncTracker.java
@@ -221,6 +221,26 @@ public class DistributedApiAsyncTracker {
return new Pair<>(RequestStatusState.NOT_FOUND, null);
}
+ /*
+ * First check the inFlightAsyncTasks, then the trackedAsyncTasks.
+ * This is the reverse of "cancelAsyncId()" and "setTaskCompleted()",
+ * because a race condition can occur where this is called between the
+ * "trackedAsyncTasks" and "inFlightAsyncTasks" modifications in those methods.
+ *
+ * Therefore, here we will start with inFlightAsyncTasks, and if it hasn't been updated yet,
+ * we return the "old-ish" state, and the client can try again. If the inFlightAsyncTasks has
+ * been updated (removed), then we know to try the trackedAsyncTasks which must have been updated
+ * at that point.
+ */
+
+ // Now dealing with the middle column "persistent=null or failed OverseerSolrResponse"
+ InFlightJobs.State ephemeralState = inFlightAsyncTasks.getInFlightState(asyncId);
+ if (ephemeralState == InFlightJobs.State.SUBMITTED) {
+ return new Pair<>(RequestStatusState.SUBMITTED, null);
+ } else if (ephemeralState == InFlightJobs.State.RUNNING) {
+ return new Pair<>(RequestStatusState.RUNNING, null);
+ }
+
byte[] data = trackedAsyncTasks.get(asyncId);
OverseerSolrResponse response =
data != null ? OverseerSolrResponseSerializer.deserialize(data) : null;
@@ -233,14 +253,6 @@ public class DistributedApiAsyncTracker {
return new Pair<>(RequestStatusState.COMPLETED, response);
}
- // Now dealing with the middle column "persistent=null or failed OverseerSolrResponse"
- InFlightJobs.State ephemeralState = inFlightAsyncTasks.getInFlightState(asyncId);
- if (ephemeralState == InFlightJobs.State.SUBMITTED) {
- return new Pair<>(RequestStatusState.SUBMITTED, null);
- } else if (ephemeralState == InFlightJobs.State.RUNNING) {
- return new Pair<>(RequestStatusState.RUNNING, null);
- }
-
// The task has failed, but there are two options: if response is null, it has failed because
// the node on which it was running has crashed. If it is not null, it has failed because the
// execution has failed. Because caller expects a non null response in any case, let's make up