You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by gu...@apache.org on 2023/09/25 00:52:36 UTC
[solr] branch branch_9x updated: SOLR-16194 Don't overwrite list of collections in a Routed Alias when updating parameters. (#864) (#1956)
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 <46...@users.noreply.github.com>
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.