psalagnac commented on code in PR #3283:
URL: https://github.com/apache/solr/pull/3283#discussion_r2010386539
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -2323,16 +2323,14 @@ public SolrCore getCore(String name, UUID id) {
// TestConfigSetsAPI and TestLazyCores
if (desc == null || zkSys.getZkController() != null) return null;
- // This will put an entry in pending core ops if the core isn't loaded.
Here's where moving the
- // waitAddPendingCoreOps to createFromDescriptor would introduce a race
condition.
- core = solrCores.waitAddPendingCoreOps(name);
-
- if (isShutDown) {
- // We're quitting, so stop. This needs to be after the wait above since
we may come off the
- // wait as a consequence of shutting down.
- return null;
- }
try {
+ // This will put an entry in pending core ops if the core isn't loaded.
Here's where moving
+ // the waitAddPendingCoreOps to createFromDescriptor would introduce a
race condition.
+ core = solrCores.waitAddPendingCoreOps(name);
Review Comment:
I think it is fine to throw an exception as long as we don't remove the lock
in case of exception. I'm good with your change in `SolrCores` (the other file).
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
