Github user fsparv commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/304#discussion_r161101824
  
    --- Diff: solr/core/src/java/org/apache/solr/cloud/CreateAliasCmd.java ---
    @@ -45,13 +147,92 @@ public CreateAliasCmd(OverseerCollectionMessageHandler 
ocmh) {
       public void call(ClusterState state, ZkNodeProps message, NamedList 
results)
           throws Exception {
         final String aliasName = message.getStr(NAME);
    -    final List<String> canonicalCollectionList = 
parseCollectionsParameter(message.get("collections"));
    -    final String canonicalCollectionsString = 
StrUtils.join(canonicalCollectionList, ',');
    -
         ZkStateReader zkStateReader = ocmh.zkStateReader;
    -    validateAllCollectionsExistAndNoDups(canonicalCollectionList, 
zkStateReader);
    +    ZkStateReader.AliasesManager holder = zkStateReader.aliasesHolder;
    +    if (!anyRoutingParams(message)) {
    +      final List<String> canonicalCollectionList = 
parseCollectionsParameter(message.get("collections"));
    +      final String canonicalCollectionsString = 
StrUtils.join(canonicalCollectionList, ',');
    +      validateAllCollectionsExistAndNoDups(canonicalCollectionList, 
zkStateReader);
    +      holder.applyModificationAndExportToZk(aliases -> 
aliases.cloneWithCollectionAlias(aliasName, canonicalCollectionsString));
    +    } else {
    +      final String routedField = message.getStr(ROUTING_FIELD);
    +      final String routingType = message.getStr(ROUTING_TYPE);
    +      final String tz = message.getStr(TZ);
    +      final String start = message.getStr(START);
    +      final String increment = message.getStr(ROUTING_INCREMENT);
    +      final String maxFutureMs = message.getStr(ROUTING_MAX_FUTURE);
    +
    +      try {
    +        if (0 > Long.valueOf(maxFutureMs)) {
    +          throw new NumberFormatException("Negative value not allowed 
here");
    +        }
    +      } catch (NumberFormatException e) {
    +        throw new SolrException(BAD_REQUEST, ROUTING_MAX_FUTURE + " must 
be a valid long integer representing a number " +
    +            "of milliseconds greater than or equal to zero");
    +      }
     
    -    zkStateReader.aliasesHolder.applyModificationAndExportToZk(aliases -> 
aliases.cloneWithCollectionAlias(aliasName, canonicalCollectionsString));
    +      // Validate we got everything we need
    +      if (routedField == null || routingType == null || start == null || 
increment == null) {
    +        SolrException solrException = new SolrException(BAD_REQUEST, "If 
any of " + CREATE_ROUTED_ALIAS_PARAMS +
    +            " are supplied, then all of " + REQUIRED_ROUTING_PARAMS + " 
must be present.");
    +        log.error("Could not create routed alias",solrException);
    +        throw solrException;
    +      }
    +
    +      if (!"time".equals(routingType)) {
    +        SolrException solrException = new SolrException(BAD_REQUEST, "Only 
time based routing is supported at this time");
    +        log.error("Could not create routed alias",solrException);
    +        throw solrException;
    +      }
    +      // Check for invalid timezone
    +      if(tz != null && !TimeZoneUtils.KNOWN_TIMEZONE_IDS.contains(tz)) {
    +        SolrException solrException = new SolrException(BAD_REQUEST, 
"Invalid timezone:" + tz);
    +        log.error("Could not create routed alias",solrException);
    +        throw solrException;
    +
    +      }
    +      TimeZone zone;
    +      if (tz != null) {
    +        zone = TimeZoneUtils.getTimeZone(tz);
    +      } else {
    +        zone = TimeZoneUtils.getTimeZone("UTC");
    +      }
    +      DateTimeFormatter fmt = 
DATE_TIME_FORMATTER.withZone(zone.toZoneId());
    --- End diff --
    
    So if someone specifies start as an iso instant, we don't want to adjust 
that to match the specified zone? This would lead to 
start=2018-01-14T20:00:00:00.0000Z&TZ=EST&router.interval=+1DAY it would seem 
that the first bucket should be 2018-01-15 but this way we would be creating a 
first bucket for 2018-01-14 instead. That seems confusing. Solr fairly 
consistently takes the position that times you speak to it must be in UTC, 
changing that here seems awkward. We could allow more general time format and 
take the timezone from the entered time, so that they can enter 
2018-01-15T00:00:00:00.0000EST which is unlike the rest of how solr handles 
time AFAIK but would make the correspondence between the collection name and 
the time value entered intuitive (and maybe complicates combining this with the 
datemath) 
    
    I had originally proposed that collections names for this be UTC and making 
them full time stamps to avoid all this TZ stuff, but the response 
(https://issues.apache.org/jira/browse/SOLR-11299?focusedCommentId=16204466&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16204466)
 was we had to make the collection names friendly for the same reason TZ is 
provided elsewhere, which means accounting for timezones. (this also leads to 
yearly double sized hourly partitions and strange mixing at sub-hour 
partitions, thought that resolution is probably an unusual case)



---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to