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 2017/02/19 08:34:58 UTC
[09/13] lucene-solr:branch_6x: SOLR-10114: add _version_ field to
child documents, fix reordered-dbq to not drop child docs
SOLR-10114: add _version_ field to child documents, fix reordered-dbq to not drop child docs
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/ad9195d7
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/ad9195d7
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/ad9195d7
Branch: refs/heads/branch_6x
Commit: ad9195d757c298a241ef2488b4b17623a44afdd7
Parents: 476cea5
Author: yonik <yo...@apache.org>
Authored: Wed Feb 15 22:51:21 2017 -0500
Committer: Ishan Chattopadhyaya <is...@apache.org>
Committed: Sun Feb 19 13:37:20 2017 +0530
----------------------------------------------------------------------
solr/CHANGES.txt | 3 +
.../apache/solr/update/AddUpdateCommand.java | 3 +
.../solr/update/DirectUpdateHandler2.java | 26 ++++--
.../org/apache/solr/search/TestRecovery.java | 98 ++++++++++++++++++--
.../java/org/apache/solr/SolrTestCaseJ4.java | 12 +++
5 files changed, 126 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad9195d7/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 6cf029c..1e8479f 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -122,6 +122,9 @@ Bug Fixes
* SOLR-10159: When DBQ is reordered with an in-place update, upon whose updated value the DBQ is based
on, the DBQ fails due to excessive caching in DeleteByQueryWrapper (Ishan Chattopadhyaya)
+* SOLR-10114: Reordered delete-by-query causes inconsistenties between shards that have
+ child documents (Mano Kovacs, Mihaly Toth, yonik)
+
Other Changes
----------------------
* SOLR-9980: Expose configVersion in core admin status (Jessica Cheng Mallet via Tom�s Fern�ndez L�bbe)
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad9195d7/solr/core/src/java/org/apache/solr/update/AddUpdateCommand.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/update/AddUpdateCommand.java b/solr/core/src/java/org/apache/solr/update/AddUpdateCommand.java
index db1d79b..0ede728 100644
--- a/solr/core/src/java/org/apache/solr/update/AddUpdateCommand.java
+++ b/solr/core/src/java/org/apache/solr/update/AddUpdateCommand.java
@@ -189,8 +189,11 @@ public class AddUpdateCommand extends UpdateCommand implements Iterable<Document
String idField = getHashableId();
+ boolean isVersion = version != 0;
+
for (SolrInputDocument sdoc : all) {
sdoc.setField("_root_", idField); // should this be a string or the same type as the ID?
+ if(isVersion) sdoc.setField(VersionInfo.VERSION_FIELD, version);
// TODO: if possible concurrent modification exception (if SolrInputDocument not cloned and is being forwarded to replicas)
// then we could add this field to the generated lucene document instead.
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad9195d7/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
----------------------------------------------------------------------
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 7f82a88..5eb65f8 100644
--- a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
+++ b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
@@ -259,7 +259,7 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState
private void doNormalUpdate(AddUpdateCommand cmd) throws IOException {
Term updateTerm;
- Term idTerm = new Term(cmd.isBlock() ? "_root_" : idField.getName(), cmd.getIndexedId());
+ Term idTerm = getIdTerm(cmd);
boolean del = false;
if (cmd.updateTerm == null) {
updateTerm = idTerm;
@@ -273,12 +273,7 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState
try {
IndexWriter writer = iw.get();
- if (cmd.isBlock()) {
- writer.updateDocuments(updateTerm, cmd);
- } else {
- updateDocOrDocValues(cmd, writer, updateTerm);
- }
- // SolrCore.verbose("updateDocument",updateTerm,"DONE");
+ updateDocOrDocValues(cmd, writer, updateTerm);
if (del) { // ensure id remains unique
BooleanQuery.Builder bq = new BooleanQuery.Builder();
@@ -323,7 +318,7 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState
}
Document luceneDocument = cmd.getLuceneDocument();
- Term idTerm = new Term(idField.getName(), cmd.getIndexedId());
+ Term idTerm = getIdTerm(cmd);
RefCounted<IndexWriter> iw = solrCoreState.getIndexWriter(core);
try {
@@ -347,6 +342,10 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState
}
+ private Term getIdTerm(AddUpdateCommand cmd) {
+ return new Term(cmd.isBlock() ? "_root_" : idField.getName(), cmd.getIndexedId());
+ }
+
private void updateDeleteTrackers(DeleteUpdateCommand cmd) {
if ((cmd.getFlags() & UpdateCommand.IGNORE_AUTOCOMMIT) == 0) {
if (commitWithinSoftCommit) {
@@ -870,13 +869,22 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState
log.debug("updateDocValues({})", cmd);
writer.updateDocValues(updateTerm, fieldsToUpdate.toArray(new Field[fieldsToUpdate.size()]));
} else {
+ updateDocument(cmd, writer, updateTerm);
+ }
+ }
+
+ private void updateDocument(AddUpdateCommand cmd, IndexWriter writer, Term updateTerm) throws IOException {
+ if(cmd.isBlock()){
+ log.debug("updateDocuments({})", cmd);
+ writer.updateDocuments(updateTerm, cmd);
+ }else{
Document luceneDocument = cmd.getLuceneDocument(false);
log.debug("updateDocument({})", cmd);
writer.updateDocument(updateTerm, luceneDocument);
}
}
-
+
/////////////////////////////////////////////////////////////////////
// SolrInfoMBean stuff: Statistics and Module Info
/////////////////////////////////////////////////////////////////////
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad9195d7/solr/core/src/test/org/apache/solr/search/TestRecovery.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/TestRecovery.java b/solr/core/src/test/org/apache/solr/search/TestRecovery.java
index 29efa52..84ba2fa 100644
--- a/solr/core/src/test/org/apache/solr/search/TestRecovery.java
+++ b/solr/core/src/test/org/apache/solr/search/TestRecovery.java
@@ -23,6 +23,7 @@ import com.codahale.metrics.Gauge;
import com.codahale.metrics.Meter;
import com.codahale.metrics.Metric;
import com.codahale.metrics.MetricRegistry;
+import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.metrics.SolrMetricManager;
import org.noggit.ObjectBuilder;
@@ -69,6 +70,9 @@ public class TestRecovery extends SolrTestCaseJ4 {
// TODO: fix this test to not require FSDirectory
static String savedFactory;
+
+ private interface RunnableWithException{void run () throws Exception;}
+
@BeforeClass
public static void beforeClass() throws Exception {
savedFactory = System.getProperty("solr.DirectoryFactory");
@@ -290,6 +294,86 @@ public class TestRecovery extends SolrTestCaseJ4 {
@Test
public void testLogReplayWithReorderedDBQ() throws Exception {
+ testLogReplayWithReorderedDBQWrapper(() -> {
+ updateJ(jsonAdd(sdoc("id", "B1", "_version_", "1010")), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ updateJ(jsonDelQ("id:B2"), params(DISTRIB_UPDATE_PARAM, FROM_LEADER, "_version_", "-1017")); // This should've arrived after the 1015th update
+ updateJ(jsonAdd(sdoc("id", "B2", "_version_", "1015")), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ updateJ(jsonAdd(sdoc("id", "B3", "_version_", "1020")), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ },
+ () -> assertJQ(req("q", "*:*"), "/response/numFound==2")
+ );
+ }
+
+ @Test
+ public void testLogReplayWithReorderedDBQByAsterixAndChildDocs() throws Exception {
+ testLogReplayWithReorderedDBQWrapper(() -> {
+ // 1010 - will be deleted
+ updateJ(jsonAdd(sdocWithChildren("B1", "1010")), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ // 1018 - should be kept, including child docs
+ updateJ(jsonAdd(sdocWithChildren("B2", "1018")), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ // 1017 - delete should affect only 1010
+ updateJ(jsonDelQ("id:*"), params(DISTRIB_UPDATE_PARAM, FROM_LEADER, "_version_", "-1017")); // This should've arrived after the 1015th update
+ // 1012 - will be deleted
+ updateJ(jsonAdd(sdoc("id", "B3", "_version_", "1012")), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ // 1020 - should be untouched
+ updateJ(jsonAdd(sdocWithChildren("B4", "1020")), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ },
+ () -> assertJQ(req("q", "*:*"), "/response/numFound==6")
+ );
+ }
+
+ @Test
+ public void testLogReplayWithReorderedDBQByIdAndChildDocs() throws Exception {
+ testLogReplayWithReorderedDBQWrapper(() -> {
+ // 1010 - will be deleted
+ updateJ(jsonAdd(sdocWithChildren("B1", "1010")), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ // 1018 - should be kept, including child docs
+ updateJ(jsonAdd(sdocWithChildren("B2", "1018")), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ // 1017 - delete should affect only 1010
+ updateJ(jsonDelQ("id:B1 id:B2 id:B3 id:B4"), params(DISTRIB_UPDATE_PARAM, FROM_LEADER, "_version_", "-1017")); // This should've arrived after the 1015th update
+ // 1012 - will be deleted
+ updateJ(jsonAdd(sdoc("id", "B3", "_version_", "1012")), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ // 1020 - should be untouched
+ updateJ(jsonAdd(sdocWithChildren("B4", "1020")), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ },
+ () -> assertJQ(req("q", "*:*"), "/response/numFound==8") // B2, B4 and 6 children docs (delete by id does not delete child docs)
+ );
+ }
+
+ @Test
+ public void testLogReplayWithReorderedDBQInsertingChildnodes() throws Exception {
+ testLogReplayWithReorderedDBQWrapper(() -> {
+ updateJ(jsonDelQ("id:B2"), params(DISTRIB_UPDATE_PARAM, FROM_LEADER, "_version_", "-1017"));
+ // test doc: B1
+ // 1013 - will be inserted with 3 children
+ updateJ(jsonAdd(sdocWithChildren("B1", "1013", 3)), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ },
+ () -> assertJQ(req("q", "*:*"), "/response/numFound==4") // B1 and B2, plus 2x 3 children
+ );
+ }
+
+
+ @Test
+ public void testLogReplayWithReorderedDBQUpdateWithDifferentChildCount() throws Exception {
+ testLogReplayWithReorderedDBQWrapper(() -> {
+ // control
+ // 1013 - will be inserted with 3 children
+ updateJ(jsonAdd(sdocWithChildren("B1", "1011", 2)), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ // 1018 - this should be the final
+ updateJ(jsonAdd(sdocWithChildren("B1", "1012", 3)), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+
+ // 1013 - will be inserted with 3 children
+ updateJ(jsonAdd(sdocWithChildren("B2", "1013", 2)), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ updateJ(jsonDelQ("id:B3"), params(DISTRIB_UPDATE_PARAM, FROM_LEADER, "_version_", "-1019"));
+ // 1018 - this should be the final
+ updateJ(jsonAdd(sdocWithChildren("B2", "1018", 3)), params(DISTRIB_UPDATE_PARAM, FROM_LEADER));
+ },
+ () -> assertJQ(req("q", "*:*"), "/response/numFound==8") // B1+3children+B2+3children
+ );
+ }
+
+ private void testLogReplayWithReorderedDBQWrapper(RunnableWithException act, RunnableWithException assrt) throws Exception {
+
try {
DirectUpdateHandler2.commitOnClose = false;
@@ -310,13 +394,11 @@ public class TestRecovery extends SolrTestCaseJ4 {
clearIndex();
assertU(commit());
- updateJ(jsonAdd(sdoc("id","B1", "_version_","1010")), params(DISTRIB_UPDATE_PARAM,FROM_LEADER));
- updateJ(jsonDelQ("id:B2"), params(DISTRIB_UPDATE_PARAM,FROM_LEADER, "_version_","-1017")); // This should've arrived after the 1015th update
- updateJ(jsonAdd(sdoc("id","B2", "_version_","1015")), params(DISTRIB_UPDATE_PARAM,FROM_LEADER));
- updateJ(jsonAdd(sdoc("id","B3", "_version_","1020")), params(DISTRIB_UPDATE_PARAM,FROM_LEADER));
+ // Adding some documents
+ act.run();
- assertJQ(req("q","*:*"),"/response/numFound==0");
+ assertJQ(req("q", "*:*"), "/response/numFound==0");
h.close();
createCore();
@@ -325,7 +407,7 @@ public class TestRecovery extends SolrTestCaseJ4 {
// verify that previous close didn't do a commit
// recovery should be blocked by our hook
- assertJQ(req("q","*:*") ,"/response/numFound==0");
+ assertJQ(req("q", "*:*"), "/response/numFound==0");
// unblock recovery
logReplay.release(1000);
@@ -333,7 +415,9 @@ public class TestRecovery extends SolrTestCaseJ4 {
// wait until recovery has finished
assertTrue(logReplayFinish.tryAcquire(timeout, TimeUnit.SECONDS));
- assertJQ(req("q","*:*") ,"/response/numFound==2");
+ // Asserting
+ assrt.run();
+
} finally {
DirectUpdateHandler2.commitOnClose = true;
UpdateLog.testing_logReplayHook = null;
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad9195d7/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
----------------------------------------------------------------------
diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
index 34cfb94..8bce06f 100644
--- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
+++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
@@ -1296,6 +1296,18 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
return sd;
}
+ public SolrInputDocument sdocWithChildren(String id, String version) {
+ return sdocWithChildren(id, version, 2);
+ }
+
+ public SolrInputDocument sdocWithChildren(String id, String version, int childCount) {
+ SolrInputDocument doc = sdoc("id", id, "_version_", version);
+ for (int i = 0; i < childCount; i++) {
+ doc.addChildDocument(sdoc("id", id + "_child" + i));
+ }
+ return doc;
+ }
+
public static List<SolrInputDocument> sdocs(SolrInputDocument... docs) {
return Arrays.asList(docs);
}