SOLR-17884: Don't depend on Apache HttpClient #3792
Conversation
solrj-streaming DatabaseMetaDataImpl ZkController ConcurrentUpdateHttp2SolrClient (jetty)
dsmiley
left a comment
dsmiley
left a comment
There was a problem hiding this comment.
I don't know how I missed this in the earlier PR for this JIRA issue where I thought I completely removed all direct/indirect Apache HttpClient references. Moving stuff to another module revealed these forgotten things.
| // the | ||
| // leader now | ||
| // the leader now | ||
| // TODO .withHttpClient(getCoreContainer().getDefaultHttpSolrClient()) |
There was a problem hiding this comment.
Preferable; however that means accepting its connection timeout. Heck; wouldn't need to create a new Http2SolrClient either; just use the default client. Personally I'd prefer that but... 🤷
| import org.apache.solr.client.solrj.SolrClient; | ||
| import org.apache.solr.client.solrj.SolrRequest; | ||
| import org.apache.solr.client.solrj.SolrServerException; | ||
| import org.apache.solr.client.solrj.impl.ConcurrentUpdateSolrClient.Update; |
There was a problem hiding this comment.
Don't want to use that internal POJO, so I created a similar record, copying the same javadoc line as well.
| .withConnectionTimeout(8000, TimeUnit.MILLISECONDS) | ||
| .withSocketTimeout(30000, TimeUnit.MILLISECONDS) | ||
| .withIdleTimeout(30000, TimeUnit.MILLISECONDS) | ||
| .build()) { |
There was a problem hiding this comment.
What about the security listeners?
There was a problem hiding this comment.
I changed it now. It's a trade-off but at least the listener issue could be a real bug
|
Planning to merge tonight (12 hours; 9:22 EST). Happy to pause for any reason. |
* ZkController. Unfortunately using longer connection timeout (30 not 8) but at least now we re-use an internal client; get listeners for observability & security. * SolrJ ConcurrentUpdateHttp2SolrClient (jetty). Used an inner class of a deprecated client. * solrj-streaming DatabaseMetaDataImpl
* ZkController. Unfortunately using longer connection timeout (30 not 8) but at least now we re-use an internal client; get listeners for observability & security. * SolrJ ConcurrentUpdateHttp2SolrClient (jetty). Used an inner class of a deprecated client. * solrj-streaming DatabaseMetaDataImpl (cherry picked from commit 5d8a539)
|
Wooot 🎉 |
* ZkController. Unfortunately using longer connection timeout (30 not 8) but at least now we re-use an internal client; get listeners for observability & security. * SolrJ ConcurrentUpdateHttp2SolrClient (jetty). Used an inner class of a deprecated client. * solrj-streaming DatabaseMetaDataImpl
3 participants
Discovered more changes needed...
https://issues.apache.org/jira/browse/SOLR-17884