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: [email protected]
For additional commands, e-mail: [email protected]
