Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/433#discussion_r212802686
  
    --- Diff: 
solr/core/src/java/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessor.java
 ---
    @@ -230,6 +188,95 @@ public void processAdd(AddUpdateCommand cmd) throws 
IOException {
         }
       }
     
    +
    +  private String createCollectionsIfRequired(Instant docTimestamp, String 
targetCollection, String printableId) {
    +    // Even though it is possible that multiple requests hit this code in 
the 1-2 sec that
    +    // it takes to create a collection, it's an established anti-pattern 
to feed data with a very large number
    +    // of client connections. This in mind, we only guard against spamming 
the overseer within a batch of
    +    // updates. We are intentionally tolerating a low level of redundant 
requests in favor of simpler code. Most
    +    // super-sized installations with many update clients will likely be 
multi-tenant and multiple tenants
    +    // probably don't write to the same alias. As such, we have deferred 
any solution to the "many clients causing
    +    // collection creation simultaneously" problem until such time as 
someone actually has that problem in a
    +    // real world use case that isn't just an anti-pattern.
    +    try {
    +      CreationType creationType = requiresCreateCollection(docTimestamp, 
timeRoutedAlias.getPreemptiveCreateWindow());
    +      switch (creationType) {
    +        case SYNCHRONOUS:
    +          // This next line blocks until all collections required by the 
current document have been created
    +          return maintain(targetCollection, docTimestamp, printableId, 
false);
    +        case ASYNC_PREEMPTIVE:
    +          // Note: creating an executor and throwing it away is slightly 
expensive, but this is only likely to happen
    +          // once per hour/day/week (depending on time slice size for the 
TRA). If the executor were retained, it
    +          // would need to be shut down in a close hook to avoid test 
failures due to thread leaks which is slightly
    +          // more complicated from a code maintenance and readability 
stand point. An executor must used instead of a
    +          // thread to ensure we pick up the proper MDC logging stuff from 
ExecutorUtil. T
    +          if (preemptiveCreationExecutor == null) {
    +            DefaultSolrThreadFactory threadFactory = new 
DefaultSolrThreadFactory("TRA-preemptive-creation");
    +            preemptiveCreationExecutor = 
newMDCAwareSingleThreadExecutor(threadFactory);
    +            preemptiveCreationExecutor.execute(() -> {
    --- End diff --
    
    the code executed in the new thread should not call maintain() since I see 
you had to make maintain more complicated to tell if it's being called from 
"async".  It can call: 
    ```
          final String mostRecentCollName = 
parsedCollectionsDesc.get(0).getValue();
          createCollectionAfter(mostRecentCollName);
    ```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to