Re: [PR] SOLR-17711 index fetcher doesn't need timeout [solr]

2025-05-29 Thread via GitHub


dsmiley commented on PR #3356:
URL: https://github.com/apache/solr/pull/3356#issuecomment-2919383276

   I quickly looked at Jetty HttpClient and there's no request timeout there.  
It's on the Jetty Request.


-- 
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]



Re: [PR] SOLR-17711 index fetcher doesn't need timeout [solr]

2025-05-27 Thread via GitHub


kotman12 commented on PR #3356:
URL: https://github.com/apache/solr/pull/3356#issuecomment-2914160190

   @dsmiley is the request timeout even at the HttpClient level? I didn't think 
it was sobI don't think we have an override to worry about. Also I moved the 
setting of the request timeout to the "root" HttpClient (recoveryOnlyClient) 
anyways.


-- 
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]



Re: [PR] SOLR-17711 index fetcher doesn't need timeout [solr]

2025-05-27 Thread via GitHub


dsmiley commented on PR #3356:
URL: https://github.com/apache/solr/pull/3356#issuecomment-2914072497

   Until the underlying timeout propagation is fixed, this PR is in limbo.  I 
view this as a bug, not an optimization, since the user experience is just like 
a bug.  For users with large shards, Solr worked and then later breaks after an 
upgrade.  Let's try to get this fixed in 9.9 somehow.  It's tempting to switch 
back to Apache HttpClient there, in the interests of stability / confidence.  
WDYT?  Then in parallel improve Jetty Http2SolrClient without feeling rushed 
for 9.9.
   
   CC @iamsanjay 


-- 
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]



Re: [PR] SOLR-17711 index fetcher doesn't need timeout [solr]

2025-05-27 Thread via GitHub


kotman12 commented on PR #3356:
URL: https://github.com/apache/solr/pull/3356#issuecomment-2912755279

   > Looks good, thanks. Can you please add to CHANGES.txt in the improvement 
section?
   
   I updated the CHANGES.txt under 10 but I'm wondering if this should go out 
in 9.9? I'd argue this is something between a bug and an optimization since the 
request timeouts were added _very_ likely by accident.
   
   I've also moved the override to the recovery client. Maybe that is overkill 
but, again, I want to stress that until we moved to Jetty's HttpClient there 
was no request timeout on this _at all_. 
   


-- 
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]



Re: [PR] SOLR-17711 index fetcher doesn't need timeout [solr]

2025-05-19 Thread via GitHub


kotman12 commented on PR #3356:
URL: https://github.com/apache/solr/pull/3356#issuecomment-2891608663

   @dsmiley Just saw your message to the mailing list, thanks. So it looks like 
what I propose here is incidentally a no-op as it currently stands? Should I 
just wait for your PR then? Looks like I'd have to make this change to the 
`recoveryOnlyClient` which may or may not be the right thing to do but the 
question might be moot if we apply your planned changes.


-- 
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]