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