markrmiller commented on code in PR #3851:
URL: https://github.com/apache/solr/pull/3851#discussion_r2525737595
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java:
##########
@@ -117,7 +119,12 @@ public abstract class CloudSolrClient extends SolrClient {
// UpdateParams.ROLLBACK
);
- protected volatile Object[] locks = objectList(3);
+ private final ConcurrentHashMap<String, CompletableFuture<DocCollection>>
collectionRefreshes =
+ new ConcurrentHashMap<>();
+ private final Object stateRefreshExecutorLock = new Object();
+ private volatile int stateRefreshParallelism =
DEFAULT_STATE_REFRESH_PARALLELISM;
+ private volatile ExecutorService stateRefreshExecutor;
Review Comment:
I hate to see comments like this on this project. The comment correctly
flags that adding another ExecutorService is questionable, but it’s mostly
expressing frustration rather than explaining the risk, and it doesn’t do much
to invite collaboration. It doesn’t say why another executor is a problem
(extra threads, lifecycle/timeout divergence, duplicated logic), and it only
implicitly suggests requestAsync as the alternative. A better version would
explicitly state the operational and maintenance concerns, clearly propose
reusing HttpSolrClientBase.requestAsync (or ask why it can’t be reused), and
frame this as a question or suggestion instead of a verdict, in a neutral tone
that focuses on the design rather than how disappointing it feels.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
