This is an automated email from the ASF dual-hosted git repository.
broustant 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 c276338a7b0 SOLR-17716: Handle interrupted exception in
SolrCores.waitAddPendingCoreOps. (#3283)
c276338a7b0 is described below
commit c276338a7b06588b238d638769037eec6feb590d
Author: Bruno Roustant <[email protected]>
AuthorDate: Tue Mar 25 08:49:59 2025 +0100
SOLR-17716: Handle interrupted exception in
SolrCores.waitAddPendingCoreOps. (#3283)
---
solr/CHANGES.txt | 2 ++
.../java/org/apache/solr/core/CoreContainer.java | 14 ++++++++-----
.../src/java/org/apache/solr/core/SolrCores.java | 23 +++++++++++++---------
3 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 229d00ef2d9..3eb7614ba67 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -103,6 +103,8 @@ Other Changes
* SOLR-17518: Deprecate UpdateRequest.getXml() and replace it with
XMLRequestWriter. (Pierre Salagnac)
+* SOLR-17716: Handle interrupted exception in SolrCores.waitAddPendingCoreOps.
(Bruno Roustant)
+
================== 9.8.1 ==================
Bug Fixes
---------------------
diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
index 7b0ef6e0ad0..b78924e8cba 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -1083,19 +1083,23 @@ public class CoreContainer {
coreLoadExecutor.execute(
() -> {
SolrCore core;
+ boolean pendingCoreOpAdded = false;
try {
if (zkSys.getZkController() != null) {
zkSys.getZkController().throwErrorIfReplicaReplaced(cd);
}
MDCLoggingContext.setCoreDescriptor(this, cd);
solrCores.waitAddPendingCoreOps(cd.getName());
+ pendingCoreOpAdded = true;
core = createFromDescriptor(cd, false, false);
} catch (Exception e) {
log.error("SolrCore failed to load on startup", e);
MDCLoggingContext.clear();
return;
} finally {
- solrCores.removeFromPendingOps(cd.getName());
+ if (pendingCoreOpAdded) {
+ solrCores.removeFromPendingOps(cd.getName());
+ }
if (asyncSolrCoreLoad) {
solrCores.markCoreAsNotLoading(cd);
}
@@ -1619,8 +1623,8 @@ public class CoreContainer {
coresLocator.create(this, cd);
SolrCore core;
+ solrCores.waitAddPendingCoreOps(cd.getName());
try {
- solrCores.waitAddPendingCoreOps(cd.getName());
core = createFromDescriptor(cd, true, newCollection);
// Write out the current core properties in case anything changed
when the core was
// created
@@ -2056,8 +2060,8 @@ public class CoreContainer {
CoreDescriptor cd = reloadCoreDescriptor(core.getCoreDescriptor());
solrCores.addCoreDescriptor(cd);
boolean success = false;
+ solrCores.waitAddPendingCoreOps(cd.getName());
try {
- solrCores.waitAddPendingCoreOps(cd.getName());
ConfigSet coreConfig = coreConfigService.loadConfigSet(cd);
if (log.isInfoEnabled()) {
log.info(
@@ -2116,8 +2120,8 @@ public class CoreContainer {
if (coreId != null) return; // yeah, this core is already
reloaded/unloaded return right away
CoreLoadFailure clf = coreInitFailures.get(name);
if (clf != null) {
+ solrCores.waitAddPendingCoreOps(clf.cd.getName());
try {
- solrCores.waitAddPendingCoreOps(clf.cd.getName());
createFromDescriptor(clf.cd, true, false);
} finally {
solrCores.removeFromPendingOps(clf.cd.getName());
@@ -2160,8 +2164,8 @@ public class CoreContainer {
*/
public void unload(
String name, boolean deleteIndexDir, boolean deleteDataDir, boolean
deleteInstanceDir) {
+ solrCores.waitAddPendingCoreOps(name);
try {
- solrCores.waitAddPendingCoreOps(name);
unloadWithoutCoreOp(name, deleteIndexDir, deleteDataDir,
deleteInstanceDir);
} finally {
solrCores.removeFromPendingOps(name);
diff --git a/solr/core/src/java/org/apache/solr/core/SolrCores.java
b/solr/core/src/java/org/apache/solr/core/SolrCores.java
index 4ea3974f0f8..39b1f68b155 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCores.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCores.java
@@ -346,26 +346,31 @@ public class SolrCores {
}
}
}
- if (container.isShutDown()) return null; // Just stop already.
+ if (container.isShutDown()) {
+ // Just stop already.
+ // Seems best to throw a SolrException if shutting down, because
returning any value,
+ // including null, would mean the waiting is complete.
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
"Server is shutting down");
+ }
if (pending) {
try {
modifyLock.wait();
} catch (InterruptedException e) {
- return null; // Seems best not to do anything at all if the thread
is interrupted
+ // Seems best to throw a SolrException if interrupted, because
returning any value,
+ // including null, would mean the waiting is complete.
+ Thread.currentThread().interrupt();
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
}
}
} while (pending);
// We _really_ need to do this within the synchronized block!
- if (!container.isShutDown()) {
- if (!pendingCoreOps.add(name)) {
- log.warn("Replaced an entry in pendingCoreOps {}, we should not be
doing this", name);
- }
- // we might have been _unloading_ the core, so return the core if it
was loaded.
- return getCoreFromAnyList(name, false);
+ if (!pendingCoreOps.add(name)) {
+ log.warn("Replaced an entry in pendingCoreOps {}, we should not be
doing this", name);
}
+ // we might have been _unloading_ the core, so return the core if it was
loaded.
+ return getCoreFromAnyList(name, false);
}
- return null;
}
// We should always be removing the first thing in the list with our name!
The idea here is to NOT
