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/01/04 14:09:04 UTC
lucene-solr:jira/solr-5944: SOLR-5944: Fixing some test nocommits,
including adding a new test
Repository: lucene-solr
Updated Branches:
refs/heads/jira/solr-5944 7febd99b2 -> 6885f625e
SOLR-5944: Fixing some test nocommits, including adding a new test
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/6885f625
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/6885f625
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/6885f625
Branch: refs/heads/jira/solr-5944
Commit: 6885f625e102dd47650e1aeb6ee854ec38707651
Parents: 7febd99
Author: Ishan Chattopadhyaya <is...@apache.org>
Authored: Wed Jan 4 19:38:49 2017 +0530
Committer: Ishan Chattopadhyaya <is...@apache.org>
Committed: Wed Jan 4 19:38:49 2017 +0530
----------------------------------------------------------------------
.../java/org/apache/solr/update/UpdateLog.java | 2 +-
.../solr/cloud/TestStressInPlaceUpdates.java | 27 ++--
.../org/apache/solr/search/TestRecovery.java | 135 ++++++++++++++++++-
.../update/TestInPlaceUpdatesStandalone.java | 13 +-
4 files changed, 149 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6885f625/solr/core/src/java/org/apache/solr/update/UpdateLog.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/update/UpdateLog.java b/solr/core/src/java/org/apache/solr/update/UpdateLog.java
index 6d43ec0..69c2033 100644
--- a/solr/core/src/java/org/apache/solr/update/UpdateLog.java
+++ b/solr/core/src/java/org/apache/solr/update/UpdateLog.java
@@ -1560,7 +1560,7 @@ public static final int VERSION_IDX = 1;
// should currently be a List<Oper,Ver,Doc/Id>
List entry = (List) o;
-
+ log.info("HELLO: "+entry);
operationAndFlags = (Integer) entry.get(UpdateLog.FLAGS_IDX);
int oper = operationAndFlags & OPERATION_MASK;
long version = (Long) entry.get(UpdateLog.VERSION_IDX);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6885f625/solr/core/src/test/org/apache/solr/cloud/TestStressInPlaceUpdates.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestStressInPlaceUpdates.java b/solr/core/src/test/org/apache/solr/cloud/TestStressInPlaceUpdates.java
index e0e4a5f..db4e7f5 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestStressInPlaceUpdates.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestStressInPlaceUpdates.java
@@ -155,30 +155,23 @@ public class TestStressInPlaceUpdates extends AbstractFullDistribZkTestBase {
Map<Integer, DocInfo> newCommittedModel;
long version;
- // nocommit: dual sync blocks w/non-synced commits in between is not thread safe
- // nocommit: see jira comments for bad scenerio
synchronized (TestStressInPlaceUpdates.this) {
// take a snapshot of the model
// this is safe to do w/o synchronizing on the model because it's a ConcurrentHashMap
newCommittedModel = new HashMap<>(model);
version = snapshotCount++;
- }
-
- int chosenClientIndex = rand.nextInt(clients.size());
- if (rand.nextInt(100) < softCommitPercent) {
- log.info("softCommit start");
- clients.get(chosenClientIndex).commit(true, true, true);
- log.info("softCommit end");
- } else {
- log.info("hardCommit start");
- clients.get(chosenClientIndex).commit();
- log.info("hardCommit end");
- }
+ int chosenClientIndex = rand.nextInt(clients.size());
- // nocommit: dual sync blocks w/non-synced commits in between is not thread safe
- // nocommit: see jira comments for bad scenerio
- synchronized (TestStressInPlaceUpdates.this) {
+ if (rand.nextInt(100) < softCommitPercent) {
+ log.info("softCommit start");
+ clients.get(chosenClientIndex).commit(true, true, true);
+ log.info("softCommit end");
+ } else {
+ log.info("hardCommit start");
+ clients.get(chosenClientIndex).commit();
+ log.info("hardCommit end");
+ }
// install this model snapshot only if it's newer than the current one
if (version >= committedModelClock) {
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6885f625/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 fa1e45f..3cab869 100644
--- a/solr/core/src/test/org/apache/solr/search/TestRecovery.java
+++ b/solr/core/src/test/org/apache/solr/search/TestRecovery.java
@@ -20,6 +20,8 @@ package org.apache.solr.search;
import static org.apache.solr.update.processor.DistributingUpdateProcessorFactory.DISTRIB_UPDATE_PARAM;
import org.noggit.ObjectBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.schema.IndexSchema;
@@ -32,6 +34,7 @@ import org.junit.Test;
import java.io.File;
import java.io.RandomAccessFile;
+import java.lang.invoke.MethodHandles;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.util.ArrayDeque;
@@ -48,6 +51,7 @@ import java.util.concurrent.TimeUnit;
import org.apache.solr.update.processor.DistributedUpdateProcessor.DistribPhase;
public class TestRecovery extends SolrTestCaseJ4 {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
// means that we've seen the leader and have version info (i.e. we are a non-leader replica)
private static String FROM_LEADER = DistribPhase.FROMLEADER.toString();
@@ -94,9 +98,6 @@ public class TestRecovery extends SolrTestCaseJ4 {
@Test
public void testLogReplay() throws Exception {
- // nocommit: we also need testing that replay works correctly when mixing in-place updates + deletes
- // nocommit: both deletes by id, and DBQ (against pre/post in-place updates values)
- // nocommit: see longer jira comment about this method
try {
@@ -1118,6 +1119,134 @@ public class TestRecovery extends SolrTestCaseJ4 {
}
}
+ @Test
+ public void testLogReplayWithInPlaceUpdatesAndDeletes() throws Exception {
+ try {
+
+ DirectUpdateHandler2.commitOnClose = false;
+ final Semaphore logReplay = new Semaphore(0);
+ final Semaphore logReplayFinish = new Semaphore(0);
+
+ UpdateLog.testing_logReplayHook = () -> {
+ try {
+ assertTrue(logReplay.tryAcquire(timeout, TimeUnit.SECONDS));
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ };
+
+ UpdateLog.testing_logReplayFinishHook = () -> logReplayFinish.release();
+
+
+ clearIndex();
+ assertU(commit());
+
+ Deque<Long> versions = new ArrayDeque<>();
+ versions.addFirst(addAndGetVersion(sdoc("id", "A1"), null));
+
+ // DBQ of updated document using id
+ versions.addFirst(addAndGetVersion(sdoc("id", "A2", "val_i_dvo", "1"), null));
+ versions.addFirst(addAndGetVersion(sdoc("id", "A2", "val_i_dvo", map("set", 2)), null)); // in-place update
+ versions.addFirst(deleteByQueryAndGetVersion("id:A2", null));
+
+ // DBQ of updated document using updated value
+ versions.addFirst(addAndGetVersion(sdoc("id", "A3", "val_i_dvo", "101"), null));
+ versions.addFirst(addAndGetVersion(sdoc("id", "A3", "val_i_dvo", map("set", 102)), null)); // in-place update
+ versions.addFirst(deleteByQueryAndGetVersion("val_i_dvo:102", null));
+
+ // DBQ using an intermediate update value (shouldn't delete anything)
+ versions.addFirst(addAndGetVersion(sdoc("id", "A4", "val_i_dvo", "200"), null));
+ versions.addFirst(addAndGetVersion(sdoc("id", "A4", "val_i_dvo", map("inc", "1")), null)); // in-place update
+ versions.addFirst(addAndGetVersion(sdoc("id", "A4", "val_i_dvo", map("inc", "1")), null)); // in-place update
+ versions.addFirst(deleteByQueryAndGetVersion("val_i_dvo:201", null));
+
+ // DBI of updated document
+ versions.addFirst(addAndGetVersion(sdoc("id", "A5", "val_i_dvo", "300"), null));
+ versions.addFirst(addAndGetVersion(sdoc("id", "A5", "val_i_dvo", map("inc", "1")), null)); // in-place update
+ versions.addFirst(addAndGetVersion(sdoc("id", "A5", "val_i_dvo", map("inc", "1")), null)); // in-place update
+ versions.addFirst(deleteAndGetVersion("A5", null));
+
+ assertJQ(req("q","*:*"),"/response/numFound==0");
+
+
+ assertJQ(req("qt","/get", "getVersions",""+versions.size()) ,"/versions==" + versions);
+
+ h.close();
+ createCore();
+ // Clearing the updatelog as it would happen after a fresh node restart
+ h.getCore().getUpdateHandler().getUpdateLog().deleteAll();
+
+ // Solr should kick this off now
+ // h.getCore().getUpdateHandler().getUpdateLog().recoverFromLog();
+
+ // verify that previous close didn't do a commit
+ // recovery should be blocked by our hook
+ assertJQ(req("q","*:*") ,"/response/numFound==0");
+
+ // make sure we can still access versions after a restart
+ assertJQ(req("qt","/get", "getVersions",""+versions.size()),"/versions==" + versions);
+
+ // unblock recovery
+ logReplay.release(1000);
+
+ // make sure we can still access versions during recovery
+ assertJQ(req("qt","/get", "getVersions",""+versions.size()),"/versions==" + versions);
+
+ // wait until recovery has finished
+ assertTrue(logReplayFinish.tryAcquire(timeout, TimeUnit.SECONDS));
+ assertJQ(req("q","val_i_dvo:202") ,"/response/numFound==1"); // assert that in-place update is retained
+
+ assertJQ(req("q","*:*") ,"/response/numFound==2");
+ assertJQ(req("q","id:A2") ,"/response/numFound==0");
+ assertJQ(req("q","id:A3") ,"/response/numFound==0");
+ assertJQ(req("q","id:A4") ,"/response/numFound==1");
+ assertJQ(req("q","id:A5") ,"/response/numFound==0");
+
+ // make sure we can still access versions after recovery
+ assertJQ(req("qt","/get", "getVersions",""+versions.size()) ,"/versions==" + versions);
+
+ assertU(adoc("id","A10"));
+
+ h.close();
+ createCore();
+ // Solr should kick this off now
+ // h.getCore().getUpdateHandler().getUpdateLog().recoverFromLog();
+
+ // wait until recovery has finished
+ assertTrue(logReplayFinish.tryAcquire(timeout, TimeUnit.SECONDS));
+ assertJQ(req("q","*:*") ,"/response/numFound==3");
+ assertJQ(req("q","id:A2") ,"/response/numFound==0");
+ assertJQ(req("q","id:A3") ,"/response/numFound==0");
+ assertJQ(req("q","id:A4") ,"/response/numFound==1");
+ assertJQ(req("q","id:A5") ,"/response/numFound==0");
+ assertJQ(req("q","id:A10"),"/response/numFound==1");
+
+ // no updates, so insure that recovery does not run
+ h.close();
+ int permits = logReplay.availablePermits();
+ createCore();
+ // Solr should kick this off now
+ // h.getCore().getUpdateHandler().getUpdateLog().recoverFromLog();
+
+ assertJQ(req("q","*:*") ,"/response/numFound==3");
+ assertJQ(req("q","val_i_dvo:202") ,"/response/numFound==1"); // assert that in-place update is retained
+ assertJQ(req("q","id:A2") ,"/response/numFound==0");
+ assertJQ(req("q","id:A3") ,"/response/numFound==0");
+ assertJQ(req("q","id:A4") ,"/response/numFound==1");
+ assertJQ(req("q","id:A5") ,"/response/numFound==0");
+ assertJQ(req("q","id:A10"),"/response/numFound==1");
+ Thread.sleep(100);
+ assertEquals(permits, logReplay.availablePermits()); // no updates, so insure that recovery didn't run
+
+ assertEquals(UpdateLog.State.ACTIVE, h.getCore().getUpdateHandler().getUpdateLog().getState());
+
+ } finally {
+ DirectUpdateHandler2.commitOnClose = true;
+ UpdateLog.testing_logReplayHook = null;
+ UpdateLog.testing_logReplayFinishHook = null;
+ }
+
+ }
// NOTE: replacement must currently be same size
private static void findReplace(byte[] from, byte[] to, byte[] data) {
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6885f625/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java
index 007d959..663e592 100644
--- a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java
+++ b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java
@@ -20,6 +20,7 @@ package org.apache.solr.update;
import static org.junit.internal.matchers.StringContains.containsString;
import static org.apache.solr.update.UpdateLogTest.buildAddUpdateCommand;
+import static org.apache.solr.update.processor.DistributingUpdateProcessorFactory.DISTRIB_UPDATE_PARAM;
import java.util.Arrays;
import java.util.HashMap;
@@ -31,6 +32,7 @@ import java.util.Set;
import org.apache.lucene.index.FieldInfo;
import org.apache.lucene.util.TestUtil;
+import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer;
import org.apache.solr.common.SolrDocument;
@@ -40,10 +42,10 @@ import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.SolrInputField;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.update.processor.DistributedUpdateProcessor;
+import org.apache.solr.update.processor.DistributedUpdateProcessor.DistribPhase;
import org.apache.solr.schema.IndexSchema;
import org.apache.solr.schema.SchemaField;
import org.apache.solr.search.SolrIndexSearcher;
-import org.apache.solr.search.TestRTGBase;
import org.apache.solr.update.processor.AtomicUpdateDocumentMerger;
import org.apache.solr.util.RefCounted;
import org.junit.Before;
@@ -54,10 +56,7 @@ import org.junit.Test;
/**
* Tests the in-place updates (docValues updates) for a standalone Solr instance.
*/
-public class TestInPlaceUpdatesStandalone extends TestRTGBase {
- // nocommit: why is this class extending TestRTGBase?
- // nocommit: it doesn't seem to use any features of that baseclass (and was subclassing SolrTestCaseJ4 in previous patches)
-
+public class TestInPlaceUpdatesStandalone extends SolrTestCaseJ4 {
private static SolrClient client;
@BeforeClass
@@ -90,8 +89,8 @@ public class TestInPlaceUpdatesStandalone extends TestRTGBase {
}
@Before
- public void deleteAllAndCommit() {
- clearIndex();
+ public void deleteAllAndCommit() throws Exception {
+ deleteByQueryAndGetVersion("*:*", params("_version_", Long.toString(-Long.MAX_VALUE), DISTRIB_UPDATE_PARAM, DistribPhase.FROMLEADER.toString()));
assertU(commit("softCommit", "false"));
}