Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/433#discussion_r208449633
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessor.java
---
@@ -405,4 +449,58 @@ protected void doClose() {
collection, slice.getName());
}
+
+ /**
+ * Create as many collections as required. This method loops to allow
for the possibility that the routeTimestamp
+ * 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
+ * @return The latest collection, including collections created during
maintenance
+ */
+ public String maintain(String targetCollection, Instant docTimestamp,
String printableId) {
+ 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
+ // TODO: Instant.now() here seems wrong...
+ final Instant maxFutureTime =
Instant.now().plusMillis(timeRoutedAlias.getMaxFutureMs());
--- End diff --
I think we can move this above the loop or maybe even super up-front. It's
documented in the ref guide and isn't tied to being associated with async/sync
or near the edge. If someone doesn't want this safety check they can
initialize it in a way as to have no practical effect (and sure, ought to be
more easily disabled by using say null or -1).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
