lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From is...@apache.org
Subject lucene-solr:jira/solr-5944: SOLR-5944: Fixing some nocommits
Date Tue, 20 Dec 2016 17:36:02 GMT
Repository: lucene-solr
Updated Branches:
  refs/heads/jira/solr-5944 fbf681006 -> cef40ac16


SOLR-5944: Fixing some nocommits


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/cef40ac1
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/cef40ac1
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/cef40ac1

Branch: refs/heads/jira/solr-5944
Commit: cef40ac160f261ed4cabc1c47f9699d90aab44ca
Parents: fbf6810
Author: Ishan Chattopadhyaya <ishan@apache.org>
Authored: Tue Dec 20 23:05:46 2016 +0530
Committer: Ishan Chattopadhyaya <ishan@apache.org>
Committed: Tue Dec 20 23:05:46 2016 +0530

----------------------------------------------------------------------
 .../processor/DistributedUpdateProcessor.java       | 16 +++++-----------
 .../org/apache/solr/update/HardAutoCommitTest.java  |  4 ----
 .../test/org/apache/solr/update/PeerSyncTest.java   |  1 -
 .../solr/update/TestInPlaceUpdatesDistrib.java      |  9 +++------
 .../test/org/apache/solr/update/UpdateLogTest.java  |  1 -
 5 files changed, 8 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/cef40ac1/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
----------------------------------------------------------------------
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 85ff7c7..5696a29 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
@@ -1295,18 +1295,12 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor
{
         throw new SolrException(ErrorCode.SERVER_ERROR, "Can't find document with id=" +
id + ", but fetching from leader "
             + "failed since we're not in cloud mode.");
       }
+      Replica leader;
       try {
-        // nocommit: do not use forceUpdateCollection here, per shalin's comments back in
August...
-        
-        // Under no circumstances should we we calling `forceUpdateCollection` in the indexing
code path.
-        // It is just too dangerous in the face of high indexing rates. Instead we should
do what the
-        // DUP is already doing i.e. calling getLeaderRetry to figure out the current leader.
If the
-        // current replica is partitioned from the leader then we have other mechanisms for
taking care
-        // of it and the replica has no business trying to determine this.
-        zkController.getZkStateReader().forceUpdateCollection(cloudDesc.getCollectionName());
// nocommit
-      } catch (KeeperException | InterruptedException e1) { /* No worries if the force refresh
failed */ }
-      Replica leader = zkController.getClusterState().
-          getCollection(cloudDesc.getCollectionName()).getLeader(cloudDesc.getShardId());
+        leader = zkController.getZkStateReader().getLeaderRetry(cloudDesc.getCollectionName(),
cloudDesc.getShardId());
+      } catch (InterruptedException e) {
+        throw new SolrException(ErrorCode.SERVER_ERROR, "Exception during fetching from leader.",
e);
+      }
       leaderUrl = leader.getCoreUrl();
     }
     

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/cef40ac1/solr/core/src/test/org/apache/solr/update/HardAutoCommitTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/update/HardAutoCommitTest.java b/solr/core/src/test/org/apache/solr/update/HardAutoCommitTest.java
index e72d37d..3c652b2 100644
--- a/solr/core/src/test/org/apache/solr/update/HardAutoCommitTest.java
+++ b/solr/core/src/test/org/apache/solr/update/HardAutoCommitTest.java
@@ -48,10 +48,6 @@ public class HardAutoCommitTest extends AbstractSolrTestCase {
     clearIndex();
     // reload the core to clear stats
     h.getCoreContainer().reload(h.getCore().getName());
-
-    // nocommit: why was this line added in last patch? .. seems fine, but also unrelated
to issue?
-    // nocommit: addition seems fishy: was this a cut/paste mistake ment for one/all of the
other tests modified by this patch?
-    assertFalse(h.getCore().getSolrConfig().getUpdateHandlerInfo().commitWithinSoftCommit);
   }
 
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/cef40ac1/solr/core/src/test/org/apache/solr/update/PeerSyncTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/update/PeerSyncTest.java b/solr/core/src/test/org/apache/solr/update/PeerSyncTest.java
index 624cea1..fed30a1 100644
--- a/solr/core/src/test/org/apache/solr/update/PeerSyncTest.java
+++ b/solr/core/src/test/org/apache/solr/update/PeerSyncTest.java
@@ -243,7 +243,6 @@ public class PeerSyncTest extends BaseDistributedSearchTestCase {
     validateQACResponse(docsAdded, qacResponse);
 
     // lets add some in-place updates
-    // v = 5000; // nocommit: dead code?
     add(client0, seenLeader, sdoc("id", "5000", "val_i_dvo", 0, "title", "mytitle", "_version_",
5000)); // full update
     docsAdded.add(5000);
     assertSync(client1, numVersions, true, shardsArr[0]);

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/cef40ac1/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java
index a85adc9..5b847d1 100644
--- a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java
+++ b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java
@@ -914,8 +914,7 @@ public class TestInPlaceUpdatesDistrib extends AbstractFullDistribZkTestBase
{
   }
 
   UpdateRequest simulatedDeleteRequest(String query, long version) throws SolrServerException,
IOException {
-    // nocommit: why clients.get(0)? ... if we want LEADER why not just use LEADER ?
-    String baseUrl = getBaseUrl((HttpSolrClient)clients.get(0));
+    String baseUrl = getBaseUrl((HttpSolrClient)LEADER);
 
     UpdateRequest ur = new UpdateRequest();
     ur.deleteByQuery(query);
@@ -961,10 +960,8 @@ public class TestInPlaceUpdatesDistrib extends AbstractFullDistribZkTestBase
{
     ureq.add(doc);
     UpdateResponse resp;
     
-    synchronized (cloudClient) { // nocommit: WTF? do we need sync or not? if so why cloudClient?
-      // send updates to leader, to avoid SOLR-8733
-      resp = ureq.process(LEADER);
-    }
+    // send updates to leader, to avoid SOLR-8733
+    resp = ureq.process(LEADER);
     
     long returnedVersion = Long.parseLong(((NamedList)resp.getResponse().get("adds")).getVal(0).toString());
     assertTrue("Due to SOLR-8733, sometimes returned version is 0. Let us assert that we
have successfully"

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/cef40ac1/solr/core/src/test/org/apache/solr/update/UpdateLogTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/update/UpdateLogTest.java b/solr/core/src/test/org/apache/solr/update/UpdateLogTest.java
index 0e8d86e..3c1befa 100644
--- a/solr/core/src/test/org/apache/solr/update/UpdateLogTest.java
+++ b/solr/core/src/test/org/apache/solr/update/UpdateLogTest.java
@@ -43,7 +43,6 @@ public class UpdateLogTest extends SolrTestCaseJ4 {
   public static void beforeClass() throws Exception {
 
     // nocommit: does this test need to randomize between diff schema/fields used?
-    // nocommit: see nocommits/jira questions related to special dynamicField logic in AtomicUpdateDocumentMerger.isInPlaceUpdate
     
     initCore("solrconfig-tlog.xml", "schema-inplace-updates.xml");
 


Mime
View raw message