You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2018/04/20 20:29:16 UTC
lucene-solr:branch_7_3: SOLR-12256: AliasesManager.update() should
call ZooKeeper.sync() * SetAliasPropCmd now calls AliasesManager.update()
first. * SetAliasPropCmd now more efficiently updates multiple values. *
Tests: Commented out BadApple annotation
Repository: lucene-solr
Updated Branches:
refs/heads/branch_7_3 ca78f0e0c -> 566c07f7d
SOLR-12256: AliasesManager.update() should call ZooKeeper.sync()
* SetAliasPropCmd now calls AliasesManager.update() first.
* SetAliasPropCmd now more efficiently updates multiple values.
* Tests: Commented out BadApple annotations on alias related stuff.
(cherry picked from commit b16b380)
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/566c07f7
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/566c07f7
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/566c07f7
Branch: refs/heads/branch_7_3
Commit: 566c07f7de3ada3a537d34ec10a38565f5094398
Parents: ca78f0e
Author: David Smiley <ds...@apache.org>
Authored: Fri Apr 20 16:22:16 2018 -0400
Committer: David Smiley <ds...@apache.org>
Committed: Fri Apr 20 16:29:08 2018 -0400
----------------------------------------------------------------------
solr/CHANGES.txt | 2 +
.../cloud/api/collections/SetAliasPropCmd.java | 45 +++++++++++---------
.../apache/solr/cloud/AliasIntegrationTest.java | 32 +++++++-------
.../solr/cloud/CreateRoutedAliasTest.java | 6 +++
.../TimeRoutedAliasUpdateProcessorTest.java | 31 ++++++++------
.../apache/solr/common/cloud/ZkStateReader.java | 5 ++-
6 files changed, 69 insertions(+), 52 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/566c07f7/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index c6aa064..1ad2c87 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -34,6 +34,8 @@ Bug Fixes
* SOLR-12204: Upgrade commons-fileupload dependency to 1.3.3 to address CVE-2016-1000031. (Steve Rowe)
+* SOLR-12256: Fixed some eventual-consistency issues with collection aliases by using ZooKeeper.sync(). (David Smiley)
+
* SOLR-12087: Deleting replicas sometimes fails and causes the replicas to exist in the down state (Cao Manh Dat)
* SOLR-12146: LIR should skip deleted replicas (Cao Manh Dat)
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/566c07f7/solr/core/src/java/org/apache/solr/cloud/api/collections/SetAliasPropCmd.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/SetAliasPropCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/SetAliasPropCmd.java
index 618b72d..fdee1d1 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/SetAliasPropCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/SetAliasPropCmd.java
@@ -18,6 +18,7 @@
package org.apache.solr.cloud.api.collections;
import java.lang.invoke.MethodHandles;
+import java.util.LinkedHashMap;
import java.util.Locale;
import java.util.Map;
@@ -29,7 +30,7 @@ import org.apache.solr.common.util.NamedList;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import static org.apache.solr.cloud.api.collections.OverseerCollectionMessageHandler.*;
+import static org.apache.solr.cloud.api.collections.OverseerCollectionMessageHandler.Cmd;
import static org.apache.solr.common.SolrException.ErrorCode.BAD_REQUEST;
import static org.apache.solr.common.params.CommonParams.NAME;
@@ -49,33 +50,35 @@ public class SetAliasPropCmd implements Cmd {
public void call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception {
String aliasName = message.getStr(NAME);
+ final ZkStateReader.AliasesManager aliasesManager = messageHandler.zkStateReader.aliasesManager;
- ZkStateReader zkStateReader = messageHandler.zkStateReader;
- if (zkStateReader.getAliases().getCollectionAliasMap().get(aliasName) == null) {
+ // Ensure we see the alias. This may be redundant but SetAliasPropCmd isn't expected to be called very frequently
+ aliasesManager.update();
+
+ if (aliasesManager.getAliases().getCollectionAliasMap().get(aliasName) == null) {
// nicer than letting aliases object throw later on...
throw new SolrException(BAD_REQUEST,
String.format(Locale.ROOT, "Can't modify non-existent alias %s", aliasName));
}
@SuppressWarnings("unchecked")
- Map<String, String> properties = (Map<String, String>) message.get(PROPERTIES);
-
- zkStateReader.aliasesManager.applyModificationAndExportToZk(aliases1 -> {
- for (Map.Entry<String, String> entry : properties.entrySet()) {
- String key = entry.getKey();
- if ("".equals(key.trim())) {
- throw new SolrException(BAD_REQUEST, "property keys must not be pure whitespace");
- }
- if (!key.equals(key.trim())) {
- throw new SolrException(BAD_REQUEST, "property keys should not begin or end with whitespace");
- }
- String value = entry.getValue();
- if ("".equals(value)) {
- value = null;
- }
- aliases1 = aliases1.cloneWithCollectionAliasProperties(aliasName, key, value);
+ Map<String, String> properties = new LinkedHashMap<>((Map<String, String>) message.get(PROPERTIES));
+
+ // check & cleanup properties. It's a mutable copy.
+ for (Map.Entry<String, String> entry : properties.entrySet()) {
+ String key = entry.getKey();
+ if ("".equals(key.trim())) {
+ throw new SolrException(BAD_REQUEST, "property keys must not be pure whitespace");
+ }
+ if (!key.equals(key.trim())) {
+ throw new SolrException(BAD_REQUEST, "property keys should not begin or end with whitespace");
}
- return aliases1;
- });
+ String value = entry.getValue();
+ if ("".equals(value)) {
+ entry.setValue(null);
+ }
+ }
+
+ aliasesManager.applyModificationAndExportToZk(aliases1 -> aliases1.cloneWithCollectionAliasProperties(aliasName, properties));
}
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/566c07f7/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java b/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
index 2944a25..2a82894 100644
--- a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
@@ -93,7 +93,7 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
}
@Test
- @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028")
+ //@BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028")
public void testProperties() throws Exception {
CollectionAdminRequest.createCollection("collection1meta", "conf", 2, 1).process(cluster.getSolrClient());
CollectionAdminRequest.createCollection("collection2meta", "conf", 1, 1).process(cluster.getSolrClient());
@@ -118,16 +118,16 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
assertTrue(((Map<String,Map<String,?>>)Utils.fromJSON(rawBytes)).get("collection").get("meta1") instanceof String);
// set properties
- UnaryOperator<Aliases> op5 = a -> a.cloneWithCollectionAliasProperties("meta1", "foo", "bar");
- aliasesManager.applyModificationAndExportToZk(op5);
+ aliasesManager.applyModificationAndExportToZk(a1 ->
+ a1.cloneWithCollectionAliasProperties("meta1", "foo", "bar"));
Map<String, String> meta = zkStateReader.getAliases().getCollectionAliasProperties("meta1");
assertNotNull(meta);
assertTrue(meta.containsKey("foo"));
assertEquals("bar", meta.get("foo"));
// set more properties
- UnaryOperator<Aliases> op4 = a -> a.cloneWithCollectionAliasProperties("meta1", "foobar", "bazbam");
- aliasesManager.applyModificationAndExportToZk(op4);
+ aliasesManager.applyModificationAndExportToZk( a1 ->
+ a1.cloneWithCollectionAliasProperties("meta1", "foobar", "bazbam"));
meta = zkStateReader.getAliases().getCollectionAliasProperties("meta1");
assertNotNull(meta);
@@ -140,8 +140,8 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
assertEquals("bazbam", meta.get("foobar"));
// remove properties
- UnaryOperator<Aliases> op3 = a -> a.cloneWithCollectionAliasProperties("meta1", "foo", null);
- aliasesManager.applyModificationAndExportToZk(op3);
+ aliasesManager.applyModificationAndExportToZk(a1 ->
+ a1.cloneWithCollectionAliasProperties("meta1", "foo", null));
meta = zkStateReader.getAliases().getCollectionAliasProperties("meta1");
assertNotNull(meta);
@@ -153,18 +153,17 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
assertEquals("bazbam", meta.get("foobar"));
// removal of non existent key should succeed.
- UnaryOperator<Aliases> op2 = a -> a.cloneWithCollectionAliasProperties("meta1", "foo", null);
- aliasesManager.applyModificationAndExportToZk(op2);
+ aliasesManager.applyModificationAndExportToZk(a2 ->
+ a2.cloneWithCollectionAliasProperties("meta1", "foo", null));
// chained invocations
- UnaryOperator<Aliases> op1 = a ->
- a.cloneWithCollectionAliasProperties("meta1", "foo2", "bazbam")
- .cloneWithCollectionAliasProperties("meta1", "foo3", "bazbam2");
- aliasesManager.applyModificationAndExportToZk(op1);
+ aliasesManager.applyModificationAndExportToZk(a1 ->
+ a1.cloneWithCollectionAliasProperties("meta1", "foo2", "bazbam")
+ .cloneWithCollectionAliasProperties("meta1", "foo3", "bazbam2"));
// some other independent update (not overwritten)
- UnaryOperator<Aliases> op = a -> a.cloneWithCollectionAlias("meta3", "collection1meta,collection2meta");
- aliasesManager.applyModificationAndExportToZk(op);
+ aliasesManager.applyModificationAndExportToZk(a1 ->
+ a1.cloneWithCollectionAlias("meta3", "collection1meta,collection2meta"));
// competing went through
assertEquals("collection1meta,collection2meta", zkStateReader.getAliases().getCollectionAliasMap().get("meta3"));
@@ -240,6 +239,7 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
}
@Test
+ //@BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // 09-Apr-2018
public void testModifyPropertiesV1() throws Exception {
// note we don't use TZ in this test, thus it's UTC
final String aliasName = getTestName();
@@ -255,7 +255,7 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
}
@Test
- @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028")
+ //@BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028")
public void testModifyPropertiesCAR() throws Exception {
// note we don't use TZ in this test, thus it's UTC
final String aliasName = getTestName();
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/566c07f7/solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java b/solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java
index 73be71e..4b81445 100644
--- a/solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java
@@ -99,6 +99,7 @@ public class CreateRoutedAliasTest extends SolrCloudTestCase {
// This is a fairly complete test where we set many options and see that it both affected the created
// collection and that the alias metadata was saved accordingly
@Test
+ //@BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // 09-Apr-2018
public void testV2() throws Exception {
// note we don't use TZ in this test, thus it's UTC
final String aliasName = getTestName();
@@ -180,6 +181,7 @@ public class CreateRoutedAliasTest extends SolrCloudTestCase {
}
@Test
+ //@BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // 09-Apr-2018
public void testV1() throws Exception {
final String aliasName = getTestName();
final String baseUrl = cluster.getRandomJetty(random()).getBaseUrl().toString();
@@ -223,6 +225,7 @@ public class CreateRoutedAliasTest extends SolrCloudTestCase {
// TZ should not affect the first collection name if absolute date given for start
@Test
+ //@BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // 09-Apr-2018
public void testTimezoneAbsoluteDate() throws Exception {
final String aliasName = getTestName();
try (SolrClient client = getCloudSolrClient(cluster)) {
@@ -241,6 +244,7 @@ public class CreateRoutedAliasTest extends SolrCloudTestCase {
}
@Test
+ //@BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // 09-Apr-2018
public void testCollectionNamesMustBeAbsent() throws Exception {
CollectionAdminRequest.createCollection("collection1meta", "_default", 2, 1).process(cluster.getSolrClient());
CollectionAdminRequest.createCollection("collection2meta", "_default", 1, 1).process(cluster.getSolrClient());
@@ -326,6 +330,7 @@ public class CreateRoutedAliasTest extends SolrCloudTestCase {
"&create-collection.numShards=1");
assertFailure(get, "Unit not recognized");
}
+
@Test
public void testNegativeFutureFails() throws Exception {
final String aliasName = getTestName();
@@ -342,6 +347,7 @@ public class CreateRoutedAliasTest extends SolrCloudTestCase {
"&create-collection.numShards=1");
assertFailure(get, "must be >= 0");
}
+
@Test
public void testUnParseableFutureFails() throws Exception {
final String aliasName = "testAlias";
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/566c07f7/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java
index ef8e1a5..cc7b7ce 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java
@@ -37,7 +37,6 @@ import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.ConfigSetAdminRequest;
import org.apache.solr.client.solrj.request.V2Request;
-import org.apache.solr.client.solrj.response.ConfigSetAdminResponse;
import org.apache.solr.client.solrj.response.FieldStatsInfo;
import org.apache.solr.client.solrj.response.QueryResponse;
import org.apache.solr.client.solrj.response.UpdateResponse;
@@ -71,6 +70,11 @@ public class TimeRoutedAliasUpdateProcessorTest extends SolrCloudTestCase {
public static void setupCluster() throws Exception {
configureCluster(2).configure();
solrClient = getCloudSolrClient(cluster);
+ //log this to help debug potential causes of problems
+ System.out.println("SolrClient: " + solrClient);
+ if (solrClient instanceof CloudSolrClient) {
+ System.out.println(((CloudSolrClient)solrClient).getClusterStateProvider());
+ }
}
@AfterClass
@@ -85,16 +89,17 @@ public class TimeRoutedAliasUpdateProcessorTest extends SolrCloudTestCase {
// Then we create a collection with the name of the eventual config.
// We configure it, and ultimately delete the collection, leaving a modified config-set behind.
// Then when we create the "real" collections referencing this modified config-set.
- final ConfigSetAdminRequest.Create adminRequest = new ConfigSetAdminRequest.Create();
- adminRequest.setConfigSetName(configName);
- adminRequest.setBaseConfigSetName("_default");
- ConfigSetAdminResponse adminResponse = adminRequest.process(solrClient);
- assertEquals(adminResponse.getStatus(), 0);
+ assertEquals(0, new ConfigSetAdminRequest.Create()
+ .setConfigSetName(configName)
+ .setBaseConfigSetName("_default")
+ .process(solrClient).getStatus());
- CollectionAdminRequest.createCollection(configName, configName,1, 1).process(solrClient);
- // manipulate the config...
+ CollectionAdminRequest.createCollection(configName, configName, 1, 1).process(solrClient);
- String conf = "{" +
+ // manipulate the config...
+ checkNoError(solrClient.request(new V2Request.Builder("/collections/" + configName + "/config")
+ .withMethod(SolrRequest.METHOD.POST)
+ .withPayload("{" +
" 'set-user-property' : {'update.autoCreateFields':false}," + // no data driven
" 'add-updateprocessor' : {" +
" 'name':'tolerant', 'class':'solr.TolerantUpdateProcessorFactory'" +
@@ -103,10 +108,8 @@ public class TimeRoutedAliasUpdateProcessorTest extends SolrCloudTestCase {
" 'name':'inc', 'class':'" + IncrementURPFactory.class.getName() + "'," +
" 'fieldName':'" + intField + "'" +
" }," +
- "}";
- checkNoError(solrClient.request(new V2Request.Builder("/collections/" + configName + "/config")
- .withMethod(SolrRequest.METHOD.POST)
- .withPayload(conf).build())); // only sometimes test with "tolerant" URP
+ "}").build()));
+ // only sometimes test with "tolerant" URP:
final String urpNames = "inc" + (random().nextBoolean() ? ",tolerant" : "");
checkNoError(solrClient.request(new V2Request.Builder("/collections/" + configName + "/config/params")
.withMethod(SolrRequest.METHOD.POST)
@@ -115,8 +118,8 @@ public class TimeRoutedAliasUpdateProcessorTest extends SolrCloudTestCase {
" '_UPDATE' : {'processor':'" + urpNames + "'}" +
" }" +
"}").build()));
- CollectionAdminRequest.deleteCollection(configName).process(solrClient);
+ CollectionAdminRequest.deleteCollection(configName).process(solrClient);
assertTrue(
new ConfigSetAdminRequest.List().process(solrClient).getConfigSets()
.contains(configName)
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/566c07f7/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
index 8f436d2..b216d01 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
@@ -1638,7 +1638,7 @@ public class ZkStateReader implements Closeable {
LOG.debug(e.toString(), e);
LOG.warn("Couldn't save aliases due to race with another modification; will update and retry until timeout");
// considered a backoff here, but we really do want to compete strongly since the normal case is
- // that we will do one update and succeed. This is left as a hot loop for 5 tries intentionally.
+ // that we will do one update and succeed. This is left as a hot loop for limited tries intentionally.
// More failures than that here probably indicate a bug or a very strange high write frequency usage for
// aliases.json, timeouts mean zk is being very slow to respond, or this node is being crushed
// by other processing and just can't find any cpu cycles at all.
@@ -1666,6 +1666,9 @@ public class ZkStateReader implements Closeable {
* @return true if an update was performed
*/
public boolean update() throws KeeperException, InterruptedException {
+ LOG.debug("Checking ZK for most up to date Aliases " + ALIASES);
+ // Call sync() first to ensure the subsequent read (getData) is up to date.
+ zkClient.getSolrZooKeeper().sync(ALIASES, null, null);
Stat stat = new Stat();
final byte[] data = zkClient.getData(ALIASES, null, stat, true);
return setIfNewer(Aliases.fromJSON(data, stat.getVersion()));