lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From nsoft <...@git.apache.org>
Subject [GitHub] lucene-solr pull request #433: SOLR-12357 Premptive creation of collections ...
Date Thu, 30 Aug 2018 22:07:26 GMT
Github user nsoft commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/433#discussion_r214170180
  
    --- Diff: solr/core/src/java/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessor.java
---
    @@ -165,31 +165,42 @@ private String getAliasName() {
     
       @Override
       public void processAdd(AddUpdateCommand cmd) throws IOException {
    -    SolrInputDocument solrInputDocument = cmd.getSolrInputDocument();
    -    final Object routeValue = solrInputDocument.getFieldValue(timeRoutedAlias.getRouteField());
    -    final Instant docTimestampToRoute = parseRouteKey(routeValue);
    -    updateParsedCollectionAliases();
    -    String candidateCollection = findCandidateCollectionGivenTimestamp(docTimestampToRoute,
cmd.getPrintableId());
    -    final Instant maxFutureTime = Instant.now().plusMillis(timeRoutedAlias.getMaxFutureMs());
    +    final Instant docTimestamp =
    +        parseRouteKey(cmd.getSolrInputDocument().getFieldValue(timeRoutedAlias.getRouteField()));
    +
         // TODO: maybe in some cases the user would want to ignore/warn instead?
    -    if (docTimestampToRoute.isAfter(maxFutureTime)) {
    +    if (docTimestamp.isAfter(Instant.now().plusMillis(timeRoutedAlias.getMaxFutureMs())))
{
           throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
    -          "The document's time routed key of " + docTimestampToRoute + " is too far in
the future given " +
    +          "The document's time routed key of " + docTimestamp + " is too far in the future
given " +
                   TimeRoutedAlias.ROUTER_MAX_FUTURE + "=" + timeRoutedAlias.getMaxFutureMs());
         }
    -    String targetCollection = createCollectionsIfRequired(docTimestampToRoute, candidateCollection,
cmd.getPrintableId());
    +
    +    // to avoid potential for race conditions, this next method should not get called
again unless
    +    // we have created a collection synchronously
    +    updateParsedCollectionAliases();
    +
    +    String targetCollection = createCollectionsIfRequired(docTimestamp, cmd.getPrintableId(),
cmd);
    +
         if (thisCollection.equals(targetCollection)) {
           // pass on through; we've reached the right collection
           super.processAdd(cmd);
         } else {
           // send to the right collection
    -      SolrCmdDistributor.Node targetLeaderNode = routeDocToSlice(targetCollection, solrInputDocument);
    +      SolrCmdDistributor.Node targetLeaderNode = routeDocToSlice(targetCollection, cmd.getSolrInputDocument());
           cmdDistrib.distribAdd(cmd, Collections.singletonList(targetLeaderNode), new ModifiableSolrParams(outParamsToLeader));
         }
       }
     
    -
    -  private String createCollectionsIfRequired(Instant docTimestamp, String targetCollection,
String printableId) {
    +  /**
    +   * Create any required collections and return the name of the collection to which the
current document should be sent.
    +   *
    +   * @param docTimestamp the date for the document taken from the field specified in
the TRA config
    --- End diff --
    
    nice catch, yeah I added the cmd late in the game. very happy to get rid of printable
id.
    Generally if I doc a method I doc it fully (goal at least) if not I don't do any of it.
I generally add to private methods only where it seems to be adding something. In this case
the method name doesn't actually indicate that we're also selecting the target collection
as well. The method name is already long so I used docs.


---

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


Mime
View raw message