[ 
https://issues.apache.org/jira/browse/SOLR-17256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17882039#comment-17882039
 ] 

Jason Gerlowski commented on SOLR-17256:
----------------------------------------

bq.  [David] I think there's a conceptual discordance of placing this concept 
on SolrRequest given that some clients want to route requests to nodes they 
choose.

bq. [Sanjay] I believe managing the URI should be the responsibility of the 
request, not the client. A good example of this is the JDK 11 Http package

I definitely understand the draw, conceptually, to have SolrRequest manage the 
base URL.  It's a nice and tidy "separation of concerns"!

But it'd undermine much of the value-add that SolrJ brings.  Client-side load 
balancing, topology-aware routing, "zombie node" detection, "new node" 
detection - all of the interesting and cool things that SolrClient impl's do 
are somewhat dependent on their freedom to manage and choose the base URL.

bq. [David] I'm imagining request(String baseUrl, SolrRequest<?> solrRequest, 
String collection)

This would get the job done, and I'm +1 on it if someone pursues it. (Ideally 
with an "experimental" designation, or named as "requestInternal" as David 
suggested).

But it has one downside that I'd love to find a way around: it exacerbates the 
"[method explosion|https://issues.apache.org/jira/browse/SOLR-12502]"; problem 
we've been having with our SolrClients.  Maybe we start with just "request(url, 
solrRequest, collection)", but next week someone will want an "async" version 
of that method.  And then there's all of the add(), delete(), query() 
convenience methods...

Wdyt about addressing this by having the new SolrClient method take a lambda?  
That'd save us from needing multiple "versions" of the method to handle sync, 
async, etc.  The new method would "just" focus on the bit it actually cares 
about (hooking in the alternate base URL) and then invokes the lambda.

{code}
final var asyncFuture = solrClient.withBaseUrl("http://alt-host:7574/solr";, 
(client) -> {
  return client.requestAsync(solrRequest, "techproducts");
});
...
final var updateRsp = solrClient.withBaseUrl("http://update-host:8983/solr";, 
(client) -> {
  return updateReq.process(client);
});
{code}

In terms of the "withBaseUrl" implementation itself, it could largely just be a 
"try with resources"-like wrapper block that:
# sets a thread-local used by ClientUtils.buildRequestForUrl
# invokes the lambda, and then...
# cleans up the thread-local

> Remove SolrRequest.getBasePath setBasePath
> ------------------------------------------
>
>                 Key: SOLR-17256
>                 URL: https://issues.apache.org/jira/browse/SOLR-17256
>             Project: Solr
>          Issue Type: Improvement
>          Components: SolrJ
>            Reporter: David Smiley
>            Priority: Minor
>              Labels: newdev
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> SolrRequest has a getBasePath & setBasePath.  The naming is poor; it's the 
> URL base to the Solr node like "http://localhost:8983/solr";.  It's only 
> recognized by HttpSolrClient; LBSolrClient (used by CloudSolrClient) ignores 
> it and will in fact mutate the passed in request to its liking, which is 
> rather ugly because it means a request cannot be used concurrently if the 
> user wants to.  But moreover I think there's a conceptual discordance of 
> placing this concept on SolrRequest given that some clients want to route 
> requests to nodes *they* choose.  I propose removing this from SolrRequest 
> and instead adding a method specific to HttpSolrClient.  Almost all existing 
> usages of setBasePath immediately execute the request on an HttpSolrClient, 
> so should be easy to change.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to