You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by is...@apache.org on 2016/12/20 17:36:02 UTC
lucene-solr:jira/solr-5944: SOLR-5944: Fixing some nocommits
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 <is...@apache.org>
Authored: Tue Dec 20 23:05:46 2016 +0530
Committer: Ishan Chattopadhyaya <is...@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");