Skip to content

SOLR-18401: HttpJettySolrClient: detect if request wasn't sent; retry - #4805

Open
dsmiley wants to merge 4 commits into
apache:mainfrom
dsmiley:SOLR-18401-retry-unsent-request
Open

SOLR-18401: HttpJettySolrClient: detect if request wasn't sent; retry#4805
dsmiley wants to merge 4 commits into
apache:mainfrom
dsmiley:SOLR-18401-retry-unsent-request

Conversation

@dsmiley

@dsmiley dsmiley commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

CloudSolrClient & LBSolrClient will detect it and retry.

https://issues.apache.org/jira/browse/SOLR-18401 (read it; very detailed)

… retry-able

CloudSolrClient & LBSolrClient will detect it.

@dsmiley dsmiley left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The try-catch logic here is a challenge; not sure if we can make it elegant/clearer.

*/
public class RequestNotSentException extends IOException {

private static final long serialVersionUID = 1L;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm; I should probably remove

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nah it's common/standard actually and of course harmless

Comment on lines +39 to +42
private static final LBSolrClient.Endpoint FIRST =
new LBSolrClient.Endpoint("http://127.0.0.1:1/solr");
private static final LBSolrClient.Endpoint SECOND =
new LBSolrClient.Endpoint("http://127.0.0.1:2/solr");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recall we have a test fixture/utility with bogus URLs we should use instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes (in STCJ4) but they needn't be referenced so I didn't

@HoustonPutman HoustonPutman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I wish these if/else statements could get easier, but not a blocker for the PR.

@dsmiley

dsmiley commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review.

Tweaked the odd naming choices in the test to be clearer to me.
Harmonized the logic in the catch blocks slightly (pure refactoring) for clarity.
Gonna merge this evening and hope it improves flaky builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants