From 6ee69ba6aabddddf34d1b64cdec28aed5fc68103 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Sun, 1 Mar 2026 21:19:33 -0500 Subject: [PATCH] SOLR-18142: Fix CloudSolrClient cache state refresh; regression. Recent improvements induced a new race condition caught by a flaky test. State sometimes isn't refreshed. --- changelog/unreleased/SOLR-18142.yml | 7 +++ .../client/solrj/impl/CloudSolrClient.java | 62 ++++++++----------- 2 files changed, 34 insertions(+), 35 deletions(-) create mode 100644 changelog/unreleased/SOLR-18142.yml diff --git a/changelog/unreleased/SOLR-18142.yml b/changelog/unreleased/SOLR-18142.yml new file mode 100644 index 000000000000..b8b20bfea78e --- /dev/null +++ b/changelog/unreleased/SOLR-18142.yml @@ -0,0 +1,7 @@ +title: CloudSolrClient- fixed state refresh race; didn't refresh. Regression from 9.10.1/10.0. +type: fixed +authors: + - name: David Smiley +links: + - name: SOLR-18142 + url: https://issues.apache.org/jira/browse/SOLR-18142 diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index 1a41044b7939..df7034acf832 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -105,7 +105,7 @@ public abstract class CloudSolrClient extends SolrClient { private final boolean directUpdatesToLeadersOnly; private final RequestReplicaListTransformerGenerator requestRLTGenerator; private final boolean parallelUpdates; - private ExecutorService threadPool = + private final ExecutorService threadPool = ExecutorUtil.newMDCAwareCachedThreadPool( new SolrNamedThreadFactory("CloudSolrClient ThreadPool")); @@ -642,9 +642,8 @@ protected boolean wasCommError(Throwable t) { public void close() { closed = true; collectionRefreshes.clear(); - if (this.threadPool != null && !ExecutorUtil.isShutdown(this.threadPool)) { + if (!ExecutorUtil.isShutdown(this.threadPool)) { ExecutorUtil.shutdownAndAwaitTermination(this.threadPool); - this.threadPool = null; } } @@ -1658,41 +1657,34 @@ protected DocCollection getDocCollection(String collection, Integer expectedVers } private CompletableFuture triggerCollectionRefresh(String collection) { - if (closed) { - ExpiringCachedDocCollection cacheEntry = collectionStateCache.peek(collection); - DocCollection cached = cacheEntry == null ? null : cacheEntry.cached; - return CompletableFuture.completedFuture(cached); - } - return collectionRefreshes.computeIfAbsent( + return collectionRefreshes.compute( collection, - key -> { - ExecutorService executor = threadPool; - CompletableFuture future; - if (executor == null || ExecutorUtil.isShutdown(executor)) { - future = new CompletableFuture<>(); - try { - future.complete(loadDocCollection(key)); - } catch (Throwable t) { - future.completeExceptionally(t); - } + (key, existingFuture) -> { + // A refresh is still in progress; return it. + if (existingFuture != null && !existingFuture.isDone()) { + return existingFuture; + } + // No refresh is in-progress, so trigger it. + + if (ExecutorUtil.isShutdown(threadPool)) { + assert closed; // see close() for the sequence + ExpiringCachedDocCollection cacheEntry = collectionStateCache.peek(key); + DocCollection cached = cacheEntry == null ? null : cacheEntry.cached; + return CompletableFuture.completedFuture(cached); } else { - future = - CompletableFuture.supplyAsync( - () -> { - stateRefreshSemaphore.acquireUninterruptibly(); - try { - return loadDocCollection(key); - } finally { - stateRefreshSemaphore.release(); - } - }, - executor); + return CompletableFuture.supplyAsync( + () -> { + stateRefreshSemaphore.acquireUninterruptibly(); + try { + return loadDocCollection(key); + } finally { + stateRefreshSemaphore.release(); + // Remove the entry in case of many collections + collectionRefreshes.remove(key); + } + }, + threadPool); } - future.whenCompleteAsync( - (result, error) -> { - collectionRefreshes.remove(key, future); - }); - return future; }); }