You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@solr.apache.org by David Smiley <ds...@apache.org> on 2025/05/19 13:45:54 UTC
SolrJ timeouts
This weekend I was harmonizing some of the timeouts between JDK & Jetty
based HTTP SolrClient implementations. I saw some inconsistencies,
duplication, and the need for javadocs to explain what these timeouts
fundamentally mean. I have some nice WIP code I was looking forward to
sharing by now.
Then I noticed something insidious: If an Http2SolrClient (Jetty) is
created using the builder's withHttpClient(...) method, then the idle (aka
socket) timeout & connection timeout (and basically any other setting
that's inside the Jetty HttpClient) cannot *actually* be customized
henceforth. This should be pretty obvious, when you think about it (duh).
But we don't do any checks in construction to alert you, plus we
redundantly put some of these settings on the Http2SolrClient (incl. base
class) which fools any onlooker into thinking it might do something. My
WIP addresses this by throwing an IllegalArgumentException on build() if
you try to set something that won't take effect. This revealed a number of
prominent places in Solr that both call withHttpClient and also customize
the timeouts (to no avail): Overseer, SolrClientCache (for streaming
expressions), IndexFetcher, and some recovery stuff; maybe I missed
something. This solution won't work.
Perhaps withHttpClient should not reuse the underlying HttpClient? It's a
shame to not reuse it but it's at least the safest choice. In my opinion,
withHttpClient doesn't have an ideal name either; it should be
withHttpSolrClient. Its current name is ambiguous; I hate confusing
HttpSolrClient with HttpClient. And if we copy configuration from the
client but don't actually use it, then maybe we should name this method
copyConfigurationFrom or withConfigurationFrom.
I'll throw up a draft PR today that does the cleanup/harmonization but not
yet solve the above conundrum.
(CC'ed some folks that worked on timeouts)
~ David Smiley
Apache Lucene/Solr Search Developer
http://www.linkedin.com/in/davidwsmiley
Re: SolrJ timeouts
Posted by David Smiley <ds...@apache.org>.
I enhanced https://github.com/apache/solr/pull/3357 to incorporate that
test and make other changes. Keeping the IllegalStateException but at
least the idle timeout can be set at the request level. The downside is
that a number of places in Solr that were customizing the connection
timeout can no longer do so. I think it's fine; it strikes me as rather
obscure to really _need_ to mess with the connection timeout on a use-case
basis.
I filed https://issues.apache.org/jira/browse/SOLR-17776 to capture this
work.
I'd like to get that into 9.x.
On Sat, May 24, 2025 at 8:55 PM Luke Kot-Zaniewski (BLOOMBERG/ 919 3RD A) <
lkotzaniewsk@bloomberg.net> wrote:
> That's a good point. I didn't immediately realize it would effectively be
> the minimum of the two idle timeout values but now that I think about it,
> it makes sense.
>
> Looking at it from a higher level though, since Jetty appears to honor
> idleTimeout at the request level (either through the aforementioned
> listener or through client Request itself
> https://javadoc.jetty.org/jetty-11/org/eclipse/jetty/client/api/Request.html#idleTimeout(long,java.util.concurrent.TimeUnit)
> and considering that we already have a client wrapper that saves these
> timeouts, it would make sense to have some convention/validation where we
> only set timeouts at the solr client level and actually never proagate it
> to the underlying HttpClient (and rather keep those at the max value). This
> would make more flexible and allow sharing underlying HttpClients in some
> circumstances. Admittedly this might only work for settings that appear to
> have jetty request-level overrides, i.e. idle timeouts and request timeouts
> (not sure about connectionTimeouts for instance), but maybe that's
> something worth exploring? In a project the size of Solr sharing
> clients/connections wherever possible might be worthwhile if it can be done
> reasonably...
>
> Sent from Bloomberg Professional for Android
>
> ----- Original Message -----
> From: David Smiley <de...@solr.apache.org>
> To: dev@solr.apache.org
> CC: LUKE KOT-ZANIEWSKI, stillalex@apache.org, gus.heck@gmail.com
> At: 05/24/25 15:12:28 UTC-04:00
>
>
> Luke, thanks for writing this test!!
>
> I looked at your test
> (esp.
> org.apache.solr.client.solrj.impl.Http2SolrClientTest#testIdleTimeoutWithHttpClient
> ) in more detail. You tested going from having a sufficient amount of idle
> timeout for the request to succeed, then down to a lower amount, asserting
> it failed. But we want the other way around -- low to high. I pushed this
> change to your PR and the test failed. Can you look please? Unless I'm
> missing something, this failure demonstrates the problem I am ringing the
> alarm bells about.
>
> https://issues.apache.org/jira/browse/SOLR-13605
> Ah, thanks for reminding me. That issue is about Apache based HTTP
> SolrClient, and yes, it seems like the "reverse" problem.
>
> On Thu, May 22, 2025 at 7:47 PM Chris Hostetter <ho...@fucit.org>
> wrote:
>
> >
> >
> > : Then I noticed something insidious: If an Http2SolrClient (Jetty) is
> > : created using the builder's withHttpClient(...) method, then the idle
> > (aka
> > : socket) timeout & connection timeout (and basically any other setting
> > : that's inside the Jetty HttpClient) cannot *actually* be customized
> > : henceforth. This should be pretty obvious, when you think about it
> > (duh).
> >
> > Deja Vu...
> >
> > https://issues.apache.org/jira/browse/SOLR-13605
> >
> > ...except now we have the reverse problem?
> >
> >
> >
> > -Hoss
> > http://www.lucidworks.com/
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@solr.apache.org
> > For additional commands, e-mail: dev-help@solr.apache.org
> >
> >
>
Re: SolrJ timeouts
Posted by "Luke Kot-Zaniewski (BLOOMBERG/ 919 3RD A)" <lk...@bloomberg.net>.
That's a good point. I didn't immediately realize it would effectively be the minimum of the two idle timeout values but now that I think about it, it makes sense.
Looking at it from a higher level though, since Jetty appears to honor idleTimeout at the request level (either through the aforementioned listener or through client Request itself https://javadoc.jetty.org/jetty-11/org/eclipse/jetty/client/api/Request.html#idleTimeout(long,java.util.concurrent.TimeUnit) and considering that we already have a client wrapper that saves these timeouts, it would make sense to have some convention/validation where we only set timeouts at the solr client level and actually never proagate it to the underlying HttpClient (and rather keep those at the max value). This would make more flexible and allow sharing underlying HttpClients in some circumstances. Admittedly this might only work for settings that appear to have jetty request-level overrides, i.e. idle timeouts and request timeouts (not sure about connectionTimeouts for instance), but maybe that's something worth exploring? In a project the size of Solr sharing clients/connections wherever possible might be worthwhile if it can be done reasonably...
Sent from Bloomberg Professional for Android
----- Original Message -----
From: David Smiley <de...@solr.apache.org>
To: dev@solr.apache.org
CC: LUKE KOT-ZANIEWSKI, stillalex@apache.org, gus.heck@gmail.com
At: 05/24/25 15:12:28 UTC-04:00
Luke, thanks for writing this test!!
I looked at your test
(esp. org.apache.solr.client.solrj.impl.Http2SolrClientTest#testIdleTimeoutWithHttpClient
) in more detail. You tested going from having a sufficient amount of idle
timeout for the request to succeed, then down to a lower amount, asserting
it failed. But we want the other way around -- low to high. I pushed this
change to your PR and the test failed. Can you look please? Unless I'm
missing something, this failure demonstrates the problem I am ringing the
alarm bells about.
https://issues.apache.org/jira/browse/SOLR-13605
Ah, thanks for reminding me. That issue is about Apache based HTTP
SolrClient, and yes, it seems like the "reverse" problem.
On Thu, May 22, 2025 at 7:47 PM Chris Hostetter <ho...@fucit.org>
wrote:
>
>
> : Then I noticed something insidious: If an Http2SolrClient (Jetty) is
> : created using the builder's withHttpClient(...) method, then the idle
> (aka
> : socket) timeout & connection timeout (and basically any other setting
> : that's inside the Jetty HttpClient) cannot *actually* be customized
> : henceforth. This should be pretty obvious, when you think about it
> (duh).
>
> Deja Vu...
>
> https://issues.apache.org/jira/browse/SOLR-13605
>
> ...except now we have the reverse problem?
>
>
>
> -Hoss
> http://www.lucidworks.com/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@solr.apache.org
> For additional commands, e-mail: dev-help@solr.apache.org
>
>
Re: SolrJ timeouts
Posted by David Smiley <ds...@apache.org>.
Luke, thanks for writing this test!!
I looked at your test
(esp. org.apache.solr.client.solrj.impl.Http2SolrClientTest#testIdleTimeoutWithHttpClient
) in more detail. You tested going from having a sufficient amount of idle
timeout for the request to succeed, then down to a lower amount, asserting
it failed. But we want the other way around -- low to high. I pushed this
change to your PR and the test failed. Can you look please? Unless I'm
missing something, this failure demonstrates the problem I am ringing the
alarm bells about.
https://issues.apache.org/jira/browse/SOLR-13605
Ah, thanks for reminding me. That issue is about Apache based HTTP
SolrClient, and yes, it seems like the "reverse" problem.
On Thu, May 22, 2025 at 7:47 PM Chris Hostetter <ho...@fucit.org>
wrote:
>
>
> : Then I noticed something insidious: If an Http2SolrClient (Jetty) is
> : created using the builder's withHttpClient(...) method, then the idle
> (aka
> : socket) timeout & connection timeout (and basically any other setting
> : that's inside the Jetty HttpClient) cannot *actually* be customized
> : henceforth. This should be pretty obvious, when you think about it
> (duh).
>
> Deja Vu...
>
> https://issues.apache.org/jira/browse/SOLR-13605
>
> ...except now we have the reverse problem?
>
>
>
> -Hoss
> http://www.lucidworks.com/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@solr.apache.org
> For additional commands, e-mail: dev-help@solr.apache.org
>
>
Re: SolrJ timeouts
Posted by Chris Hostetter <ho...@fucit.org>.
: Then I noticed something insidious: If an Http2SolrClient (Jetty) is
: created using the builder's withHttpClient(...) method, then the idle (aka
: socket) timeout & connection timeout (and basically any other setting
: that's inside the Jetty HttpClient) cannot *actually* be customized
: henceforth. This should be pretty obvious, when you think about it (duh).
Deja Vu...
https://issues.apache.org/jira/browse/SOLR-13605
...except now we have the reverse problem?
-Hoss
http://www.lucidworks.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@solr.apache.org
For additional commands, e-mail: dev-help@solr.apache.org
Re: SolrJ timeouts
Posted by "Luke Kot-Zaniewski (BLOOMBERG/ 919 3RD A)" <lk...@bloomberg.net>.
Hi David,
I added the test you suggested here https://github.com/apache/solr/pull/3362
Surprisingly they pass. I think it is because although the idleTimeout set on the jetty HttpClient passed into the Http2SolrClient remains unchanged, the Http2SolrClient-level timeout is still enforced via Jetty's InputStreamResponseListener::get which effectively works like an idle timeout:
https://github.com/apache/solr/blob/b91905a653ead168cea9de2a1f1791c76fb17882/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java#L517
I got somewhat paranoid and added the SlowStream test servlet to make sure that both timeouts are respected when they are overridden. As far as I can tell, they are.
I will admit that it still feels weird to pass an idleTimeout into a Http2SolrClient and then *not* propagate it to the underlying Jetty HttpClient. Also, there could be other properties that don't work as nicely as these two when overridden on an Http2SolrClient that takes in a Jetty HttpClient. But I haven't given that a deeper look.
Thanks,
Luke
From: dev@solr.apache.org At: 05/19/25 16:32:09 UTC-4:00To: dev@solr.apache.org
Cc: Luke Kot-Zaniewski (BLOOMBERG/ 919 3RD A ) , stillalex@apache.org, gus.heck@gmail.com
Subject: Re: SolrJ timeouts
My annotated draft PR:
https://github.com/apache/solr/pull/3357
Doesn't solve the issue but its exception throwing means many tests fail
since those places in Solr were trying to both use an existing HttpClient
_and_ set timeouts. We can't do that and we *weren't* doing that! 'course
if I'm missing something obvious; let me know!
> I guess my attempt to fix it (https://github.com/apache/solr/pull/3356)
is doomed to fail as it currently stands?
Agreed. But let's see: Maybe a test would be: configure a super low idle
timeout on an Http2SolrClient that's constructed manually. Then create a
new Http2SolrClient with the first using withHttpClient and set a
reasonable timeout. We should be able to do a request that takes time
(like a Solr request referencing "sleep(5000)" in a function query. See
features-slow.json with such an excerpt. I believe we'll sadly have a
timeout because the idle timeout customization isn't honored when
withHttpClient is used. Such a test would go in Http2SolrClientTest.
On Mon, May 19, 2025 at 9:45 AM David Smiley <ds...@apache.org> wrote:
> This weekend I was harmonizing some of the timeouts between JDK & Jetty
> based HTTP SolrClient implementations. I saw some inconsistencies,
> duplication, and the need for javadocs to explain what these timeouts
> fundamentally mean. I have some nice WIP code I was looking forward to
> sharing by now.
>
> Then I noticed something insidious: If an Http2SolrClient (Jetty) is
> created using the builder's withHttpClient(...) method, then the idle (aka
> socket) timeout & connection timeout (and basically any other setting
> that's inside the Jetty HttpClient) cannot *actually* be customized
> henceforth. This should be pretty obvious, when you think about it (duh).
> But we don't do any checks in construction to alert you, plus we
> redundantly put some of these settings on the Http2SolrClient (incl. base
> class) which fools any onlooker into thinking it might do something. My
> WIP addresses this by throwing an IllegalArgumentException on build() if
> you try to set something that won't take effect. This revealed a number of
> prominent places in Solr that both call withHttpClient and also customize
> the timeouts (to no avail): Overseer, SolrClientCache (for streaming
> expressions), IndexFetcher, and some recovery stuff; maybe I missed
> something. This solution won't work.
>
> Perhaps withHttpClient should not reuse the underlying HttpClient? It's a
> shame to not reuse it but it's at least the safest choice. In my opinion,
> withHttpClient doesn't have an ideal name either; it should be
> withHttpSolrClient. Its current name is ambiguous; I hate confusing
> HttpSolrClient with HttpClient. And if we copy configuration from the
> client but don't actually use it, then maybe we should name this method
> copyConfigurationFrom or withConfigurationFrom.
>
> I'll throw up a draft PR today that does the cleanup/harmonization but not
> yet solve the above conundrum.
>
> (CC'ed some folks that worked on timeouts)
>
> ~ David Smiley
> Apache Lucene/Solr Search Developer
> http://www.linkedin.com/in/davidwsmiley
>
Re: SolrJ timeouts
Posted by David Smiley <ds...@apache.org>.
My annotated draft PR:
https://github.com/apache/solr/pull/3357
Doesn't solve the issue but its exception throwing means many tests fail
since those places in Solr were trying to both use an existing HttpClient
_and_ set timeouts. We can't do that and we *weren't* doing that! 'course
if I'm missing something obvious; let me know!
> I guess my attempt to fix it (https://github.com/apache/solr/pull/3356)
is doomed to fail as it currently stands?
Agreed. But let's see: Maybe a test would be: configure a super low idle
timeout on an Http2SolrClient that's constructed manually. Then create a
new Http2SolrClient with the first using withHttpClient and set a
reasonable timeout. We should be able to do a request that takes time
(like a Solr request referencing "sleep(5000)" in a function query. See
features-slow.json with such an excerpt. I believe we'll sadly have a
timeout because the idle timeout customization isn't honored when
withHttpClient is used. Such a test would go in Http2SolrClientTest.
On Mon, May 19, 2025 at 9:45 AM David Smiley <ds...@apache.org> wrote:
> This weekend I was harmonizing some of the timeouts between JDK & Jetty
> based HTTP SolrClient implementations. I saw some inconsistencies,
> duplication, and the need for javadocs to explain what these timeouts
> fundamentally mean. I have some nice WIP code I was looking forward to
> sharing by now.
>
> Then I noticed something insidious: If an Http2SolrClient (Jetty) is
> created using the builder's withHttpClient(...) method, then the idle (aka
> socket) timeout & connection timeout (and basically any other setting
> that's inside the Jetty HttpClient) cannot *actually* be customized
> henceforth. This should be pretty obvious, when you think about it (duh).
> But we don't do any checks in construction to alert you, plus we
> redundantly put some of these settings on the Http2SolrClient (incl. base
> class) which fools any onlooker into thinking it might do something. My
> WIP addresses this by throwing an IllegalArgumentException on build() if
> you try to set something that won't take effect. This revealed a number of
> prominent places in Solr that both call withHttpClient and also customize
> the timeouts (to no avail): Overseer, SolrClientCache (for streaming
> expressions), IndexFetcher, and some recovery stuff; maybe I missed
> something. This solution won't work.
>
> Perhaps withHttpClient should not reuse the underlying HttpClient? It's a
> shame to not reuse it but it's at least the safest choice. In my opinion,
> withHttpClient doesn't have an ideal name either; it should be
> withHttpSolrClient. Its current name is ambiguous; I hate confusing
> HttpSolrClient with HttpClient. And if we copy configuration from the
> client but don't actually use it, then maybe we should name this method
> copyConfigurationFrom or withConfigurationFrom.
>
> I'll throw up a draft PR today that does the cleanup/harmonization but not
> yet solve the above conundrum.
>
> (CC'ed some folks that worked on timeouts)
>
> ~ David Smiley
> Apache Lucene/Solr Search Developer
> http://www.linkedin.com/in/davidwsmiley
>
Re:SolrJ timeouts
Posted by "Luke Kot-Zaniewski (BLOOMBERG/ 919 3RD A)" <lk...@bloomberg.net>.
Nice find. Look forward to seeing the PR as the IndexFetcher timeout regression has already bitten us. I guess my attempt to fix it (https://github.com/apache/solr/pull/3356) is doomed to fail as it currently stands?
From: dev@solr.apache.org At: 05/19/25 09:46:11 UTC-4:00To: dev@solr.apache.org
Cc: Luke Kot-Zaniewski (BLOOMBERG/ 919 3RD A ) , stillalex@apache.org, gus.heck@gmail.com
Subject: SolrJ timeouts
This weekend I was harmonizing some of the timeouts between JDK & Jetty
based HTTP SolrClient implementations. I saw some inconsistencies,
duplication, and the need for javadocs to explain what these timeouts
fundamentally mean. I have some nice WIP code I was looking forward to
sharing by now.
Then I noticed something insidious: If an Http2SolrClient (Jetty) is
created using the builder's withHttpClient(...) method, then the idle (aka
socket) timeout & connection timeout (and basically any other setting
that's inside the Jetty HttpClient) cannot *actually* be customized
henceforth. This should be pretty obvious, when you think about it (duh).
But we don't do any checks in construction to alert you, plus we
redundantly put some of these settings on the Http2SolrClient (incl. base
class) which fools any onlooker into thinking it might do something. My
WIP addresses this by throwing an IllegalArgumentException on build() if
you try to set something that won't take effect. This revealed a number of
prominent places in Solr that both call withHttpClient and also customize
the timeouts (to no avail): Overseer, SolrClientCache (for streaming
expressions), IndexFetcher, and some recovery stuff; maybe I missed
something. This solution won't work.
Perhaps withHttpClient should not reuse the underlying HttpClient? It's a
shame to not reuse it but it's at least the safest choice. In my opinion,
withHttpClient doesn't have an ideal name either; it should be
withHttpSolrClient. Its current name is ambiguous; I hate confusing
HttpSolrClient with HttpClient. And if we copy configuration from the
client but don't actually use it, then maybe we should name this method
copyConfigurationFrom or withConfigurationFrom.
I'll throw up a draft PR today that does the cleanup/harmonization but not
yet solve the above conundrum.
(CC'ed some folks that worked on timeouts)
~ David Smiley
Apache Lucene/Solr Search Developer
http://www.linkedin.com/in/davidwsmiley