You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2020/11/15 02:56:27 UTC

[lucene-solr] branch reference_impl_dev updated: @1205 Start working out rare issues where tests can fail with a null updatelog and should have one.

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 cf9bc29  @1205 Start working out rare issues where tests can fail with a null updatelog and should have one.
cf9bc29 is described below

commit cf9bc2976c552bab7d4a387c7d2f008d3ae31b1b
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Sat Nov 14 20:53:35 2020 -0600

    @1205 Start working out rare issues where tests can fail with a null updatelog and should have one.
---
 .../solr/cloud/ShardLeaderElectionContext.java     | 155 ++++++++++-----------
 .../java/org/apache/solr/cloud/SyncStrategy.java   |   5 +-
 .../java/org/apache/solr/update/UpdateHandler.java |  10 +-
 .../solr/configsets/minimal/conf/solrconfig.xml    |   4 +-
 .../configsets/resource-sharing/solrconfig.xml     |   4 +-
 .../solr/configsets/upload/regular/solrconfig.xml  |   4 +-
 .../upload/with-lib-directive/solrconfig.xml       |   6 +
 .../upload/with-script-processor/solrconfig.xml    |   4 +-
 .../apache/solr/cloud/ShardRoutingCustomTest.java  |   4 +-
 9 files changed, 107 insertions(+), 89 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContext.java b/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContext.java
