bruno-roustant commented on code in PR #3283:
URL: https://github.com/apache/solr/pull/3283#discussion_r2010382686
##########
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:
You're right. I wanted to have a consistent way to try-finally between all
the calls to waitAddPendingCoreOps.
That means that changing waitAddPendingCoreOps to throw an exception is
either not appropriate, or needs more care with the other callers. And btw, the
other callers may have currently an issue if an exception is thrown inside
waitAddPendingCoreOps.
--
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]
