This is an automated email from the ASF dual-hosted git repository.
gus pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new adeab2d88f5 SOLR-16194 Don't overwrite list of collections in a Routed
Alias when updating parameters. (#864)
adeab2d88f5 is described below
commit adeab2d88f5bbb5bf56d5d4fc5fda349b9c8ee10
Author: Gus Heck <46900717+gus-...@users.noreply.github.com>
AuthorDate: Wed May 18 00:14:26 2022 -0400
SOLR-16194 Don't overwrite list of collections in a Routed Alias when
updating parameters. (#864)
---
.../solr/cloud/api/collections/CreateAliasCmd.java | 42 ++++---
.../java/org/apache/solr/core/CoreContainer.java | 20 ++++
.../solr/handler/admin/CollectionsHandler.java | 31 +++---
.../java/org/apache/solr/servlet/HttpSolrCall.java | 8 +-
.../apache/solr/cloud/CreateRoutedAliasTest.java | 121 +++++++++++++++++----
.../deployment-guide/pages/alias-management.adoc | 34 +++---
6 files changed, 182 insertions(+), 74 deletions(-)
diff --git
a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateAliasCmd.java
b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateAliasCmd.java
index e4cd3ebb4fd..a346aa1f653 100644
---
a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateAliasCmd.java
+++
b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateAliasCmd.java
@@ -29,6 +29,7 @@ import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.Aliases;
import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.cloud.ZkNodeProps;
import org.apache.solr.common.cloud.ZkStateReader;
@@ -55,6 +56,7 @@ public class CreateAliasCmd extends AliasCmd {
final String aliasName = message.getStr(CommonParams.NAME);
ZkStateReader zkStateReader = ccc.getZkStateReader();
// make sure we have the latest version of existing aliases
+ //noinspection ConstantConditions
if (zkStateReader.aliasesManager != null) { // not a mock ZkStateReader
zkStateReader.aliasesManager.update();
}
@@ -72,12 +74,13 @@ public class CreateAliasCmd extends AliasCmd {
// Solr's view of the cluster is eventually consistent. *Eventually* all
nodes and
// CloudSolrClients will be aware of alias changes, but not immediately.
If a newly created
// alias is queried, things should work right away since Solr will attempt
to see if it needs to
- // get the latest aliases when it can't otherwise resolve the name.
However modifications to an
- // alias will take some time.
+ // get the latest aliases when it can't otherwise resolve the name.
However, modifications to
+ // an alias will take some time.
//
- // We could levy this requirement on the client but they would probably
always add an obligatory
- // sleep, which is just kicking the can down the road. Perhaps ideally at
this juncture here we
- // could somehow wait until all Solr nodes in the cluster have the latest
aliases?
+ // We could levy this requirement on the client, but they would probably
always add an
+ // obligatory sleep, which is just kicking the can down the road. Perhaps
ideally at this
+ // juncture here we could somehow wait until all Solr nodes in the cluster
have the
+ // latest aliases?
Thread.sleep(100);
}
@@ -100,10 +103,10 @@ public class CreateAliasCmd extends AliasCmd {
* "b"]). We also maintain support for the legacy format, a comma-separated
list (e.g. a,b).
*/
@SuppressWarnings("unchecked")
- private List<String> parseCollectionsParameter(Object colls) {
- if (colls == null) throw new SolrException(BAD_REQUEST, "missing
collections param");
- if (colls instanceof List) return (List<String>) colls;
- return StrUtils.splitSmart(colls.toString(), ",", true).stream()
+ private List<String> parseCollectionsParameter(Object collections) {
+ if (collections == null) throw new SolrException(BAD_REQUEST, "missing
collections param");
+ if (collections instanceof List) return (List<String>) collections;
+ return StrUtils.splitSmart(collections.toString(), ",", true).stream()
.map(String::trim)
.filter(s -> !s.isEmpty())
.collect(Collectors.toList());
@@ -140,15 +143,22 @@ public class CreateAliasCmd extends AliasCmd {
Sets.difference(routedAlias.getRequiredParams(),
props.keySet()), ','));
}
- // Create the first collection.
- String initialColl = routedAlias.computeInitialCollectionName();
- ensureAliasCollection(
- aliasName, zkStateReader, state, routedAlias.getAliasMetadata(),
initialColl);
+ Aliases aliases = zkStateReader.aliasesManager.getAliases();
+
+ final String collectionListStr;
+ if (!aliases.isRoutedAlias(aliasName)) {
+ // Create the first collection. Prior validation ensures that this is
not a standard alias
+ collectionListStr = routedAlias.computeInitialCollectionName();
+ ensureAliasCollection(
+ aliasName, zkStateReader, state, routedAlias.getAliasMetadata(),
collectionListStr);
+ } else {
+ List<String> collectionList = aliases.resolveAliases(aliasName);
+ collectionListStr = String.join(",", collectionList);
+ }
// Create/update the alias
zkStateReader.aliasesManager.applyModificationAndExportToZk(
- aliases ->
- aliases
- .cloneWithCollectionAlias(aliasName, initialColl)
+ a ->
+ a.cloneWithCollectionAlias(aliasName, collectionListStr)
.cloneWithCollectionAliasProperties(aliasName,
routedAlias.getAliasMetadata()));
}
diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
index b04f824597d..b32f8980faa 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -79,6 +79,7 @@ import
org.apache.solr.cluster.placement.impl.PlacementPluginFactoryLoader;
import org.apache.solr.common.AlreadyClosedException;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.cloud.Aliases;
import org.apache.solr.common.cloud.DocCollection;
import org.apache.solr.common.cloud.Replica;
import org.apache.solr.common.cloud.Replica.State;
@@ -2299,6 +2300,25 @@ public class CoreContainer {
return status;
}
+ /**
+ * Retrieve the aliases from zookeeper. This is typically cached and does
not hit zookeeper after
+ * the first use.
+ *
+ * @return an immutable instance of {@code Aliases} accurate as of at the
time this method is
+ * invoked, less any zookeeper update lag.
+ * @throws RuntimeException if invoked on a {@code CoreContainer} where
{@link
+ * #isZooKeeperAware()} returns false
+ */
+ public Aliases getAliases() {
+ if (isZooKeeperAware()) {
+ return getZkController().getZkStateReader().getAliases();
+ } else {
+ // fail fast because it's programmer error, but give slightly more info
than NPE.
+ throw new IllegalStateException(
+ "Aliases don't exist in a non-cloud context, check
isZookeeperAware() before calling this method.");
+ }
+ }
+
// Occasionally we need to access the transient cache handler in places
other than coreContainer.
public TransientSolrCoreCache getTransientCache() {
return solrCores.getTransientCacheHandler();
diff --git
a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
index dd0881295ca..cab32779b7c 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
@@ -599,12 +599,7 @@ public class CollectionsHandler extends RequestHandlerBase
implements Permission
SYNCSHARD,
(req, rsp, h) -> {
String extCollection = req.getParams().required().get("collection");
- String collection =
- h.coreContainer
- .getZkController()
- .getZkStateReader()
- .getAliases()
- .resolveSimpleAlias(extCollection);
+ String collection =
h.coreContainer.getAliases().resolveSimpleAlias(extCollection);
String shard = req.getParams().required().get("shard");
ClusterState clusterState =
h.coreContainer.getZkController().getClusterState();
@@ -669,6 +664,17 @@ public class CollectionsHandler extends RequestHandlerBase
implements Permission
//////////////////////////////////////
return copy(finalParams.required(), null, NAME, "collections");
}
+ } else {
+ if (routedAlias != null) {
+ CoreContainer coreContainer1 = h.getCoreContainer();
+ Aliases aliases = coreContainer1.getAliases();
+ String aliasName = routedAlias.getAliasName();
+ if (aliases.hasAlias(aliasName) &&
!aliases.isRoutedAlias(aliasName)) {
+ throw new SolrException(
+ BAD_REQUEST,
+ "Cannot add routing parameters to existing non-routed
Alias: " + aliasName);
+ }
+ }
}
/////////////////////////////////////////////////
@@ -929,12 +935,7 @@ public class CollectionsHandler extends RequestHandlerBase
implements Permission
COLLECTIONPROP,
(req, rsp, h) -> {
String extCollection = req.getParams().required().get(NAME);
- String collection =
- h.coreContainer
- .getZkController()
- .getZkStateReader()
- .getAliases()
- .resolveSimpleAlias(extCollection);
+ String collection =
h.coreContainer.getAliases().resolveSimpleAlias(extCollection);
String name = req.getParams().required().get(PROPERTY_NAME);
String val = req.getParams().get(PROPERTY_VALUE);
CollectionProperties cp =
@@ -1356,11 +1357,7 @@ public class CollectionsHandler extends
RequestHandlerBase implements Permission
final String collectionName =
SolrIdentifierValidator.validateCollectionName(req.getParams().get(COLLECTION_PROP));
- if (h.coreContainer
- .getZkController()
- .getZkStateReader()
- .getAliases()
- .hasAlias(collectionName)) {
+ if (h.coreContainer.getAliases().hasAlias(collectionName)) {
throw new SolrException(
ErrorCode.BAD_REQUEST,
"Collection '" + collectionName + "' is an existing alias, no
action taken.");
diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
index 50b0527f1f4..3de3d919e04 100644
--- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -221,12 +221,6 @@ public class HttpSolrCall {
return queryParams;
}
- protected Aliases getAliases() {
- return cores.isZooKeeperAware()
- ? cores.getZkController().getZkStateReader().getAliases()
- : Aliases.EMPTY;
- }
-
/** The collection(s) referenced in this request. Populated in {@link
#init()}. Not null. */
public List<String> getCollectionsList() {
return collectionsList != null ? collectionsList : Collections.emptyList();
@@ -416,7 +410,7 @@ public class HttpSolrCall {
}
List<String> result = null;
LinkedHashSet<String> uniqueList = null;
- Aliases aliases = getAliases();
+ Aliases aliases = cores.getAliases();
List<String> inputCollections = StrUtils.splitSmart(collectionStr, ",",
true);
if (inputCollections.size() > 1) {
uniqueList = new LinkedHashSet<>();
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 7f3a8722fb2..b5cb4127609 100644
--- a/solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java
@@ -20,9 +20,11 @@ package org.apache.solr.cloud;
import static org.apache.solr.client.solrj.RoutedAliasTypes.TIME;
import java.io.IOException;
+import java.time.Duration;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Date;
+import java.util.List;
import java.util.Map;
import java.util.TimeZone;
import org.apache.http.client.methods.CloseableHttpResponse;
@@ -39,6 +41,7 @@ import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.CloudLegacySolrClient;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.response.CollectionAdminResponse;
import org.apache.solr.cloud.api.collections.TimeRoutedAlias;
import org.apache.solr.common.cloud.Aliases;
import org.apache.solr.common.cloud.CompositeIdRouter;
@@ -189,25 +192,8 @@ public class CreateRoutedAliasTest extends
SolrCloudTestCase {
@Test
public void testV1() throws Exception {
final String aliasName = getSaferTestName();
- final String baseUrl =
cluster.getRandomJetty(random()).getBaseUrl().toString();
Instant start = Instant.now().truncatedTo(ChronoUnit.HOURS); // mostly
make sure no millis
- HttpGet get =
- new HttpGet(
- baseUrl
- + "/admin/collections?action=CREATEALIAS"
- + "&wt=xml"
- + "&name="
- + aliasName
- + "&router.field=evt_dt"
- + "&router.name=time"
- + "&router.start="
- + start
- + "&router.interval=%2B30MINUTE"
- + "&create-collection.collection.configName=_default"
- + "&create-collection.router.field=foo_s"
- + "&create-collection.numShards=1"
- + "&create-collection.replicationFactor=2");
- assertSuccess(get);
+ createTRAv1(aliasName, start);
String initialCollectionName =
TimeRoutedAlias.formatCollectionNameFromInstant(aliasName, start);
@@ -231,7 +217,104 @@ public class CreateRoutedAliasTest extends
SolrCloudTestCase {
assertNotNull(meta);
assertEquals("evt_dt", meta.get("router.field"));
assertEquals("_default",
meta.get("create-collection.collection.configName"));
- assertEquals(null, meta.get("start"));
+ assertNull(meta.get("start"));
+ }
+
+ @Test
+ public void testUpdateRoudetedAliasDoesNotChangeCollectionList() throws
Exception {
+
+ final String aliasName = getSaferTestName();
+ Instant start = Instant.now().truncatedTo(ChronoUnit.HOURS); // mostly
make sure no millis
+ createTRAv1(aliasName, start);
+
+ String initialCollectionName =
+ TimeRoutedAlias.formatCollectionNameFromInstant(aliasName, start);
+ assertCollectionExists(initialCollectionName);
+
+ // Note that this is convenient for the test because it implies a
different collection name, but
+ // doing this is an advanced operation, typically preceded by manual
collection creations and
+ // manual tweaking of the collection list. This is here merely to test
that we don't blow away
+ // the existing (possibly tweaked) list. DO NOT use this as an example of
normal operations.
+ Instant earlierStart = start.minus(Duration.ofMinutes(3));
+ createTRAv1(aliasName, earlierStart);
+ assertCollectionExists(initialCollectionName);
+
+ // Test Alias metadata
+ Aliases aliases = cluster.getZkStateReader().getAliases();
+ Map<String, String> collectionAliasMap = aliases.getCollectionAliasMap();
+ String alias = collectionAliasMap.get(aliasName);
+ assertNotNull(alias);
+ Map<String, String> meta = aliases.getCollectionAliasProperties(aliasName);
+ assertNotNull(meta);
+ assertEquals("evt_dt", meta.get("router.field"));
+ assertEquals("_default",
meta.get("create-collection.collection.configName"));
+
+ // This should be equal to the new start value
+ assertEquals(earlierStart.toString(), meta.get("router.start"));
+ List<String> collectionList = aliases.resolveAliases(aliasName);
+ assertEquals(1, collectionList.size());
+ assertTrue(collectionList.contains(initialCollectionName));
+ }
+
+ public void testCantAddRoutingToNonRouted() throws Exception {
+ String aliasName = getSaferTestName() + "Alias";
+ createCollection();
+ final String baseUrl =
cluster.getRandomJetty(random()).getBaseUrl().toString();
+ HttpGet get =
+ new HttpGet(
+ baseUrl
+ + "/admin/collections?action=CREATEALIAS"
+ + "&wt=xml"
+ + "&name="
+ + aliasName
+ + "&collections="
+ + getSaferTestName());
+ assertSuccess(get);
+
+ HttpGet get2 =
+ new HttpGet(
+ baseUrl
+ + "/admin/collections?action=CREATEALIAS"
+ + "&wt=json"
+ + "&name="
+ + aliasName
+ + "&router.field=evt_dt"
+ + "&router.name=time"
+ + "&router.start=2018-01-15T00:00:00Z"
+ + "&router.interval=%2B30MINUTE"
+ + "&create-collection.collection.configName=_default"
+ + "&create-collection.numShards=1");
+ assertFailure(get2, "Cannot add routing parameters to existing non-routed
Alias");
+ }
+
+ private void createCollection() throws SolrServerException, IOException {
+ final CollectionAdminResponse response =
+ CollectionAdminRequest.createCollection(getSaferTestName(),
"_default", 1, 1)
+ .process(solrClient);
+ if (response.getStatus() != 0) {
+ fail("failed to create collection " + getSaferTestName());
+ }
+ }
+
+ private void createTRAv1(String aliasName, Instant start) throws IOException
{
+ final String baseUrl =
cluster.getRandomJetty(random()).getBaseUrl().toString();
+ HttpGet get =
+ new HttpGet(
+ baseUrl
+ + "/admin/collections?action=CREATEALIAS"
+ + "&wt=xml"
+ + "&name="
+ + aliasName
+ + "&router.field=evt_dt"
+ + "&router.name=time"
+ + "&router.start="
+ + start
+ + "&router.interval=%2B30MINUTE"
+ + "&create-collection.collection.configName=_default"
+ + "&create-collection.router.field=foo_s"
+ + "&create-collection.numShards=1"
+ + "&create-collection.replicationFactor=2");
+ assertSuccess(get);
}
// TZ should not affect the first collection name if absolute date given for
start
diff --git
a/solr/solr-ref-guide/modules/deployment-guide/pages/alias-management.adoc
b/solr/solr-ref-guide/modules/deployment-guide/pages/alias-management.adoc
index 15fdc27912e..c01bf492bd3 100644
--- a/solr/solr-ref-guide/modules/deployment-guide/pages/alias-management.adoc
+++ b/solr/solr-ref-guide/modules/deployment-guide/pages/alias-management.adoc
@@ -111,13 +111,15 @@ If routing parameters are present this parameter is
prohibited.
==== Routed Alias Parameters
-Most routed alias parameters become _alias properties_ that can subsequently
be inspected and <<aliasprop,modified>>.
+Most routed alias parameters become _alias properties_ that can subsequently
be inspected and modified either by issuing a new CREATEALIAS for the same name
or via <<aliasprop,ALIASPROP>>.
+CREATEALIAS will validate against many (but not all) bad values, whereas
ALIASPROP blindly accepts any key or value you give it.
+Some "valid" modifications allowed by CREATEALIAS may still be unwise, see
notes below. "Expert only" modifications are technically possible, but require
good understanding of how the code works and may require several precursor
operations.
`router.name`::
+
[%autowidth,frame=none]
|===
-s|Required |Default: none
+s|Required |Default: none |Modify: Do not change after creation
|===
+
The type of routing to use.
@@ -126,15 +128,17 @@ Presently only `time` and `category` and `Dimensional[]`
are valid.
In the case of a
xref:aliases.adoc#dimensional-routed-aliases[multi-dimensional routed alias]
(aka "DRA"), it is required to express all the dimensions in the same order
that they will appear in the dimension
array.
The format for a DRA `router.name` is `Dimensional[dim1,dim2]` where `dim1`
and `dim2` are valid `router.name` values for each sub-dimension.
-Note that DRA's are very new, and only 2D DRA's are presently supported.
-Higher numbers of dimensions will be supported soon.
+Note that DRA's are experimental, and only 2D DRA's are presently supported.
+Higher numbers of dimensions may be supported in the future.
+Careful design of dimensional routing is required to avoid an explosion in the
number of collections in the cluster.
+Solr Cloud may have difficulty managing more than a thousand collections.
See examples below for further clarification on how to configure individual
dimensions.
`router.field`::
+
[%autowidth,frame=none]
|===
-s|Required |Default: none
+s|Required |Default: none |Modify: Do not change after creation
|===
+
The field to inspect to determine which underlying collection an incoming
document should be routed to.
@@ -144,7 +148,7 @@ This field is required on all incoming documents.
+
[%autowidth,frame=none]
|===
-|Optional |Default: none
+|Optional |Default: none | Modify: Yes, only new collections affected, use
with care
|===
+
The `*` wildcard can be replaced with any parameter from the
xref:collection-management.adoc#create[CREATE] command except `name`.
@@ -158,7 +162,7 @@ It's probably a bad idea to use "data driven" mode as
schema mutations might hap
+
[%autowidth,frame=none]
|===
-s|Required |Default: none
+s|Required |Default: none | Modify: Expert only
|===
+
The start date/time of data for this time routed alias in Solr's standard
date/time format (i.e., ISO-8601 or "NOW" optionally with
xref:indexing-guide:date-formatting-math.adoc#date-math[date math]).
@@ -172,7 +176,7 @@ Particularly, this means `NOW` will fail 999 times out of
1000, though `NOW/SECO
+
[%autowidth,frame=none]
|===
-|Optional |Default: `UTC`
+|Optional |Default: `UTC` | Modify: Expert only
|===
+
The timezone to be used when evaluating any date math in `router.start` or
`router.interval`.
@@ -186,7 +190,7 @@ If GMT-4 is supplied for this value then a document dated
2018-01-14T21:00:00:01
+
[%autowidth,frame=none]
|===
-s|Required |Default: none
+s|Required |Default: none | Modify: Yes
|===
+
A date math expression that will be appended to a timestamp to determine the
next collection in the series.
@@ -196,7 +200,7 @@ Any date math expression that can be evaluated if appended
to a timestamp of the
+
[%autowidth,frame=none]
|===
-|Optional |Default: `600000` milliseconds
+|Optional |Default: `600000` milliseconds | Modify: Yes
|===
+
The maximum milliseconds into the future that a document is allowed to have in
`router.field` for it to be accepted without error.
@@ -206,7 +210,7 @@ If there was no limit, then an erroneous value could
trigger many collections to
+
[%autowidth,frame=none]
|===
-|Optional |Default: none
+|Optional |Default: none | Modify: Yes
|===
+
A date math expression that results in early creation of new collections.
@@ -233,7 +237,7 @@ This property is empty by default indicating just-in-time,
synchronous creation
+
[%autowidth,frame=none]
|===
-|Optional |Default: none
+|Optional |Default: none | Modify: Yes, Possible data loss, use with care!
|===
+
A date math expression that results in the oldest collections getting deleted
automatically.
@@ -251,7 +255,7 @@ The default is not to delete.
+
[%autowidth,frame=none]
|===
-|Optional |Default: none
+|Optional |Default: none | Modify: Yes
|===
+
The maximum number of categories allowed for this alias.
@@ -261,7 +265,7 @@ This setting safeguards against the inadvertent creation of
an infinite number o
+
[%autowidth,frame=none]
|===
-|Optional |Default: none
+|Optional |Default: none | Modify: Yes
|===
+
A regular expression that the value of the field specified by `router.field`
must match before a corresponding collection will be created.
@@ -277,7 +281,7 @@ Overly complex patterns will produce CPU or garbage
collection overhead during i
+
[%autowidth,frame=none]
|===
-|Optional |Default: none
+|Optional |Default: none | Modify: As per above
|===
+
This prefix denotes which position in the dimension array is being referred to
for purposes of dimension configuration.