index 869b8ae..6c066f3 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContext.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContext.java
@@ -126,8 +126,6 @@ final class ShardLeaderElectionContext extends ShardLeaderElectionContextBase {
     String coreName = leaderProps.getName();
 
     log.info("Run leader process for shard election {}", coreName);
-
-    ActionThrottle lt;
     try (SolrCore core = cc.getCore(coreName)) {
       if (core == null) {
         log.error("No SolrCore found, cannot become leader {}", coreName);
@@ -137,105 +135,107 @@ final class ShardLeaderElectionContext extends ShardLeaderElectionContextBase {
         log.info("We are closed, will not become leader");
         return;
       }
+      try {
+        ActionThrottle lt;
 
-      MDCLoggingContext.setCore(core);
-      lt = core.getUpdateHandler().getSolrCoreState().getLeaderThrottle();
+        MDCLoggingContext.setCore(core);
+        lt = core.getUpdateHandler().getSolrCoreState().getLeaderThrottle();
 
-      lt.minimumWaitBetweenActions();
-      lt.markAttemptingAction();
+        lt.minimumWaitBetweenActions();
+        lt.markAttemptingAction();
 
-      int leaderVoteWait = cc.getZkController().getLeaderVoteWait();
+        int leaderVoteWait = cc.getZkController().getLeaderVoteWait();
 
-      if (log.isDebugEnabled()) log.debug("Running the leader process for shard={} and weAreReplacement={} and leaderVoteWait={}", shardId, weAreReplacement, leaderVoteWait);
+        if (log.isDebugEnabled()) log.debug("Running the leader process for shard={} and weAreReplacement={} and leaderVoteWait={}", shardId, weAreReplacement, leaderVoteWait);
 
-      Replica.Type replicaType;
-      String coreNodeName;
-      boolean setTermToMax = false;
+        if (core.getUpdateHandler().getUpdateLog() == null) {
+          log.error("No UpdateLog found - cannot sync");
+          throw new SolrException(ErrorCode.SERVER_ERROR, "Replica with no update log configured cannot be leader");
+        }
 
-      CoreDescriptor cd = core.getCoreDescriptor();
-      CloudDescriptor cloudCd = cd.getCloudDescriptor();
-      replicaType = cloudCd.getReplicaType();
-      // should I be leader?
-      ZkShardTerms zkShardTerms = zkController.getShardTerms(collection, shardId);
-      if (zkShardTerms.registered(coreName) && !zkShardTerms.canBecomeLeader(coreName)) {
-        if (!waitForEligibleBecomeLeaderAfterTimeout(zkShardTerms, coreName, leaderVoteWait)) {
-          rejoinLeaderElection(core);
-          return;
-        } else {
-          // only log an error if this replica win the election
-          setTermToMax = true;
+        Replica.Type replicaType;
+        String coreNodeName;
+        boolean setTermToMax = false;
+
+        CoreDescriptor cd = core.getCoreDescriptor();
+        CloudDescriptor cloudCd = cd.getCloudDescriptor();
+        replicaType = cloudCd.getReplicaType();
+        // should I be leader?
+        ZkShardTerms zkShardTerms = zkController.getShardTerms(collection, shardId);
+        if (zkShardTerms.registered(coreName) && !zkShardTerms.canBecomeLeader(coreName)) {
+          if (!waitForEligibleBecomeLeaderAfterTimeout(zkShardTerms, coreName, leaderVoteWait)) {
+            rejoinLeaderElection(core);
+            return;
+          } else {
+            // only log an error if this replica win the election
+            setTermToMax = true;
+          }
         }
-      }
 
+        log.info("I may be the new leader - try and sync");
 
-      log.info("I may be the new leader - try and sync");
+        // nocommit
+        // we are going to attempt to be the leader
+        // first cancel any current recovery
+        core.getUpdateHandler().getSolrCoreState().cancelRecovery();
 
-      // nocommit
-      // we are going to attempt to be the leader
-      // first cancel any current recovery
-      core.getUpdateHandler().getSolrCoreState().cancelRecovery();
+        PeerSync.PeerSyncResult result = null;
+        boolean success = false;
 
-      PeerSync.PeerSyncResult result = null;
-      boolean success = false;
-      try {
         result = syncStrategy.sync(zkController, core, leaderProps, weAreReplacement);
         success = result.isSuccess();
-      } catch (Exception e) {
-        ParWork.propagateInterrupt("Exception while trying to sync", e);
-        throw new SolrException(ErrorCode.SERVER_ERROR, e);
-      }
-      UpdateLog ulog = core.getUpdateHandler().getUpdateLog();
 
-      if (!success) {
+        UpdateLog ulog = core.getUpdateHandler().getUpdateLog();
 
-        log.warn("Our sync attempt failed ulog={}", ulog);
-        boolean hasRecentUpdates = false;
-        if (ulog != null) {
-          // TODO: we could optimize this if necessary
-          try (UpdateLog.RecentUpdates recentUpdates = ulog.getRecentUpdates()) {
-            hasRecentUpdates = !recentUpdates.getVersions(1).isEmpty();
+        if (!success) {
+
+          log.warn("Our sync attempt failed ulog={}", ulog);
+          boolean hasRecentUpdates = false;
+          if (ulog != null) {
+            // TODO: we could optimize this if necessary
+            try (UpdateLog.RecentUpdates recentUpdates = ulog.getRecentUpdates()) {
+              hasRecentUpdates = !recentUpdates.getVersions(1).isEmpty();
+            }
           }
-        }
 
-        log.warn("Checking for recent versions in the update log", hasRecentUpdates);
-        if (!hasRecentUpdates) {
-          // we failed sync, but we have no versions - we can't sync in that case
-          // - we were active
-          // before, so become leader anyway if no one else has any versions either
-          if (result.getOtherHasVersions().orElse(false)) {
-            log.info("We failed sync, but we have no versions - we can't sync in that case. But others have some versions, so we should not become leader");
+          log.warn("Checking for recent versions in the update log", hasRecentUpdates);
+          if (!hasRecentUpdates) {
+            // we failed sync, but we have no versions - we can't sync in that case
+            // - we were active
+            // before, so become leader anyway if no one else has any versions either
+            if (result.getOtherHasVersions().orElse(false)) {
+              log.info("We failed sync, but we have no versions - we can't sync in that case. But others have some versions, so we should not become leader");
               rejoinLeaderElection(core);
               return;
-          } else {
-            log.info("We failed sync, but we have no versions - we can't sync in that case - we were active before, so become leader anyway");
-            success = true;
+            } else {
+              log.info("We failed sync, but we have no versions - we can't sync in that case - we were active before, so become leader anyway");
+              success = true;
+            }
           }
         }
-      }
-      log.info("Our sync attempt succeeded");
+        log.info("Our sync attempt succeeded");
 
-      // solrcloud_debug
-      if (log.isDebugEnabled()) {
-        try {
-          RefCounted<SolrIndexSearcher> searchHolder = core.getNewestSearcher(false);
-          SolrIndexSearcher searcher = searchHolder.get();
+        // solrcloud_debug
+        if (log.isDebugEnabled()) {
           try {
-            log.debug(core.getCoreContainer().getZkController().getNodeName() + " synched " + searcher.count(new MatchAllDocsQuery()));
-          } finally {
-            searchHolder.decref();
+            RefCounted<SolrIndexSearcher> searchHolder = core.getNewestSearcher(false);
+            SolrIndexSearcher searcher = searchHolder.get();
+            try {
+              log.debug(core.getCoreContainer().getZkController().getNodeName() + " synched " + searcher.count(new MatchAllDocsQuery()));
+            } finally {
+              searchHolder.decref();
+            }
+          } catch (Exception e) {
+            ParWork.propagateInterrupt(e);
+            throw new SolrException(ErrorCode.SERVER_ERROR, e);
           }
-        } catch (Exception e) {
-          ParWork.propagateInterrupt(e);
-          throw new SolrException(ErrorCode.SERVER_ERROR, e);
         }
-      }
-      if (!success) {
-        log.info("Sync with potential leader failed, rejoining election ...");
-        rejoinLeaderElection(core);
-        return;
-      }
+        if (!success) {
+          log.info("Sync with potential leader failed, rejoining election ...");
+          rejoinLeaderElection(core);
+          return;
+        }
 
-      try {
         if (replicaType == Replica.Type.TLOG) {
           // stop replicate from old leader
           zkController.stopReplicationFromLeader(coreName);
@@ -264,8 +264,8 @@ final class ShardLeaderElectionContext extends ShardLeaderElectionContextBase {
         core.getCoreDescriptor().getCloudDescriptor().setLeader(true);
 
         ZkNodeProps zkNodes = ZkNodeProps
-            .fromKeyVals(Overseer.QUEUE_OPERATION, OverseerAction.STATE.toLower(), ZkStateReader.COLLECTION_PROP, collection, ZkStateReader.CORE_NAME_PROP,
-                leaderProps.getName(), ZkStateReader.STATE_PROP, "leader");
+            .fromKeyVals(Overseer.QUEUE_OPERATION, OverseerAction.STATE.toLower(), ZkStateReader.COLLECTION_PROP, collection, ZkStateReader.CORE_NAME_PROP, leaderProps.getName(),
+                ZkStateReader.STATE_PROP, "leader");
         assert zkController != null;
         assert zkController.getOverseer() != null;
 
@@ -289,7 +289,6 @@ final class ShardLeaderElectionContext extends ShardLeaderElectionContextBase {
         rejoinLeaderElection(core);
       }
 
-
     } catch (AlreadyClosedException e) {
       log.info("CoreContainer is shutting down, won't become leader");
     } finally {
diff --git a/solr/core/src/java/org/apache/solr/cloud/SyncStrategy.java b/solr/core/src/java/org/apache/solr/cloud/SyncStrategy.java
index 5ce4e2b..7cca66a 100644
--- a/solr/core/src/java/org/apache/solr/cloud/SyncStrategy.java
+++ b/solr/core/src/java/org/apache/solr/cloud/SyncStrategy.java
@@ -80,10 +80,7 @@ public class SyncStrategy implements Closeable {
       log.info("Sync replicas to {}", leaderProps.getCoreUrl());
     }
 
-    if (core.getUpdateHandler().getUpdateLog() == null) {
-      log.error("No UpdateLog found - cannot sync");
-      return PeerSync.PeerSyncResult.failure();
-    }
+
     this.zkController = zkController;
     return syncReplicas(zkController, core, leaderProps, peerSyncOnlyWithActive);
   }
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 bc9002d..a35b896 100644
--- a/solr/core/src/java/org/apache/solr/update/UpdateHandler.java
+++ b/solr/core/src/java/org/apache/solr/update/UpdateHandler.java
@@ -163,16 +163,22 @@ UpdateHandler implements SolrInfoBean, Closeable {
       } else {
         ourUpdateLog = updateLog;
       }
+      ulog = ourUpdateLog;
+
+      if (ulog == null) {
+        log.info("No UpdateLog configured for UpdateHandler {} {} skip={}", updateLog, ulogPluginInfo, skipUpdateLog);
+      }
     } catch (Throwable e) {
+      log.error("Could not initialize the UpdateHandler", e);
       IOUtils.closeQuietly(ourUpdateLog);
       assert ObjectReleaseTracker.release(this);
       if (e instanceof Error) {
         throw e;
       }
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
-    } finally {
-      ulog = ourUpdateLog;
     }
+
+
   }
 
   /**
diff --git a/solr/core/src/test-files/solr/configsets/minimal/conf/solrconfig.xml b/solr/core/src/test-files/solr/configsets/minimal/conf/solrconfig.xml
index 63ea75e..ee004b3 100644
--- a/solr/core/src/test-files/solr/configsets/minimal/conf/solrconfig.xml
+++ b/solr/core/src/test-files/solr/configsets/minimal/conf/solrconfig.xml
@@ -33,7 +33,9 @@
     <commitWithin>
       <softCommit>${solr.commitwithin.softcommit:true}</softCommit>
     </commitWithin>
-
+    <updateLog>
+      <str name="dir">${solr.data.dir:}</str>
+    </updateLog>
   </updateHandler>
   <requestHandler name="/select" class="solr.SearchHandler">
     <lst name="defaults">
diff --git a/solr/core/src/test-files/solr/configsets/resource-sharing/solrconfig.xml b/solr/core/src/test-files/solr/configsets/resource-sharing/solrconfig.xml
index 163b274..0af4cd1 100644
--- a/solr/core/src/test-files/solr/configsets/resource-sharing/solrconfig.xml
+++ b/solr/core/src/test-files/solr/configsets/resource-sharing/solrconfig.xml
@@ -37,7 +37,9 @@
     <commitWithin>
       <softCommit>${solr.commitwithin.softcommit:true}</softCommit>
     </commitWithin>
-
+    <updateLog>
+      <str name="dir">${solr.data.dir:}</str>
+    </updateLog>
   </updateHandler>
   <searchComponent name="testComponent" class="org.apache.solr.handler.component.ResourceSharingTestComponent" />
 
diff --git a/solr/core/src/test-files/solr/configsets/upload/regular/solrconfig.xml b/solr/core/src/test-files/solr/configsets/upload/regular/solrconfig.xml
index 76612e5..f74c540 100644
--- a/solr/core/src/test-files/solr/configsets/upload/regular/solrconfig.xml
+++ b/solr/core/src/test-files/solr/configsets/upload/regular/solrconfig.xml
@@ -46,7 +46,9 @@
     <commitWithin>
       <softCommit>${solr.commitwithin.softcommit:true}</softCommit>
     </commitWithin>
-
+    <updateLog>
+      <str name="dir">${solr.data.dir:}</str>
+    </updateLog>
   </updateHandler>
   <requestHandler name="/select" class="solr.SearchHandler">
     <lst name="defaults">
diff --git a/solr/core/src/test-files/solr/configsets/upload/with-lib-directive/solrconfig.xml b/solr/core/src/test-files/solr/configsets/upload/with-lib-directive/solrconfig.xml
index bb875e3..265fc28 100644
--- a/solr/core/src/test-files/solr/configsets/upload/with-lib-directive/solrconfig.xml
+++ b/solr/core/src/test-files/solr/configsets/upload/with-lib-directive/solrconfig.xml
@@ -39,6 +39,12 @@
 
   <lib dir="${solr.install.dir:../../../..}/dist/" regex="solr-ltr-\d.*\.jar" />
 
+  <updateHandler class="solr.DirectUpdateHandler2">
+    <updateLog>
+      <str name="dir">${solr.data.dir:}</str>
+    </updateLog>
+  </updateHandler>
+
   <requestHandler name="/select" class="solr.SearchHandler">
     <lst name="defaults">
       <str name="echoParams">explicit</str>
diff --git a/solr/core/src/test-files/solr/configsets/upload/with-script-processor/solrconfig.xml b/solr/core/src/test-files/solr/configsets/upload/with-script-processor/solrconfig.xml
index 1f71487..8445c5c 100644
--- a/solr/core/src/test-files/solr/configsets/upload/with-script-processor/solrconfig.xml
+++ b/solr/core/src/test-files/solr/configsets/upload/with-script-processor/solrconfig.xml
@@ -46,7 +46,9 @@
     <commitWithin>
       <softCommit>${solr.commitwithin.softcommit:true}</softCommit>
     </commitWithin>
-
+    <updateLog>
+      <str name="dir">${solr.data.dir:}</str>
+    </updateLog>
   </updateHandler>
   <requestHandler name="/select" class="solr.SearchHandler">
     <lst name="defaults">
diff --git a/solr/core/src/test/org/apache/solr/cloud/ShardRoutingCustomTest.java b/solr/core/src/test/org/apache/solr/cloud/ShardRoutingCustomTest.java
index 95273a9..c8c372c 100644
--- a/solr/core/src/test/org/apache/solr/cloud/ShardRoutingCustomTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/ShardRoutingCustomTest.java
@@ -64,7 +64,9 @@ public class ShardRoutingCustomTest extends SolrCloudBridgeTestCase {
   }
 
   private boolean useTlogReplicas() {
-    return random().nextBoolean();
+    return false;
+    // nocommit - a TLog replica can end up configured with no update log
+    //return random().nextBoolean();
   }
 
 }