This is an automated email from the ASF dual-hosted git repository.
gus pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new 5653fea0f44 SOLR-16194 Don't overwrite list of collections in a Routed
Alias when updating parameters. (#864) (#1956)
5653fea0f44 is described below
commit 5653fea0f4446c3d1aa04e92e63edd71b5ec8f9d
Author: Gus Heck <[email protected]>
AuthorDate: Sun Sep 24 20:52:31 2023 -0400
SOLR-16194 Don't overwrite list of collections in a Routed Alias when
updating parameters. (#864) (#1956)
(cherry picked from commit adeab2d88f5bbb5bf56d5d4fc5fda349b9c8ee10)
---
.../solr/cloud/api/collections/CreateAliasCmd.java | 31 ++++--
.../java/org/apache/solr/servlet/HttpSolrCall.java | 8 +-
.../apache/solr/cloud/CreateRoutedAliasTest.java | 120 +++++++++++++++++----
.../deployment-guide/pages/alias-management.adoc | 34 +++---
4 files changed, 142 insertions(+), 51 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 58932e3033c..370001c124d 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
@@ -28,6 +28,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;
@@ -54,6 +55,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();
}
@@ -99,10 +101,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 +142,22 @@ public class CreateAliasCmd extends AliasCmd {
.collect(Collectors.joining(",")));
}
- // 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/servlet/HttpSolrCall.java
b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
index 4a9d6c63816..185cf6cb54c 100644
--- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -214,12 +214,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();
@@ -409,7 +403,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 4df4ca61a2f..69f8f1adc1e 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;
@@ -34,9 +36,11 @@ import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.util.EntityUtils;
import org.apache.solr.SolrTestCaseJ4;
+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;
@@ -178,25 +182,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);
@@ -223,6 +210,103 @@ public class CreateRoutedAliasTest extends
SolrCloudTestCase {
assertNull(meta.get("start"));
}
+ @Test
+ public void testUpdateRoutedAliasDoesNotChangeCollectionList() 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
@Test
public void testTimezoneAbsoluteDate() throws Exception {
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 da647ece543..2853a039e5a 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,7 +111,9 @@ 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.
Routed aliases currently support up to two "dimensions" of routing, with each
dimension being either a "time" or "category"-based.
Each dimension takes a number of parameters, which vary based on its type.
@@ -126,7 +128,7 @@ On v2 requests, routing-dimensions are specified as
individual objects within a
+
[%autowidth,frame=none]
|===
-s|Required |Default: none
+s|Required |Default: none |Modify: Do not change after creation
|===
+
The type of routing to use.
@@ -136,15 +138,17 @@ v2 requests only allow `time` or `category` since
dimensionality information liv
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` (v1), `field` (v2)::
+
[%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.
@@ -154,7 +158,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`.
@@ -170,7 +174,7 @@ On v2 requests, `create-collection` takes a JSON object
containing all provided
+
[%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]).
@@ -184,7 +188,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`.
@@ -198,7 +202,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.
@@ -208,7 +212,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.
@@ -218,7 +222,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.
@@ -245,7 +249,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.
@@ -263,7 +267,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.
@@ -273,7 +277,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.
@@ -289,7 +293,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
|===
+
A prefix used on v1 request parameters to associate the parameter with a
particular dimensional, in multi-dimensional aliases.
