diff --git a/changelog/unreleased/SOLR-13696-dimensional-routed-alias.yml b/changelog/unreleased/SOLR-13696-dimensional-routed-alias.yml new file mode 100644 index 000000000000..c3b42b1f6699 --- /dev/null +++ b/changelog/unreleased/SOLR-13696-dimensional-routed-alias.yml @@ -0,0 +1,7 @@ +title: Creating a dimensional routed alias through the V2 collections API no longer fails for missing router parameters, and time routed alias routing no longer throws a ClassCastException when a document timestamp is still a String +type: fixed +authors: + - name: Nick Shanin +links: + - name: SOLR-13696 + url: https://issues.apache.org/jira/browse/SOLR-13696 diff --git a/changelog/unreleased/SOLR-13943.yml b/changelog/unreleased/SOLR-13943.yml new file mode 100644 index 000000000000..17e1e2f0e17d --- /dev/null +++ b/changelog/unreleased/SOLR-13943.yml @@ -0,0 +1,7 @@ +title: "Fix the race in TimeRoutedAliasUpdateProcessorTest.testDateMathInStart: poll the cluster state provider for the rewritten router.start instead of reading it once after the test's own ZooKeeper watcher fires" +type: fixed +authors: + - name: Nick Shanin +links: + - name: SOLR-13943 + url: https://issues.apache.org/jira/browse/SOLR-13943 diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/RoutedAlias.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/RoutedAlias.java index 4e2ab6f591b3..49cbb049fe99 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/RoutedAlias.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/RoutedAlias.java @@ -44,6 +44,7 @@ import org.apache.solr.handler.admin.CollectionsHandler; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.update.AddUpdateCommand; +import org.apache.solr.util.TestInjection; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -433,7 +434,11 @@ private String doPreemptive( private void preemptiveAsync(Runnable r, SolrCore core) { preemptiveCreateOnceAlready = true; - core.runAsync(r); + core.runAsync( + () -> { + TestInjection.injectPreemptiveCreateLatch(); + r.run(); + }); } private SolrException unknownCreateType() { diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/TimeRoutedAlias.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/TimeRoutedAlias.java index 4df16b611d20..ddc41b41b915 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/TimeRoutedAlias.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/TimeRoutedAlias.java @@ -218,8 +218,8 @@ public String computeInitialCollectionName() { @Override String[] formattedRouteValues(SolrInputDocument doc) { String routeField = getRouteField(); - Date fieldValue = (Date) doc.getFieldValue(routeField); - String dest = calcCandidateCollection(fieldValue.toInstant()).getDestinationCollection(); + Instant fieldValue = parseRouteKey(doc.getFieldValue(routeField)); + String dest = calcCandidateCollection(fieldValue).getDestinationCollection(); int nonValuePrefix = getAliasName().length() + getRoutedAliasType().getSeparatorPrefix().length(); return new String[] {dest.substring(nonValuePrefix)}; diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/CreateAlias.java b/solr/core/src/java/org/apache/solr/handler/admin/api/CreateAlias.java index ac054f118b77..3730aa9f681f 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/CreateAlias.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/CreateAlias.java @@ -20,6 +20,8 @@ import static org.apache.solr.client.solrj.request.beans.V2ApiConstants.COLLECTIONS; import static org.apache.solr.cloud.api.collections.RoutedAlias.CATEGORY; import static org.apache.solr.cloud.api.collections.RoutedAlias.CREATE_COLLECTION_PREFIX; +import static org.apache.solr.cloud.api.collections.RoutedAlias.DIMENSIONAL; +import static org.apache.solr.cloud.api.collections.RoutedAlias.ROUTER_FIELD; import static org.apache.solr.cloud.api.collections.RoutedAlias.ROUTER_TYPE_NAME; import static org.apache.solr.cloud.api.collections.RoutedAlias.TIME; import static org.apache.solr.cloud.api.collections.TimeRoutedAlias.ROUTER_MAX_FUTURE; @@ -35,6 +37,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Locale; import java.util.Map; import org.apache.solr.client.api.endpoint.CreateAliasApi; @@ -123,10 +126,19 @@ public static ZkNodeProps createRemoteMessageForRoutedAlias(CreateAliasRequestBo remoteMessage.put(NAME, requestBody.name); if (requestBody.routers.size() > 1) { // Multi-dimensional alias + final List dimensionTypes = new ArrayList<>(); + final List dimensionFields = new ArrayList<>(); for (int i = 0; i < requestBody.routers.size(); i++) { - createValidationHelper(requestBody.routers.get(i)) - .addRemoteMessageProperties(remoteMessage, "router." + i + "."); + final RoutedAliasProperties router = requestBody.routers.get(i); + final RoutedAliasValidationHelper helper = createValidationHelper(router); + helper.addRemoteMessageProperties(remoteMessage, "router." + i + "."); + dimensionTypes.add(helper.getRouterType().name()); + dimensionFields.add(router.field); } + // The overseer also requires the top level router name and field, in the same shape + // that SolrJ sends for a dimensional alias. + remoteMessage.put(ROUTER_TYPE_NAME, DIMENSIONAL + String.join(",", dimensionTypes) + "]"); + remoteMessage.put(ROUTER_FIELD, String.join(",", dimensionFields)); } else if (requestBody.routers.size() == 1) { // Single dimensional alias createValidationHelper(requestBody.routers.get(0)) .addRemoteMessageProperties(remoteMessage, "router."); @@ -238,6 +250,8 @@ private interface RoutedAliasValidationHelper { void addRemoteMessageProperties(Map remoteMessage, String prefix); + RoutedAliasTypes getRouterType(); + default void ensureRequiredFieldPresent(Object val, String name) { if (val == null) { throw new SolrException( @@ -282,6 +296,11 @@ public void validate() { } } + @Override + public RoutedAliasTypes getRouterType() { + return RoutedAliasTypes.TIME; + } + @Override public void addRemoteMessageProperties(Map remoteMessage, String prefix) { remoteMessage.put(prefix + CoreAdminParams.NAME, TIME); @@ -327,6 +346,11 @@ public void validate() { ensureRequiredFieldPresent(aliasProperties.field, "'field' on category routed alias"); } + @Override + public RoutedAliasTypes getRouterType() { + return RoutedAliasTypes.CATEGORY; + } + @Override public void addRemoteMessageProperties(Map remoteMessage, String prefix) { remoteMessage.put(prefix + CoreAdminParams.NAME, CATEGORY); diff --git a/solr/core/src/java/org/apache/solr/util/TestInjection.java b/solr/core/src/java/org/apache/solr/util/TestInjection.java index 3ca9e5d78a6e..9446b74cc80d 100644 --- a/solr/core/src/java/org/apache/solr/util/TestInjection.java +++ b/solr/core/src/java/org/apache/solr/util/TestInjection.java @@ -143,6 +143,8 @@ static Random random() { // non-private for testing public static volatile CountDownLatch reindexLatch = null; + public static volatile CountDownLatch preemptiveCreateLatch = null; + public static volatile String reindexFailure = null; public static volatile String failIndexFingerprintRequests = null; @@ -213,6 +215,7 @@ public static void reset() { splitLatch = null; directUpdateLatch = null; reindexLatch = null; + preemptiveCreateLatch = null; reindexFailure = null; prepRecoveryOpPauseForever = null; countPrepRecoveryOpPauseForever = new AtomicInteger(0); @@ -631,6 +634,17 @@ public static boolean injectReindexLatch() { return true; } + public static void injectPreemptiveCreateLatch() { + CountDownLatch latch = preemptiveCreateLatch; + if (latch != null) { + try { + latch.await(30, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } + private static Pair parseValue(final String raw) { if (raw == null) return new Pair<>(false, 0); Matcher m = ENABLED_PERCENT.matcher(raw); diff --git a/solr/core/src/test/org/apache/solr/handler/admin/api/CreateAliasAPITest.java b/solr/core/src/test/org/apache/solr/handler/admin/api/CreateAliasAPITest.java index d44152becaa7..4a20b0778c88 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/api/CreateAliasAPITest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/api/CreateAliasAPITest.java @@ -305,8 +305,10 @@ public void testRemoteMessageCreationForMultiDimensionalRoutedAlias() throws Exc validateRunCommand( CollectionParams.CollectionAction.CREATEALIAS, message -> { - assertEquals(10, message.size()); + assertEquals(12, message.size()); assertEquals("someAliasName", message.get("name")); + assertEquals("Dimensional[TIME,CATEGORY]", message.get("router.name")); + assertEquals("someField,someField", message.get("router.field")); assertEquals("time", message.get("router.0.name")); assertEquals("someField", message.get("router.0.field")); assertEquals("NOW/HOUR", message.get("router.0.start")); diff --git a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java index 6b3ddf6b9d20..0db706c13f7d 100644 --- a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java +++ b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java @@ -50,7 +50,7 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro "Constructor", "Heart of Gold", "Stunt Ship", "B-ark", "Bi$tromath" }; - private static final String categoryField = "ship_name_en"; + private static final String categoryField = "ship_name_s"; private static final String intField = "integer_i"; private int lastDocId = 0; diff --git a/solr/core/src/test/org/apache/solr/update/processor/DimensionalRoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/DimensionalRoutedAliasUpdateProcessorTest.java index e7166b159739..26000cec1fdc 100644 --- a/solr/core/src/test/org/apache/solr/update/processor/DimensionalRoutedAliasUpdateProcessorTest.java +++ b/solr/core/src/test/org/apache/solr/update/processor/DimensionalRoutedAliasUpdateProcessorTest.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.lang.invoke.MethodHandles; import java.time.Instant; +import java.time.temporal.ChronoUnit; import java.util.Collections; import java.util.Date; import java.util.List; @@ -127,7 +128,7 @@ public void testTimeCat() throws Exception { ap("tabby", "calico")); testFailedDocument("shorthair", "2017-10-23T00:00:00Z", "couldn't be routed"); - testFailedDocument("shorthair", "2020-10-23T00:00:00Z", "too far in the future"); + testFailedDocument("shorthair", tooFarInTheFuture(), "too far in the future"); testFailedDocument(null, "2019-07-02T00:00:00Z", "Route value is null"); testFailedDocument("foo__CRA__bar", "2019-07-02T00:00:00Z", "7 character sequence __CRA__"); testFailedDocument("fóóCRAóóbar", "2019-07-02T00:00:00Z", "7 character sequence __CRA__"); @@ -278,18 +279,22 @@ Here we need to be testing that something that should not be created (extra pree ap("shorthair", "tabby", "calico")); // verify that all the documents ended up in the right collections. + // [shard] is requested with style='urls' because the replica core URLs + // contain the collection name, which encodes the category and the day + // each document should have been routed to. The default [shard] style + // returns only the shard id, which names neither. QueryResponse resp = solrClient.query( getAlias(), params( "q", "*:*", "rows", "100", - "fl", "*,[shard]", + "fl", "*,shard:[shard style='urls']", "sort", "id asc")); SolrDocumentList results = resp.getResults(); assertEquals(18, results.getNumFound()); for (SolrDocument result : results) { - String shard = String.valueOf(result.getFieldValue("[shard]")); + String shard = String.valueOf(result.getFieldValue("shard")); String cat = String.valueOf(result.getFieldValue("cat_s")); Date date = (Date) result.getFieldValue("timestamp_dt"); String day = date.toInstant().toString().split("T")[0]; @@ -349,7 +354,7 @@ public void testCatTime() throws Exception { ap("tabby", "calico")); testFailedDocument("shorthair", "2017-10-23T00:00:00Z", "couldn't be routed"); - testFailedDocument("shorthair", "2020-10-23T00:00:00Z", "too far in the future"); + testFailedDocument("shorthair", tooFarInTheFuture(), "too far in the future"); testFailedDocument(null, "2019-07-02T00:00:00Z", "Route value is null"); testFailedDocument("foo__CRA__bar", "2019-07-02T00:00:00Z", "7 character sequence __CRA__"); testFailedDocument("fóóCRAóóbar", "2019-07-02T00:00:00Z", "7 character sequence __CRA__"); @@ -502,18 +507,22 @@ Here we need to be testing that something that should not be created (extra pree ap("shorthair", "tabby", "calico")); // verify that all the documents ended up in the right collections. + // [shard] is requested with style='urls' because the replica core URLs + // contain the collection name, which encodes the category and the day + // each document should have been routed to. The default [shard] style + // returns only the shard id, which names neither. QueryResponse resp = solrClient.query( getAlias(), params( "q", "*:*", "rows", "100", - "fl", "*,[shard]", + "fl", "*,shard:[shard style='urls']", "sort", "id asc")); SolrDocumentList results = resp.getResults(); assertEquals(18, results.getNumFound()); for (SolrDocument result : results) { - String shard = String.valueOf(result.getFieldValue("[shard]")); + String shard = String.valueOf(result.getFieldValue("shard")); String cat = String.valueOf(result.getFieldValue("cat_s")); Date date = (Date) result.getFieldValue("timestamp_dt"); String day = date.toInstant().toString().split("T")[0]; @@ -603,6 +612,14 @@ private void assertCatTimeInvariants(String[] expectedCols, String[] categories) assertEquals("COLS FOUND:" + cols, expectedCols.length, cols.size()); } + // Computed relative to now so the document is always beyond router.maxFutureMs and is + // rejected by TimeRoutedAlias.validateRouteValue. A fixed "future" date eventually becomes + // the past; the document is then routed instead, and creating the collections between the + // alias start and that date one day at a time stalls the update until it times out. + private static String tooFarInTheFuture() { + return Instant.now().plus(30, ChronoUnit.DAYS).toString(); + } + private void testFailedDocument(String category, String timestamp, String errorMsg) throws SolrServerException, IOException { try { diff --git a/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java index 888cba4c4ffc..afb26694a28d 100644 --- a/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java +++ b/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java @@ -31,8 +31,6 @@ import java.util.TreeSet; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; -import java.util.stream.Collectors; -import org.apache.lucene.tests.util.LuceneTestCase.AwaitsFix; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.impl.CloudSolrClient; @@ -60,17 +58,29 @@ import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; import org.apache.solr.update.UpdateCommand; +import org.junit.After; import org.junit.Ignore; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -@AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-13696") @Ignore // don't try to run abstract base class public abstract class RoutedAliasUpdateProcessorTest extends SolrCloudTestCase { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private static final String intField = "integer_i"; + /** + * The subclasses create a fresh cluster in every test method (their {@code @Before} calls {@code + * configureCluster}), because each class routes to a fixed alias name whose collections cannot be + * shared between methods. The framework only shuts down the cluster still referenced when the + * class ends, so each method's cluster is shut down here; otherwise the earlier clusters stay + * open and their cores, clients and directories fail the suite's resource tracking checks. + */ + @After + public void shutdownMethodCluster() throws Exception { + shutdownCluster(); + } + void waitColAndAlias(String alias, String separator, final String suffix, int slices) throws InterruptedException { // collection to exist @@ -317,8 +327,6 @@ void addDocsAndCommit(boolean aliasOnly, SolrInputDocument... solrInputDocuments .get(getAlias())); } - int commitWithin = random().nextBoolean() ? -1 : 500; // if -1, we commit explicitly instead - if (random().nextBoolean()) { // Send in separate threads. Choose random collection & solrClient ExecutorService exec = null; @@ -330,7 +338,7 @@ void addDocsAndCommit(boolean aliasOnly, SolrInputDocument... solrInputDocuments List> futures = new ArrayList<>(solrInputDocuments.length); for (SolrInputDocument solrInputDocument : solrInputDocuments) { String col = collections.get(random().nextInt(collections.size())); - futures.add(exec.submit(() -> solrClient.add(col, solrInputDocument, commitWithin))); + futures.add(exec.submit(() -> solrClient.add(col, solrInputDocument))); } for (Future future : futures) { assertUpdateResponse(future.get()); @@ -347,33 +355,18 @@ void addDocsAndCommit(boolean aliasOnly, SolrInputDocument... solrInputDocuments // send in a batch. String col = collections.get(random().nextInt(collections.size())); CloudSolrClient solrClient = cluster.getSolrClient(); - assertUpdateResponse(solrClient.add(col, Arrays.asList(solrInputDocuments), commitWithin)); + assertUpdateResponse(solrClient.add(col, Arrays.asList(solrInputDocuments))); } - String col = collections.get(random().nextInt(collections.size())); - if (commitWithin == -1) { + // The docs may have been routed to any collection of the alias, so commit each of them. + // A commit waits for every replica to open a new searcher. commitWithin does not. + List aliasCollections = + new CollectionAdminRequest.ListAliases() + .process(getSolrClient()) + .getAliasesAsLists() + .get(getAlias()); + assertNotNull("alias " + getAlias() + " is not listed", aliasCollections); + for (String col : aliasCollections) { getSolrClient().commit(col); - } else { - // check that it all got committed eventually - String docsQ = - "{!terms f=id}" - + Arrays.stream(solrInputDocuments) - .map(d -> d.getFieldValue("id").toString()) - .collect(Collectors.joining(",")); - int numDocs = queryNumDocs(docsQ); - if (numDocs == solrInputDocuments.length) { - System.err.println("Docs committed sooner than expected. Bug or slow test env?"); - return; - } - // wait until it's committed - Thread.sleep(commitWithin); - // Loop for up to 10 seconds waiting for commit to catch up - for (int idx = 0; idx < 100; ++idx) { - numDocs = queryNumDocs(docsQ); - if (numDocs == solrInputDocuments.length) break; - Thread.sleep(100); - } - - assertEquals("not committed. Bug or a slow test?", solrInputDocuments.length, numDocs); } } @@ -384,11 +377,6 @@ void assertUpdateResponse(UpdateResponse rsp) { assertTrue("Expected no errors: " + errors, errors == null || errors.isEmpty()); } - private int queryNumDocs(String q) throws SolrServerException, IOException { - return (int) - getSolrClient().query(getAlias(), params("q", q, "rows", "0")).getResults().getNumFound(); - } - /** Adds the docs to Solr via {@link #getSolrClient()} with the params */ @SuppressWarnings("SameParameterValue") protected UpdateResponse add( diff --git a/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasDateMathInStartTest.java b/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasDateMathInStartTest.java new file mode 100644 index 000000000000..60008cff3ae8 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasDateMathInStartTest.java @@ -0,0 +1,176 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.update.processor; + +import static org.apache.solr.cloud.api.collections.TimeRoutedAlias.ROUTER_START; + +import java.lang.invoke.MethodHandles; +import java.time.Instant; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeParseException; +import java.util.Arrays; +import java.util.Date; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import org.apache.solr.client.solrj.impl.BaseHttpClusterStateProvider; +import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.impl.ClusterStateProvider; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.util.TimeSource; +import org.apache.solr.util.DateMathParser; +import org.apache.solr.util.TimeOut; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.WatchedEvent; +import org.apache.zookeeper.Watcher; +import org.apache.zookeeper.data.Stat; +import org.junit.Before; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Tests date math in the start parameter of a time routed alias: after a document is added, the + * stored start is rewritten without date math so it parses as an instant. + */ +public class TimeRoutedAliasDateMathInStartTest extends RoutedAliasUpdateProcessorTest { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + private static final String alias = "myalias"; + private static final String timeField = "timestamp_dt"; + + private CloudSolrClient solrClient; + + @Before + public void doBefore() throws Exception { + configureCluster(4).configure(); + solrClient = cluster.getSolrClient(); + // log this to help debug potential causes of problems + if (log.isInfoEnabled()) { + log.info("SolrClient: {}", solrClient); + log.info("ClusterStateProvider {}", solrClient.getClusterStateProvider()); // nowarn + } + } + + @Override + public String getAlias() { + return alias; + } + + @Override + public CloudSolrClient getSolrClient() { + return solrClient; + } + + private String getTimeField() { + return timeField; + } + + @Test + public void testDateMathInStart() throws Exception { + ClusterStateProvider clusterStateProvider = solrClient.getClusterStateProvider(); + Class aClass = clusterStateProvider.getClass(); + System.out.println("CSPROVIDER:" + aClass); + + // This test prevents recurrence of SOLR-13760 + + String configName = getSaferTestName(); + createConfigSet(configName); + CountDownLatch aliasUpdate = new CountDownLatch(1); + monitorAlias(aliasUpdate); + + // each collection has 4 shards with 3 replicas for 12 possible destinations + // 4 of which are leaders, and 8 of which should fail this test. + final int numShards = 1 + random().nextInt(4); + final int numReplicas = 1 + random().nextInt(3); + CollectionAdminRequest.createTimeRoutedAlias( + alias, + "2019-09-14T03:00:00Z/DAY", + "+1DAY", + getTimeField(), + CollectionAdminRequest.createCollection("_unused_", configName, numShards, numReplicas)) + .process(solrClient); + + aliasUpdate.await(); + if (BaseHttpClusterStateProvider.class.isAssignableFrom(aClass)) { + ((BaseHttpClusterStateProvider) clusterStateProvider).resolveAlias(getAlias(), true); + } + + ModifiableSolrParams params = params(); + String nowDay = + DateTimeFormatter.ISO_INSTANT.format( + DateMathParser.parseMath(new Date(), "2019-09-14T01:00:00Z").toInstant()); + assertUpdateResponse( + add( + alias, + Arrays.asList( + sdoc( + "id", + "1", + "timestamp_dt", + nowDay)), // should not cause preemptive creation of 10-28 now + params)); + + // this process should have lead to the modification of the start time for the alias, converting + // it into a parsable date, removing the DateMath + + // what we test next happens in a separate thread, so we have to give it some time to happen. + // Our own watcher on /aliases.json may fire before the provider's ZkStateReader has refreshed + // its aliases, so poll the provider itself instead of waiting on a latch. + final TimeOut timeOut = new TimeOut(30, TimeUnit.SECONDS, TimeSource.NANO_TIME); + String hopeFullyModified; + while (true) { + if (BaseHttpClusterStateProvider.class.isAssignableFrom(aClass)) { + ((BaseHttpClusterStateProvider) clusterStateProvider).resolveAlias(getAlias(), true); + } + hopeFullyModified = clusterStateProvider.getAliasProperties(getAlias()).get(ROUTER_START); + if (hopeFullyModified != null) { + try { + Instant.parse(hopeFullyModified); + break; + } catch (DateTimeParseException e) { + // still has date math, try again + } + } + if (timeOut.hasTimedOut()) { + fail( + ROUTER_START + + " should not have any date math by this point and parse as an instant. Using " + + aClass + + " Found:" + + hopeFullyModified); + } + timeOut.sleep(100); + } + } + + private void monitorAlias(CountDownLatch aliasUpdate) + throws KeeperException, InterruptedException { + Stat stat = new Stat(); + zkClient() + .getData( + "/aliases.json", + new Watcher() { + @Override + public void process(WatchedEvent watchedEvent) { + aliasUpdate.countDown(); + } + }, + stat); + } +} 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 b0d0097b6d3d..0a2c3a4c8972 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 @@ -19,7 +19,6 @@ import static org.apache.solr.client.solrj.request.RoutedAliasTypes.TIME; import static org.apache.solr.cloud.api.collections.RoutedAlias.ROUTED_ALIAS_NAME_CORE_PROP; -import static org.apache.solr.cloud.api.collections.TimeRoutedAlias.ROUTER_START; import static org.apache.solr.common.cloud.ZkStateReader.COLLECTIONS_ZKNODE; import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROPS_ZKNODE; @@ -30,7 +29,6 @@ import java.time.ZoneOffset; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; -import java.time.format.DateTimeParseException; import java.time.temporal.ChronoUnit; import java.util.Arrays; import java.util.Collections; @@ -40,7 +38,6 @@ import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; -import org.apache.lucene.tests.util.LuceneTestCase; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.impl.BaseHttpClusterStateProvider; import org.apache.solr.client.solrj.impl.CloudSolrClient; @@ -63,16 +60,12 @@ import org.apache.solr.update.UpdateCommand; import org.apache.solr.util.DateMathParser; import org.apache.solr.util.LogLevel; -import org.apache.zookeeper.KeeperException; -import org.apache.zookeeper.WatchedEvent; -import org.apache.zookeeper.Watcher; -import org.apache.zookeeper.data.Stat; +import org.apache.solr.util.TestInjection; import org.junit.Before; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -@LuceneTestCase.AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-13059") // TBD public class TimeRoutedAliasUpdateProcessorTest extends RoutedAliasUpdateProcessorTest { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -343,7 +336,8 @@ public void testPreemptiveCreation() throws Exception { ExecutorUtil.newMDCAwareCachedThreadPool( "TimeRoutedAliasProcessorTestx-testPreemptiveCreation"); - Exception[] threadExceptions = new Exception[2]; + TestInjection.preemptiveCreateLatch = new CountDownLatch(2); + Throwable[] threadExceptions = new Throwable[2]; boolean[] threadStarted = new boolean[2]; boolean[] threadFinished = new boolean[2]; try { @@ -354,8 +348,8 @@ public void testPreemptiveCreation() throws Exception { try { starter.await(); concurrentUpdates(params, alias); - } catch (Exception e) { - threadExceptions[0] = e; + } catch (Throwable t) { + threadExceptions[0] = t; } threadFinished[0] = true; }); @@ -366,21 +360,25 @@ public void testPreemptiveCreation() throws Exception { try { starter.await(); concurrentUpdates(params, alias2); - } catch (Exception e) { - threadExceptions[1] = e; + } catch (Throwable t) { + threadExceptions[1] = t; } threadFinished[1] = true; }); starter.countDown(); } finally { ExecutorUtil.shutdownAndAwaitTermination(executorService); + TestInjection.preemptiveCreateLatch = null; } // threads are known to be terminated by now, check for exceptions - for (Exception threadException : threadExceptions) { + for (Throwable threadException : threadExceptions) { if (threadException != null) { Thread.sleep(5000); // avoid spurious fails due to TRA thread not done yet - throw threadException; + if (threadException instanceof Error) { + throw (Error) threadException; + } + throw (Exception) threadException; } } @@ -596,7 +594,16 @@ public void testPreemptiveCreation() throws Exception { cols = new CollectionAdminRequest.ListAliases().process(solrClient).getAliasesAsLists().get(alias); - assertEquals(8, cols.size()); + assertTrue( + "preemptive creation of 2017-10-31 may or may not have finished, but nothing beyond it " + + "may exist yet: " + + cols, + cols.size() == 8 || cols.size() == 9); + assertEquals( + "the only collection that can follow 2017-10-30 is 2017-10-31", + cols.size() == 9, + cols.contains(alias + TRA + "2017-10-31")); + assertFalse(cols.contains(alias + TRA + "2017-11-01")); assertNumDocs("2017-10-23", 1, alias); assertNumDocs("2017-10-24", 1, alias); assertNumDocs("2017-10-25", 7, alias); @@ -664,7 +671,7 @@ public void testPreemptiveCreation() throws Exception { cols = new CollectionAdminRequest.ListAliases().process(solrClient).getAliasesAsLists().get(alias); - assertFalse(cols.contains("myalias" + TRA + "2017-11-03")); + assertFalse(cols.contains(alias + TRA + "2017-11-03")); assertUpdateResponse( add( @@ -705,13 +712,13 @@ private void concurrentUpdates(ModifiableSolrParams params, String alias) params)); assertUpdateResponse(solrClient.commit(alias)); - List colsT1; - colsT1 = + List colsT1 = new CollectionAdminRequest.ListAliases().process(solrClient).getAliasesAsLists().get(alias); - assertEquals(3, colsT1.size()); assertFalse( - "Preemptive creation appears to not be asynchronous anymore", - colsT1.contains("myalias" + TRA + "2017-10-26")); + "preemptive creation of 2017-10-26 must still be held by the test latch" + colsT1, + colsT1.contains(alias + TRA + "2017-10-26")); + assertEquals(3, colsT1.size()); + TestInjection.preemptiveCreateLatch.countDown(); assertNumDocs("2017-10-23", 1, alias); assertNumDocs("2017-10-24", 1, alias); assertNumDocs("2017-10-25", 3, alias); @@ -740,7 +747,7 @@ private void checkPreemptiveCase1(String alias) throws SolrServerException, IOEx assertFalse( "Preemptive creation happened twice and created a collection " + "further in the future than the configured time slice!", - cols.contains("myalias" + TRA + "2017-10-27")); + cols.contains(alias + TRA + "2017-10-27")); validateCollectionCountAndAvailability( alias, 4, "Only 4 cols expected (preemptive create happened" + "twice among threads"); @@ -963,92 +970,6 @@ public void testMaxFutureMs() throws Exception { } } - @AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-13943") - @Test - public void testDateMathInStart() throws Exception { - ClusterStateProvider clusterStateProvider = solrClient.getClusterStateProvider(); - Class aClass = clusterStateProvider.getClass(); - System.out.println("CSPROVIDER:" + aClass); - - // This test prevents recurrence of SOLR-13760 - - String configName = getSaferTestName(); - createConfigSet(configName); - CountDownLatch aliasUpdate = new CountDownLatch(1); - monitorAlias(aliasUpdate); - - // each collection has 4 shards with 3 replicas for 12 possible destinations - // 4 of which are leaders, and 8 of which should fail this test. - final int numShards = 1 + random().nextInt(4); - final int numReplicas = 1 + random().nextInt(3); - CollectionAdminRequest.createTimeRoutedAlias( - alias, - "2019-09-14T03:00:00Z/DAY", - "+1DAY", - getTimeField(), - CollectionAdminRequest.createCollection("_unused_", configName, numShards, numReplicas)) - .process(solrClient); - - aliasUpdate.await(); - if (BaseHttpClusterStateProvider.class.isAssignableFrom(aClass)) { - ((BaseHttpClusterStateProvider) clusterStateProvider).resolveAlias(getAlias(), true); - } - aliasUpdate = new CountDownLatch(1); - monitorAlias(aliasUpdate); - - ModifiableSolrParams params = params(); - String nowDay = - DateTimeFormatter.ISO_INSTANT.format( - DateMathParser.parseMath(new Date(), "2019-09-14T01:00:00Z").toInstant()); - assertUpdateResponse( - add( - alias, - Arrays.asList( - sdoc( - "id", - "1", - "timestamp_dt", - nowDay)), // should not cause preemptive creation of 10-28 now - params)); - - // this process should have lead to the modification of the start time for the alias, converting - // it into a parsable date, removing the DateMath - - // what we test next happens in a separate thread, so we have to give it some time to happen - aliasUpdate.await(); - if (BaseHttpClusterStateProvider.class.isAssignableFrom(aClass)) { - ((BaseHttpClusterStateProvider) clusterStateProvider).resolveAlias(getAlias(), true); - } - - String hopeFullyModified = - clusterStateProvider.getAliasProperties(getAlias()).get(ROUTER_START); - try { - Instant.parse(hopeFullyModified); - } catch (DateTimeParseException e) { - fail( - ROUTER_START - + " should not have any date math by this point and parse as an instant. Using " - + aClass - + " Found:" - + hopeFullyModified); - } - } - - private void monitorAlias(CountDownLatch aliasUpdate) - throws KeeperException, InterruptedException { - Stat stat = new Stat(); - zkClient() - .getData( - "/aliases.json", - new Watcher() { - @Override - public void process(WatchedEvent watchedEvent) { - aliasUpdate.countDown(); - } - }, - stat); - } - /** * Need to ensure that the existing TRA's gracefully handle, old, new and mixtures thereof. TRA's * with an autoDeleteAge setting will gracefully convert to the new format over time.