VishnuPriyaChandraSekar commented on code in PR #4626:
URL: https://github.com/apache/solr/pull/4626#discussion_r3554333017


##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/ConcurrentUpdateJdkSolrClientTest.java:
##########
@@ -28,7 +28,11 @@ public HttpSolrClient solrClient(Integer 
overrideIdleTimeoutMs) {
     var builder =
         new 
HttpJdkSolrClient.Builder().withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT);
     if (overrideIdleTimeoutMs != null) {
-      builder.withIdleTimeout(overrideIdleTimeoutMs, TimeUnit.MILLISECONDS);
+      builder
+          .withIdleTimeout(overrideIdleTimeoutMs, TimeUnit.MILLISECONDS)
+          // override the infinite request timeout with idle timeout to ensure 
idle requests times
+          // out.

Review Comment:
   The idle timeout provided in the tests was ineffective for JDK and Jetty 
clients. 
   In case of Jetty, the idle timeout is hardcoded as 
[infinite](https://github.com/apache/solr/blob/main/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java#L304)
 (not sure whether this is a bug or intended behavior). While in JDK, we are 
not even passing the override idle Time out to the [JDK http client 
](https://github.com/apache/solr/blob/main/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java#L98)
 (looks like JDK does not support configuring idle timeout using builder). 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to