Github user dsmiley commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/433#discussion_r214118388 --- Diff: solr/core/src/java/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessor.java --- @@ -437,45 +461,46 @@ protected void doClose() { /** - * Create as many collections as required. This method loops to allow for the possibility that the routeTimestamp + * Create as many collections as required. This method loops to allow for the possibility that the docTimestamp * requires more than one collection to be created. Since multiple threads may be invoking maintain on separate * requests to the same alias, we must pass in the name of the collection that this thread believes to be the most * recent collection. This assumption is checked when the command is executed in the overseer. When this method * finds that all collections required have been created it returns the (possibly new) most recent collection. * The return value is ignored by the calling code in the async preemptive case. * - * @param targetCollection the initial notion of the latest collection available. * @param docTimestamp the timestamp from the document that determines routing * @param printableId an identifier for the add command used in error messages + * @param targetCollectionDesc the descriptor for the presently selected collection which should also be + * the most recent collection in all cases where this method is invoked. * @return The latest collection, including collections created during maintenance */ - public String maintain(String targetCollection, Instant docTimestamp, String printableId, boolean asyncSinglePassOnly) { - do { // typically we don't loop; it's only when we need to create a collection - - // Note: This code no longer short circuits immediately when it sees that the expected latest - // collection is the current latest collection. With the advent of preemptive collection creation - // we always need to do the time based checks. Otherwise, we cannot handle the case where the - // preemptive window is larger than our TRA's time slices - - // Check the doc isn't too far in the future - - if (NONE == requiresCreateCollection(docTimestamp, timeRoutedAlias.getPreemptiveCreateWindow())) - return targetCollection; // thus we don't need another collection + private String createAllRequiredCollections( Instant docTimestamp, String printableId, + Map.Entry<Instant, String> targetCollectionDesc) { + do { + switch(typeOfCreationRequired(docTimestamp, targetCollectionDesc.getKey())) { + case NONE: + return targetCollectionDesc.getValue(); // we don't need another collection + case ASYNC_PREEMPTIVE: + // can happen when preemptive interval is longer than one time slice + preemptiveAsync(() -> createNextCollection(this.parsedCollectionsDesc.get(0).getValue())); + return targetCollectionDesc.getValue(); + case SYNCHRONOUS: + createNextCollection(targetCollectionDesc.getValue()); // *should* throw if fails for some reason but... + if (!updateParsedCollectionAliases()) { // thus we didn't make progress... + // this is not expected, even in known failure cases, but we check just in case + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, + "We need to create a new time routed collection but for unknown reasons were unable to do so."); + } + // then retry the loop ... have to do find again in case other requests also added collections + // that were made visible when we called updateParsedCollectionAliases() + targetCollectionDesc = findCandidateGivenTimestamp(docTimestamp, printableId); + break; + default: + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown creation type while adding " + --- End diff -- I wouldn't waste your virtual breath (lines of code with thoughtful explanation) on a default case of an enum switch. Another approach I like is to `assert ENUMNAME.values().length == 3;` before the switch which will be caught at test time. I forget but if java still insists we have a default then throw IllegalStateException or something like that in a one-liner without explaination. (keep it short & sweet)
--- --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org