dsmiley commented on code in PR #2474:
URL: https://github.com/apache/solr/pull/2474#discussion_r1610754549


##########
solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java:
##########
@@ -93,69 +87,69 @@ public class CoreContainerProvider implements 
ServletContextListener {
   private HttpClient httpClient;
   private SolrMetricManager metricManager;
   private RateLimitManager rateLimitManager;
-  private final CountDownLatch init = new CountDownLatch(1);
   private String registryName;
-  // AFAIK the only reason we need this is to support JettySolrRunner for 
tests. In tests we might
-  // have multiple CoreContainers in the same JVM, but I *think* that doesn't 
happen in a real
-  // server.
-  private static final Map<ContextInitializationKey, ServiceHolder> services =
-      Collections.synchronizedMap(new WeakHashMap<>());
-
-  // todo: dependency injection instead, but for now this method and the 
associated map will have
-  // to suffice.
-  // Note that this relies on ServletContext.equals() not implementing 
anything significantly
-  // different than Object.equals for its .equals method (I've found no 
implementation that even
-  // implements it).
-  public static ServiceHolder serviceForContext(ServletContext ctx) throws 
InterruptedException {
-    ContextInitializationKey key = new ContextInitializationKey(ctx);
-    return services.computeIfAbsent(key, ServiceHolder::new);
+
+  /** Acquires an instance from the context, waiting if necessary. */
+  public static CoreContainerProvider serviceForContext(ServletContext ctx)
+      throws InterruptedException {
+    long startWait = System.nanoTime();
+    synchronized (ctx) {

Review Comment:
   > Worse yet the use of JettySolrRunner for our tests means that we don't 
have any tests for what start.jar is doing.
   
   BATS integration tests is our answer to that.  JettySolrRunner just needs to 
handle the sequencing realistically (what starts and stops when).  You might 
have also noticed some bolted-on requirements for tests like debug filter, 
proxy port, and more, so I don't see JettySolrRunner going away.  It could be 
simplified further!
   
   I'm not optimistic about that QuickStart thing helping us -- we've already 
tuned Jetty.  Most notably, our web.xml has `metadata-complete="true"` thus 
there is no annotation scanning.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Reply via email to