diff --git a/changelog/unreleased/SOLR-18562-tra-start-keeps-other-aliases.yml b/changelog/unreleased/SOLR-18562-tra-start-keeps-other-aliases.yml new file mode 100644 index 000000000000..8b751362a44b --- /dev/null +++ b/changelog/unreleased/SOLR-18562-tra-start-keeps-other-aliases.yml @@ -0,0 +1,12 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc + +title: > + Fixing a time routed alias's date math router.start on its first document no longer removes + aliases created after the receiving node last read aliases.json +type: fixed +authors: + - name: Boyi Dai + nick: SpookyJumpyBeans +links: + - name: SOLR-18562 + url: https://issues.apache.org/jira/browse/SOLR-18562 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..7d2921d9a001 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 @@ -37,7 +37,6 @@ import java.util.AbstractMap; import java.util.ArrayList; import java.util.Date; -import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -406,19 +405,23 @@ public void validateRouteValue(AddUpdateCommand cmd) throws SolrException { startTime = DateMathParser.parseMath(new Date(), start).toInstant(); SolrCore core = cmd.getReq().getCore(); ZkStateReader zkStateReader = core.getCoreContainer().getZkController().zkStateReader; - Aliases aliases = zkStateReader.getAliases(); - Map props = new HashMap<>(aliases.getCollectionAliasProperties(aliasName)); start = DateTimeFormatter.ISO_INSTANT.format(startTime); - props.put(ROUTER_START, start); + final String fixedStart = start; // This could race, but it only occurs when the alias is first used and the values produced // should all be identical and who wins won't matter (baring cases of Date Math involving // seconds, which is pretty far fetched). Putting this in a separate thread to ensure that any - // failed races don't cause documents to get rejected. + // failed races don't cause documents to get rejected. The modification must be applied to the + // aliases it is given, which are current, so that it doesn't undo other changes to aliases. + // If the alias has been deleted meanwhile, there is nothing to update. core.runAsync( () -> zkStateReader.aliasesManager.applyModificationAndExportToZk( - (a) -> aliases.cloneWithCollectionAliasProperties(aliasName, props))); + (a) -> + a.hasAlias(aliasName) + ? a.cloneWithCollectionAliasProperties( + aliasName, ROUTER_START, fixedStart) + : a)); } if (docTimestamp.isBefore(startTime)) { throw new SolrException( 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 9184308b1085..9a9e6a0169ea 100644 --- a/solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java @@ -20,6 +20,7 @@ import static org.apache.solr.client.solrj.request.RoutedAliasTypes.TIME; import java.io.IOException; +import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.time.Duration; import java.time.Instant; @@ -28,19 +29,25 @@ import java.util.List; import java.util.Map; import java.util.TimeZone; +import java.util.concurrent.TimeUnit; +import org.apache.lucene.util.SuppressForbidden; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrServerException; 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.SolrInputDocument; import org.apache.solr.common.cloud.Aliases; import org.apache.solr.common.cloud.CompositeIdRouter; import org.apache.solr.common.cloud.DocCollection; import org.apache.solr.common.cloud.ImplicitDocRouter; import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.cloud.ZkStateReader; +import org.apache.solr.common.util.TimeSource; +import org.apache.solr.embedded.JettySolrRunner; import org.apache.solr.util.DateMathParser; +import org.apache.solr.util.TimeOut; import org.eclipse.jetty.client.Request; import org.eclipse.jetty.client.StringRequestContent; import org.junit.After; @@ -177,6 +184,90 @@ public void testV2() throws Exception { assertEquals(createNode, meta.get("create-collection.createNodeSet")); } + /** + * Fixing a date math router.start when the first document arrives must not undo alias changes + * that the node handling the document hasn't seen yet. + */ + @Test + @SuppressForbidden( + reason = "Reflection needed to give a node cached aliases that are behind ZooKeeper") + public void testFixingStartKeepsOtherAliases() throws Exception { + final String aliasName = getSaferTestName(); + CollectionAdminRequest.createTimeRoutedAlias( + aliasName, + "NOW/DAY", + "+1DAY", + "evt_dt", + CollectionAdminRequest.createCollection("_unused_", "_default", 1, 1)) + .process(solrClient); + ZkStateReader clientReader = cluster.getZkStateReader(); + clientReader.aliasesManager.update(); + final String collectionName = + clientReader.getAliases().getCollectionAliasListMap().get(aliasName).get(0); + cluster.waitForActiveCollection(collectionName, 1, 1); + + // send the update to the node hosting the collection, so that node handles it itself + String nodeName = + clientReader + .getCollection(collectionName) + .replicaStream() + .findFirst() + .orElseThrow() + .getNodeName(); + JettySolrRunner jetty = + cluster.getJettySolrRunners().stream() + .filter(j -> j.getNodeName().equals(nodeName)) + .findFirst() + .orElseThrow(); + ZkStateReader reader = jetty.getCoreContainer().getZkController().getZkStateReader(); + int zkVersion = cluster.getZkClient().exists(ZkStateReader.ALIASES, null).getVersion(); + new TimeOut(30, TimeUnit.SECONDS, TimeSource.NANO_TIME) + .waitFor( + "node did not catch up with aliases.json", + () -> reader.getAliases().getZNodeVersion() == zkVersion); + Aliases aliasesBeforeOtherAlias = reader.getAliases(); + + // another alias is created and a property is set on this one, and the node is put back to + // before it saw them, as if the document arrived before the node processed the watch events + final String otherAlias = aliasName + "_other"; + CollectionAdminRequest.createAlias(otherAlias, collectionName).process(solrClient); + CollectionAdminRequest.setAliasProperty(aliasName) + .addProperty("foo", "bar") + .process(solrClient); + new TimeOut(30, TimeUnit.SECONDS, TimeSource.NANO_TIME) + .waitFor( + "node did not see the alias changes", + () -> + reader.getAliases().hasAlias(otherAlias) + && "bar" + .equals( + reader + .getAliases() + .getCollectionAliasProperties(aliasName) + .get("foo"))); + Field aliasesField = ZkStateReader.AliasesManager.class.getDeclaredField("aliases"); + aliasesField.setAccessible(true); + aliasesField.set(reader.aliasesManager, aliasesBeforeOtherAlias); + + SolrInputDocument doc = new SolrInputDocument("id", "1", "evt_dt", Instant.now().toString()); + assertEquals(0, jetty.getSolrClient().add(collectionName, doc).getStatus()); + + // router.start is fixed asynchronously + new TimeOut(30, TimeUnit.SECONDS, TimeSource.NANO_TIME) + .waitFor( + "router.start was not fixed", + () -> + !"NOW/DAY" + .equals( + clientReader + .getAliases() + .getCollectionAliasProperties(aliasName) + .get(TimeRoutedAlias.ROUTER_START))); + assertEquals(collectionName, clientReader.getAliases().getCollectionAliasMap().get(otherAlias)); + assertEquals( + "bar", clientReader.getAliases().getCollectionAliasProperties(aliasName).get("foo")); + } + @Test public void testV1() throws Exception { final String aliasName = getSaferTestName();