diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index f64f552768f..035a1f6af3f 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -188,6 +188,9 @@ Bug Fixes * SOLR-8695: Ensure ZK watchers are not triggering our watch logic on connection events and make this handling more consistent. (Scott Blum via Mark Miller) +* SOLR-8633: DistributedUpdateProcess processCommit/deleteByQuery call finish on DUP and + SolrCmdDistributor, which violates the lifecycle and can cause bugs. (hossman via Mark Miller) + Optimizations ---------------------- * SOLR-7876: Speed up queries and operations that use many terms when timeAllowed has not been diff --git a/solr/core/src/java/org/apache/solr/update/SolrCmdDistributor.java b/solr/core/src/java/org/apache/solr/update/SolrCmdDistributor.java index 0244b0efc0d..d9b6478119f 100644 --- a/solr/core/src/java/org/apache/solr/update/SolrCmdDistributor.java +++ b/solr/core/src/java/org/apache/solr/update/SolrCmdDistributor.java @@ -55,7 +55,8 @@ public class SolrCmdDistributor { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private StreamingSolrClients clients; - + private boolean finished = false; // see finish() + private int retryPause = 500; private int maxRetriesOnForward = MAX_RETRIES_ON_FORWARD; @@ -86,6 +87,9 @@ public class SolrCmdDistributor { public void finish() { try { + assert ! finished : "lifecycle sanity check"; + finished = true; + blockAndDoRetries(); } finally { clients.shutdown(); @@ -227,7 +231,7 @@ public class SolrCmdDistributor { } - private void blockAndDoRetries() { + public void blockAndDoRetries() { clients.blockUntilFinished(); // wait for any async commits to complete diff --git a/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java b/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java index d0e72db79bf..8815c3fca5c 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java +++ b/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java @@ -221,6 +221,9 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor { public static final String COMMIT_END_POINT = "commit_end_point"; public static final String LOG_REPLAY = "log_replay"; + + // used to assert we don't call finish more than once, see finish() + private boolean finished = false; private final SolrQueryRequest req; private final SolrQueryResponse rsp; @@ -1373,7 +1376,7 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor { } if (someReplicas) { - cmdDistrib.finish(); + cmdDistrib.blockAndDoRetries(); } } @@ -1618,7 +1621,7 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor { zkController.getBaseUrl(), req.getCore().getName())); if (nodes != null) { cmdDistrib.distribCommit(cmd, nodes, params); - finish(); + cmdDistrib.blockAndDoRetries(); } } } @@ -1645,6 +1648,9 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor { @Override public void finish() throws IOException { + assert ! finished : "lifecycle sanity check"; + finished = true; + if (zkEnabled) doFinish(); if (next != null && nodes == null) next.finish();