dsmiley commented on a change in pull request #180:
URL: https://github.com/apache/solr/pull/180#discussion_r675533368
##########
File path: solr/CHANGES.txt
##########
@@ -396,7 +396,8 @@ Bug Fixes
Other Changes
---------------------
-(No changes)
+* SOLR-15486: During node shutdown pausing of updates and waiting for
in-flight update requests to finish
+ before closing cores is no longer SolrCloud specific. (Christine Poerschke,
David Smiley)
Review comment:
LOL all I did was agree with the idea. But fine; I'm reviewing because
I want to be deserving of my name being there ;-)
##########
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##########
@@ -1008,6 +1008,29 @@ public boolean isShutDown() {
return isShutDown;
}
+ /**
Review comment:
really minor but I prefer that methods, especially private ones,
_follow_ its callers so that you can follow the code reading down without
bouncing back up
##########
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##########
@@ -1033,27 +1056,10 @@ public void shutdown() {
if (isZooKeeperAware()) {
cancelCoreRecoveries();
zkSys.zkController.preClose();
- /*
- * Pause updates for all cores on this node and wait for all in-flight
update requests to finish.
- * Here, we (slightly) delay leader election so that in-flight update
requests succeed and we can preserve consistency.
- *
- * Jetty already allows a grace period for in-flight requests to
complete and our solr cores, searchers etc
- * are reference counted to allow for graceful shutdown. So we don't
worry about any other kind of requests.
- *
- * We do not need to unpause ever because the node is being shut down.
- */
- getCores().parallelStream().forEach(solrCore -> {
- SolrCoreState solrCoreState = solrCore.getSolrCoreState();
- try {
- solrCoreState.pauseUpdatesAndAwaitInflightRequests();
- } catch (TimeoutException e) {
- log.warn("Timed out waiting for in-flight update requests to
complete for core: {}", solrCore.getName());
- } catch (InterruptedException e) {
- log.warn("Interrupted while waiting for in-flight update requests
to complete for core: {}", solrCore.getName());
- Thread.currentThread().interrupt();
- }
- });
+ pauseUpdatesAndAwaitInflightRequests();
Review comment:
Couldn't this be moved out of the if/else around SolrCloud and thus call
out once?
--
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]
