Github user nsoft commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/422#discussion_r204756996
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessor.java
---
@@ -170,55 +179,35 @@ public void processAdd(AddUpdateCommand cmd) throws
IOException {
final Instant routeTimestamp = parseRouteKey(routeValue);
updateParsedCollectionAliases();
- String targetCollection;
- do { // typically we don't loop; it's only when we need to create a
collection
- targetCollection =
findTargetCollectionGivenTimestamp(routeTimestamp);
-
- if (targetCollection == null) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
- "Doc " + cmd.getPrintableId() + " couldn't be routed with " +
timeRoutedAlias.getRouteField() + "=" + routeTimestamp);
- }
-
- // Note: the following rule is tempting but not necessary and is not
compatible with
- // only using this URP when the alias distrib phase is NONE;
otherwise a doc may be routed to from a non-recent
- // collection to the most recent only to then go there directly
instead of realizing a new collection is needed.
- // // If it's going to some other collection (not "this") then
break to just send it there
- // if (!thisCollection.equals(targetCollection)) {
- // break;
- // }
- // Also tempting but not compatible: check that we're the leader,
if not then break
-
- // If the doc goes to the most recent collection then do some checks
below, otherwise break the loop.
- final Instant mostRecentCollTimestamp =
parsedCollectionsDesc.get(0).getKey();
- final String mostRecentCollName =
parsedCollectionsDesc.get(0).getValue();
- if (!mostRecentCollName.equals(targetCollection)) {
- break;
- }
-
- // Check the doc isn't too far in the future
- final Instant maxFutureTime =
Instant.now().plusMillis(timeRoutedAlias.getMaxFutureMs());
- if (routeTimestamp.isAfter(maxFutureTime)) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
- "The document's time routed key of " + routeValue + " is too
far in the future given " +
- TimeRoutedAlias.ROUTER_MAX_FUTURE + "=" +
timeRoutedAlias.getMaxFutureMs());
- }
-
- // Create a new collection?
- final Instant nextCollTimestamp =
timeRoutedAlias.computeNextCollTimestamp(mostRecentCollTimestamp);
- if (routeTimestamp.isBefore(nextCollTimestamp)) {
- break; // thus we don't need another collection
- }
-
- createCollectionAfter(mostRecentCollName); // *should* throw if
fails for some reason but...
- final boolean updated = updateParsedCollectionAliases();
- if (!updated) { // 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.");
+ String targetCollection =
findCandidateCollectionGivenTimestamp(routeTimestamp, cmd.getPrintableId());
+ try {
+ String preemptiveCreateWindow =
timeRoutedAlias.getPreemptiveCreateWindow();
+ if (StringUtils.isBlank(preemptiveCreateWindow) ||
// default: no pre-create
+ requiresCreateCollection(routeTimestamp, null)) { // or
no collection exists for doc
+ targetCollection = new Maintainer(routeTimestamp,
cmd.getPrintableId()).maintain(targetCollection); // then
do it synchronously.
+ } else if (requiresCreateCollection(routeTimestamp,
preemptiveCreateWindow )) { // else async...
+ String finalTargetCollection = targetCollection;
+ try {
+
asyncMaintainers.computeIfAbsent(timeRoutedAlias.getAliasName(),(alias) ->
+ singleThreadSingleTaskExecutor()).submit(() ->
+ new Maintainer(routeTimestamp,
cmd.getPrintableId()).maintain(finalTargetCollection));
+ } catch (RejectedExecutionException e) {
+ // Note: There are some esoteric cases where the pre-create
interval is a lot longer than
--- End diff --
Yeah, repeated throws would probably be a resource drain. My initial
impulse was to keep this simple. I think I originally had an async boolean
floating around and started trying to set up something with semaphores but then
I thought of this and it seemed way simpler. "just fire it off and if someone
already got there just fail quietly here rather than making the overseer
execute a noop" I'll fiddle with it and see if there's a way to keep it simple
without allowing a procession of exception throws.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
