[lucene-solr] branch reference_impl_dev updated: @1382 A little shutdown/close work, use correct name prop for replica when getting props direct from leader znode entry.

[ma...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

markrmiller pushed a commit to branch reference_impl_dev
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/reference_impl_dev by this push:
     new 692a04a  @1382 A little shutdown/close work, use correct name prop for replica when getting props direct from leader znode entry.
692a04a is described below

commit 692a04a6c90a9a15f0fac607a1431fff5f40cb3a
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Wed Feb 24 15:05:40 2021 -0600

    @1382 A little shutdown/close work, use correct name prop for replica when getting props direct from leader znode entry.
    
    Took 1 hour 26 minutes
---
 gradle/testing/defaults-tests.gradle               |  2 +-
 .../legacy/expression/LegacyExpressionTest.java    |  1 +
 .../apache/solr/cloud/OverseerElectionContext.java |  2 +-
 .../java/org/apache/solr/cloud/ZkController.java   |  4 +--
 .../org/apache/solr/metrics/SolrMetricManager.java | 29 +++++++++++-----------
 .../java/org/apache/solr/search/CaffeineCache.java | 21 ++++++----------
 .../apache/solr/update/DirectUpdateHandler2.java   |  3 +++
 .../java/org/apache/solr/update/UpdateHandler.java |  1 +
 .../org/apache/solr/update/UpdateShardHandler.java |  6 +++++
 .../processor/DistributedUpdateProcessor.java      |  2 +-
 .../processor/DistributedZkUpdateProcessor.java    |  6 ++++-
 .../apache/solr/cloud/DocValuesNotIndexedTest.java |  1 +
 .../java/org/apache/solr/common/cloud/Replica.java | 13 ++++++----
 .../apache/solr/common/cloud/ZkStateReader.java    | 10 +++++---
 .../src/java/org/apache/solr/SolrTestCase.java     |  5 ----
 15 files changed, 57 insertions(+), 49 deletions(-)

diff --git a/gradle/testing/defaults-tests.gradle b/gradle/testing/defaults-tests.gradle
index 30e3ebf..6303f33 100644
--- a/gradle/testing/defaults-tests.gradle
+++ b/gradle/testing/defaults-tests.gradle
@@ -104,7 +104,7 @@ allprojects {
       workingDir testsCwd
       useJUnit()
 
-      minHeapSize = propertyOrDefault("tests.minheapsize", "512m")
+      minHeapSize = propertyOrDefault("tests.minheapsize", "256m")
       maxHeapSize = propertyOrDefault("tests.heapsize", "512m")
 
       jvmArgs Commandline.translateCommandline(propertyOrDefault(
diff --git a/solr/contrib/analytics/src/test/org/apache/solr/analytics/legacy/expression/LegacyExpressionTest.java b/solr/contrib/analytics/src/test/org/apache/solr/analytics/legacy/expression/LegacyExpressionTest.java
index 4b5f99b..e153a55 100644
--- a/solr/contrib/analytics/src/test/org/apache/solr/analytics/legacy/expression/LegacyExpressionTest.java
+++ b/solr/contrib/analytics/src/test/org/apache/solr/analytics/legacy/expression/LegacyExpressionTest.java
@@ -26,6 +26,7 @@ import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+@LuceneTestCase.Nightly
 public class LegacyExpressionTest extends LegacyAbstractAnalyticsTest {
   private static final String fileName = "expressions.txt";
 
diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerElectionContext.java b/solr/core/src/java/org/apache/solr/cloud/OverseerElectionContext.java
index 481bdc9..e931271 100644
--- a/solr/core/src/java/org/apache/solr/cloud/OverseerElectionContext.java
+++ b/solr/core/src/java/org/apache/solr/cloud/OverseerElectionContext.java
@@ -39,7 +39,7 @@ final class OverseerElectionContext extends ShardLeaderElectionContextBase {
   private final Overseer overseer;
 
   public OverseerElectionContext(final String zkNodeName, SolrZkClient zkClient, Overseer overseer) {
-    super(zkNodeName, Overseer.OVERSEER_ELECT, Overseer.OVERSEER_ELECT + "/leader", new Replica("overseer:" + overseer.getZkController().getNodeName(), getIDMap(zkNodeName, overseer), null, null, overseer.getZkStateReader()), null, zkClient);
+    super(zkNodeName, Overseer.OVERSEER_ELECT, Overseer.OVERSEER_ELECT + "/leader", new Replica("overseer:" + overseer.getZkController().getNodeName(), getIDMap(zkNodeName, overseer), "overseer", "overseer", overseer.getZkStateReader()), null, zkClient);
     this.overseer = overseer;
     this.zkClient = zkClient;
   }
diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
index cf32f7f..7d271a2 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
@@ -1535,8 +1535,8 @@ public class ZkController implements Closeable, Runnable {
 
       byte[] data = zkClient.getData(ZkStateReader.getShardLeadersPath(collection, slice), null, null);
       ZkCoreNodeProps leaderProps = new ZkCoreNodeProps(ZkNodeProps.load(data));
-      // MRM TODO: - right key for leader name?
-      return new Replica(leaderProps.getNodeProps().getStr("name"), leaderProps.getNodeProps().getProperties(), collection, slice, zkStateReader);
+
+      return new Replica(leaderProps.getNodeProps().getStr(CORE_NAME_PROP), leaderProps.getNodeProps().getProperties(), collection, slice, zkStateReader);
 
     } catch (Exception e) {
       SolrZkClient.checkInterrupted(e);
diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
index 7260d74..087b650 100644
--- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
+++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
@@ -31,7 +31,6 @@ import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.regex.Pattern;
@@ -769,26 +768,26 @@ public class SolrMetricManager {
     registerMetric(context, registry, new GaugeWrapper(gauge, tag), force, metricName, metricPath);
   }
 
-  public int unregisterGauges(String registryName, String tagSegment) {
+  public void unregisterGauges(String registryName, String tagSegment) {
     if (tagSegment == null) {
-      return 0;
+      return;
     }
     MetricRegistry registry = registry(registryName);
-    if (registry == null) return 0;
-    AtomicInteger removed = new AtomicInteger();
-    registry.removeMatching((name, metric) -> {
-      if (metric instanceof GaugeWrapper) {
-        GaugeWrapper wrapper = (GaugeWrapper) metric;
-        boolean toRemove = wrapper.getTag().contains(tagSegment);
-        if (toRemove) {
-          removed.incrementAndGet();
+    if (registry == null) return;
+
+    ParWork.getRootSharedExecutor().submit(() -> {
+      registry.removeMatching((name, metric) -> {
+        if (metric instanceof GaugeWrapper) {
+          GaugeWrapper wrapper = (GaugeWrapper) metric;
+          boolean toRemove = wrapper.getTag().contains(tagSegment);
+          return toRemove;
         }
-        return toRemove;
-      }
-      return false;
+        return false;
 
+      });
     });
-    return removed.get();
+
+    return;
   }
 
   /**
diff --git a/solr/core/src/java/org/apache/solr/search/CaffeineCache.java b/solr/core/src/java/org/apache/solr/search/CaffeineCache.java
index 225954b..5243e2c 100644
--- a/solr/core/src/java/org/apache/solr/search/CaffeineCache.java
+++ b/solr/core/src/java/org/apache/solr/search/CaffeineCache.java
@@ -25,6 +25,7 @@ import com.github.benmanes.caffeine.cache.stats.CacheStats;
 import com.google.common.annotations.VisibleForTesting;
 import org.apache.lucene.util.Accountable;
 import org.apache.lucene.util.RamUsageEstimator;
+import org.apache.solr.common.ParWork;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.metrics.MetricsMap;
 import org.apache.solr.metrics.SolrMetricsContext;
@@ -40,7 +41,6 @@ import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Optional;
 import java.util.concurrent.Executor;
-import java.util.concurrent.ForkJoinPool;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.LongAdder;
 import java.util.function.Function;
@@ -113,7 +113,7 @@ public class CaffeineCache<K, V> extends SolrCacheBase implements SolrCache<K, V
     str = (String) args.get(CLEANUP_THREAD_PARAM);
     cleanupThread = str != null && Boolean.parseBoolean(str);
     if (cleanupThread) {
-      executor = ForkJoinPool.commonPool();
+      executor = ParWork.getRootSharedExecutor();
     } else {
       executor = Runnable::run;
     }
@@ -228,18 +228,11 @@ public class CaffeineCache<K, V> extends SolrCacheBase implements SolrCache<K, V
   @Override
   public void close() throws IOException {
     SolrCache.super.close();
-//    try (ParWork closer = new ParWork(this, true)) {
-//      closer.collect("superClose", () -> {
-//        try {
-//          SolrCache.super.close();
-//        } catch (IOException e) {
-//          log.warn("IOException on close", e);
-//        }
-//      });
-//      closer.collect("invalidateAll", () -> {
-//        cache.invalidateAll();
-//      });
-//    }
+    ParWork.getRootSharedExecutor().submit(()->{
+      cache.invalidateAll();
+      cache.cleanUp();
+    });
+
     ramBytes.reset();
   }
 
diff --git a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
index 8699d5b..6a5cd7d 100644
--- a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
+++ b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
@@ -55,6 +55,8 @@ import org.apache.solr.common.util.ObjectReleaseTracker;
 import org.apache.solr.core.CoreDescriptor;
 import org.apache.solr.core.SolrConfig.UpdateHandlerInfo;
 import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean;
+import org.apache.solr.metrics.SolrMetricProducer;
 import org.apache.solr.metrics.SolrMetricsContext;
 import org.apache.solr.request.LocalSolrQueryRequest;
 import org.apache.solr.request.SolrQueryRequest;
@@ -839,6 +841,7 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState
     }
     super.close();
     numDocsPending.reset();
+
     assert ObjectReleaseTracker.release(this);
   }
 
diff --git a/solr/core/src/java/org/apache/solr/update/UpdateHandler.java b/solr/core/src/java/org/apache/solr/update/UpdateHandler.java
index 7facbde..e17ef6a 100644
--- a/solr/core/src/java/org/apache/solr/update/UpdateHandler.java
+++ b/solr/core/src/java/org/apache/solr/update/UpdateHandler.java
@@ -96,6 +96,7 @@ UpdateHandler implements SolrInfoBean, Closeable {
   @Override
   public void close() throws IOException {
     this.closed = true;
+    SolrInfoBean.super.close();
     assert ObjectReleaseTracker.release(this);
   }
 
diff --git a/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java b/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java
index 92aa409..4caea5b 100644
--- a/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java
+++ b/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java
@@ -16,6 +16,7 @@
  */
 package org.apache.solr.update;
 
+import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.util.HashSet;
 import java.util.Set;
@@ -238,6 +239,11 @@ public class UpdateShardHandler implements SolrInfoBean {
         return this;
       });
     }
+    try {
+      SolrInfoBean.super.close();
+    } catch (IOException e) {
+      log.warn("Error closing", e);
+    }
     assert ObjectReleaseTracker.release(this);
   }
 
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 3e3531a..122331a 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
@@ -1033,7 +1033,7 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor {
 
   // internal helper method to setup request by processors who use this class.
   // NOTE: not called by this class!
-  void setupRequest(UpdateCommand cmd) {
+  protected void setupRequest(UpdateCommand cmd) {
     updateCommand = cmd;
     isLeader = getNonZkLeaderAssumption(req);
   }
diff --git a/solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java b/solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java
index 2a184e3..b3b58d5 100644
--- a/solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java
+++ b/solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java
@@ -637,7 +637,7 @@ public class DistributedZkUpdateProcessor extends DistributedUpdateProcessor {
   }
 
   @Override
-  void setupRequest(UpdateCommand cmd) {
+  protected void setupRequest(UpdateCommand cmd) {
     updateCommand = cmd;
     zkCheck();
     if (cmd instanceof AddUpdateCommand) {
@@ -786,6 +786,10 @@ public class DistributedZkUpdateProcessor extends DistributedUpdateProcessor {
         List<SolrCmdDistributor.Node> nodes = Collections.singletonList(
                 new SolrCmdDistributor.ForwardNode(zkController.getZkStateReader(), leaderReplica, collection, shardId));
         if (log.isDebugEnabled()) log.debug("Forward update to leader {}", nodes);
+
+        if (desc.getName().equals(leaderReplica.getName())) {
+          throw new IllegalStateException("We were asked to forward an update to ourself, which should not happen name=" + desc.getName());
+        }
         return nodes;
       }
 
diff --git a/solr/core/src/test/org/apache/solr/cloud/DocValuesNotIndexedTest.java b/solr/core/src/test/org/apache/solr/cloud/DocValuesNotIndexedTest.java
index be92fae..4a6b8c6 100644
--- a/solr/core/src/test/org/apache/solr/cloud/DocValuesNotIndexedTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/DocValuesNotIndexedTest.java
@@ -197,6 +197,7 @@ public class DocValuesNotIndexedTest extends SolrCloudTestCase {
       prop.resetBase();
     }
   }
+
   @Test
   public void testDistribFaceting() throws IOException, SolrServerException {
     // For this test, I want to insure that there are shards that do _not_ have a doc with any of the DV_only 
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/Replica.java b/solr/solrj/src/java/org/apache/solr/common/cloud/Replica.java
index 5c05e95..e206b93 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/Replica.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/Replica.java
@@ -18,6 +18,7 @@ package org.apache.solr.common.cloud;
 
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 import org.apache.solr.common.util.Utils;
@@ -150,14 +151,16 @@ public class Replica extends ZkNodeProps {
     this.collection = collection;
     this.slice = slice;
     this.name = name;
+
     this.nodeName = (String) propMap.get(ZkStateReader.NODE_NAME_PROP);
+
     this.baseUrl = nodeNameToBaseUrl.getBaseUrlForNodeName(this.nodeName);
     type = Type.get((String) propMap.get(ZkStateReader.REPLICA_TYPE));
-//    Objects.requireNonNull(this.collection, "'collection' must not be null");
-//    Objects.requireNonNull(this.slice, "'slice' must not be null");
-//    Objects.requireNonNull(this.name, "'name' must not be null");
-//    Objects.requireNonNull(this.nodeName, "'node_name' must not be null");
-//    Objects.requireNonNull(this.type, "'type' must not be null");
+    Objects.requireNonNull(this.collection, "'collection' must not be null");
+    Objects.requireNonNull(this.slice, "'slice' must not be null");
+    Objects.requireNonNull(this.name, "'name' must not be null");
+    Objects.requireNonNull(this.nodeName, "'node_name' must not be null");
+    Objects.requireNonNull(this.type, "'type' must not be null");
     if (propMap.get(ZkStateReader.STATE_PROP) != null) {
       if (propMap.get(ZkStateReader.STATE_PROP) instanceof  State) {
         this.state = (State) propMap.get(ZkStateReader.STATE_PROP);
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
index 6e61f2c..84da163 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
@@ -923,10 +923,10 @@ public class ZkStateReader implements SolrCloseable, Replica.NodeNameToBaseUrl {
   }
 
   public Replica getLeader(String collection, String shard) {
-    return getLeader(liveNodes, getClusterState().getCollection(collection), shard);
+    return getLeader(getClusterState().getCollection(collection), shard);
   }
 
-  public Replica getLeader(Set<String> liveNodes, DocCollection docCollection, String shard) {
+  private Replica getLeader(DocCollection docCollection, String shard) {
     Replica replica = docCollection != null ? docCollection.getLeader(shard) : null;
     if (replica != null && replica.getState() == Replica.State.ACTIVE) {
       return replica;
@@ -1023,13 +1023,15 @@ public class ZkStateReader implements SolrCloseable, Replica.NodeNameToBaseUrl {
           + " with live_nodes=" + liveNodes + " zkLeaderNode=" + getLeaderProps(collection, shard));
     }
 
-    if (returnLeader.get() == null) {
+    Replica leader = returnLeader.get();
+
+    if (leader == null) {
       throw new SolrException(ErrorCode.SERVER_ERROR, "No registered leader was found "
           + "collection: " + collection + " slice: " + shard + " saw state=" + clusterState.getCollectionOrNull(collection)
           + " with live_nodes=" + liveNodes + " zkLeaderNode=" + getLeaderProps(collection, shard));
     }
 
-    return returnLeader.get();
+    return leader;
   }
 
   public Replica getLeaderProps(final String collection, final String slice) {
diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
index 3d8975c..91ecf6d 100644
--- a/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
+++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
@@ -705,11 +705,6 @@ public class SolrTestCase extends Assert {
     if (thread.getName().contains(ParWork.ROOT_EXEC_NAME + "-") || thread.getName().contains("ParWork-") || thread.getName().contains("Core-")
         || thread.getName().contains("ProcessThread(")) {
       log.warn("interrupt on {}", thread.getName());
-      try {
-        thread.join(50);
-      } catch (InterruptedException e) {
-
-      }
       thread.interrupt();
       return true;
     }