If you feel inclined for a cleanup go ahead. But I think we can just as well move on as-is for now, and if we want nicer sounding client names in 10.x then re-visit.
Jan > 22. mar. 2022 kl. 16:22 skrev David Smiley <[email protected]>: > > I pushed the rename we discussed and added change notes. > > I can see that the naming/deprecation choice varies between standalone vs > SolrCloud (simple deprecation vs rename & swap). Not a problem but not > ideal. I don't care too much because a user will likely do just one or the > other and not care much about the other side. Still, for HttpSolrClient in > particular (the most common), it's not too late to un-deprecate it, and a > similar change could happen later. Interestingly, BaseHttpSolrClient has > nothing of interest, unlike the cloud side of things. > > ~ David Smiley > Apache Lucene/Solr Search Developer > http://www.linkedin.com/in/davidwsmiley > <http://www.linkedin.com/in/davidwsmiley> > > On Tue, Mar 22, 2022 at 10:16 AM Jan Høydahl <[email protected] > <mailto:[email protected]>> wrote: > Hi all, > > Please help close this last(?) 9.0 blocker by reviewing > https://github.com/apache/solr/pull/750 > <https://github.com/apache/solr/pull/750> > > Jan > >> 18. mar. 2022 kl. 13:51 skrev David Smiley <[email protected] >> <mailto:[email protected]>>: >> >> The name CloudHttp1SolrClient is understandable because we have one with >> "Http2" in the name. But our Http2 one speaks HTTP 1.1 too :-) >> I think the names CloudApacheHttpSolrClient or CloudLegacySolrClient are >> good names, and I lean to the latter because with the word "legacy" in its >> name, it screams, don't use me if you can avoid it ;-). Also, >> CloudApacheHttpSolrClient is even more of a mouthful, and it could be not so >> obvious how to parse that (to our users) since Solr is also under the ASF >> and the Http part could be sort of obvious vs what we intend to mean -- a >> specific "Apache" http client vs whatever other ones. >> >> ~ David Smiley >> Apache Lucene/Solr Search Developer >> http://www.linkedin.com/in/davidwsmiley >> <http://www.linkedin.com/in/davidwsmiley> >> >> On Fri, Mar 18, 2022 at 12:28 AM David Smiley <[email protected] >> <mailto:[email protected]>> wrote: >> Thank you for accepting my proposal -- I definitely volunteer to implement >> it! >> >> ETA: I've started... I should have a PR to share this weekend. >> >> One thing I want to point out that I see is that, as tempting as it may be, >> all the places inside Solr that call the existing Http1 (using Apache >> HttpClient) based builder will *continue* to do so. Migrating to Http2 is >> out of scope of this issue. There are risks around authentication >> propagation since there are known gaps there. There will be some judgement >> calls as to which internal method signatures should take CloudSolrClient or >> CloudHttp1SolrClient but I lean to keep CloudSolrClient and do a bit of >> casting on occasion when necessary (e.g. to access the HttpClient inside). >> >> ~ David Smiley >> Apache Lucene/Solr Search Developer >> http://www.linkedin.com/in/davidwsmiley >> <http://www.linkedin.com/in/davidwsmiley> >> >> On Thu, Mar 17, 2022 at 12:17 PM Jan Høydahl <[email protected] >> <mailto:[email protected]>> wrote: >> To wrap up: >> >> David's proposal: >> * Un-deprecate CloudSolrClient to use it as the main cluster client going >> forward >> * Rename CloudSolrClient -> CloudHttp1SolrClient and rename >> BaseCloudSolrClient -> CloudSolrClient >> * Make a new Builder that will instantiate a CloudSolrClient instance with a >> LBHttp2SolrClient / Jetty client backing it >> Users who want / need to use the old apache clients will now use >> CloudHttp1SolrClient's Builder instead >> * CloudHttp2SolrClient will remain (but can be deprecated?) >> >> Most SolrJ users will need to adapt their app when upgrading to solrj 9.0, >> but we are willing to accept that even if things are not pre-announced with >> deprecations. >> We can introduce some deprecations and "bridge" code in 8.11.2 if we want to >> provide a smoother path. >> >> I'm also willing to accept such a compat break, given that users can still >> use 8.11 solrj with solr9 as a bridge, and that we document the changes. >> >> David, I assume you were volunteering to land the proposed refactorings? :) >> Do you have an ETA? >> >> >> Can someone also please also comment on whether the rest of the deprecations >> look ok? The Auth stuff is closely tied to apache-http-client so will need >> to switch to Jetty-client before 10.0 if we're going to get rid of the >> dependency from Solr.: >> >> ConcurrentUpdateSolrClient >> HttpClientUtil >> HttpClusterStateProvider >> HttpSolrClient >> Krb5HttpClientBuilder >> LBHttpSolrClient >> PreemptiveAuth >> PreemptiveBasicAuthClientBuilderFactory >> SolrClientBuilder >> SolrHttpClientBuilder >> SolrHttpClientContextBuilder >> SolrHttpRequestRetryHandler >> >> Jan >> >>> 17. mar. 2022 kl. 16:47 skrev Houston Putman <[email protected] >>> <mailto:[email protected]>>: >>> >>> I think it's fine to change the SolrJ code in 9.0, it's a major version and >>> we are not doing it for a silly reason. >>> >>> As long as we document the changes well (maybe we need a separate page for >>> Major changes in SolrJ-9), I don't see a reason why we can't make these >>> changes. >>> >>> It could be that we should be even bolder in 10.0 and provide a more modern >>> Cluster SolrJ client that supports an instant pub/sub over HTTP/2 for >>> clusterstate changes (i.e. a push from Solr server to client over HTTP/2), >>> eliminating the need for user apps talking to Zookeeper at all. That would >>> also make it easier for 3rd party clients to implement a good Solr client. >>> >>> That sounds like a great idea (would love to eliminate the need for users >>> to talk to ZK). >>> >>> On Thu, Mar 17, 2022 at 8:54 AM Jan Høydahl <[email protected] >>> <mailto:[email protected]>> wrote: >>> One simple solution is to revert SOLR-15223 Deprecate HttpSolrClient and >>> friends in 9.0, do the 9.0 release and then continue planning for the >>> next-gen Cloud client. >>> >>> It could be that we should be even bolder in 10.0 and provide a more modern >>> Cluster SolrJ client that supports an instant pub/sub over HTTP/2 for >>> clusterstate changes (i.e. a push from Solr server to client over HTTP/2), >>> eliminating the need for user apps talking to Zookeeper at all. That would >>> also make it easier for 3rd party clients to implement a good Solr client. >>> >>> Jan >>> >>>> 17. mar. 2022 kl. 04:40 skrev David Smiley <[email protected] >>>> <mailto:[email protected]>>: >>>> >>>> "ClusterSolrClient" is a fine name but we already have a fine name that >>>> users are using. Waiting till 10.0 is depressing to me, particularly >>>> because it seems unnecessary. Is there disagreement that the possibility >>>> of some users having to change something is too much to ask in a major >>>> version? >>>> >>>> ~ David Smiley >>>> Apache Lucene/Solr Search Developer >>>> http://www.linkedin.com/in/davidwsmiley >>>> <http://www.linkedin.com/in/davidwsmiley> >>>> >>>> On Wed, Mar 16, 2022 at 4:53 PM Jason Gerlowski <[email protected] >>>> <mailto:[email protected]>> wrote: >>>> > We can only hypothesize _why_ a client is dependent in the first place >>>> > ... Perhaps to tweak/tune advanced options, timeouts. Perhaps to >>>> > instrument mTLS details >>>> >>>> Another use-case to add to this list would be auth settings. I'm >>>> struggling to come up with a concrete example this minute, but I know >>>> I've written SolrJ code that customized the underlying HttpClient for >>>> auth-related purposes. >>>> >>>> > "CloudSolrClient" is an intuitive/obvious name to a user that wants to >>>> > talk to SolrCloud [...] HTTP protocol or wether the client is using >>>> > whatever HTTP library is all an implementation detail. >>>> >>>> +1 I like the idea of keeping implementation details out of the name >>>> of any types we're putting front-and-center for SolrJ users. But I >>>> share Jan's concern about breaking clients who rely on a particular >>>> underlying client type. >>>> >>>> My favorite idea so far is probably Jan's point that balancing those >>>> two gets a lot easier if we introduce some "new" name like >>>> "ClusterSolrClient" as the long term successor to >>>> CloudSolrClient/BaseCloudSolrClient. It'd be nice to keep the name >>>> 'CloudSolrClient' itself for the sake of continuity, but >>>> ClusterSolrClient at least preserves the reasons we like >>>> 'CloudSolrClient' as a name and it makes keeping backcompat pretty >>>> easy: >>>> >>>> I guess concretely, this would look something like: >>>> >>>> 1. Create a new class, 'ClusterSolrClient', that's a trivial extension >>>> of BaseCloudSolrClient. (i.e. `class ClusterSolrClient extends >>>> BaseCloudSolrClient {}`) >>>> 2. Add a builder for the new 'ClusterSolrClient' that can create >>>> either the apache or jetty-powered CloudSolrClient based on the >>>> builder methods invoked. >>>> 3. Deprecate BaseCloudSolrClient, CloudSolrClient, CSC.Builder, and >>>> CloudHttp2SolrClient.Builder for 9.0, directing users over to the new >>>> ClusterSolrClient and its builder. >>>> 4. Remove the deprecated classes in 10.0 >>>> >>>> Does something like this sound do-able? >>>> >>>> Jason >>>> >>>> On Wed, Mar 16, 2022 at 10:50 AM Mike Drob <[email protected] >>>> <mailto:[email protected]>> wrote: >>>> > >>>> > I feel like CloudSolrClient doesn't imply anything about HTTP 1 or 2, >>>> > anything about Apache or Jetty (or java.net.http). If we have exposed >>>> > those internal details in some ways, then that is unfortunate and should >>>> > be addressed. >>>> > >>>> > I personally never use CloudHttp2SolrClient because I kind of assumed >>>> > that it was an implementation detail and the various builders would give >>>> > me the http2 client when I needed it. Maybe that's not the case. I've >>>> > never thought about it too much. CloudSolrClient looks like the >>>> > "simpler" one to use so that's what people gravitate towards. >>>> > >>>> > A quick look in my editor suggests that we have 100 uses of >>>> > CloudSolrClient, including some in the ref guide. If we want to >>>> > deprecate this, then we should update our documentation to guide people >>>> > away from it as well. I suspect that if we try to examine which uses of >>>> > CloudSolrClient in our code could just be SolrClient, we wouldn't make >>>> > much progress on this though. >>>> > >>>> > I know this isn't offering much in the way of solutions, but I'm mostly >>>> > trying to say that I agree it is a problem. >>>> > >>>> > >>>> > Mike >>>> > >>>> > On Wed, Mar 16, 2022 at 12:05 AM David Smiley <[email protected] >>>> > <mailto:[email protected]>> wrote: >>>> >> >>>> >> On Tue, Mar 15, 2022 at 8:47 AM Jan Høydahl <[email protected] >>>> >> <mailto:[email protected]>> wrote: >>>> >>> >>>> >>> I re-opened SOLR-15223 to highlight that we are still blocked by this >>>> >>> decision. >>>> >>> >>>> >>> I don't clearly see the full effects of your suggestion right now. >>>> >>> Does your proposal also involve deprecating CloudHttp2SolrClient as a >>>> >>> separate class? >>>> >> >>>> >> >>>> >> No; it would stay. Perhaps ideally it would have a name reflecting it >>>> >> uses the Jetty client but no big deal; it can stay as-is. Its name >>>> >> already isn't necessarily true; you can use this class (and thus the >>>> >> Jetty client) and tell it not to use Http2 :-). I'm reminded that >>>> >> HdfsDirectory doesn't require HDFS :-). (It requires the HDFS client >>>> >> libs but not necessarily an HDFS backend, if you're curious). >>>> >> >>>> >>> >>>> >>> I would imagine users with existing SolrJ code would after upgrading >>>> >>> get an instance of BaseCloudSolrClient (with a new name) using Jetty >>>> >>> client under the hood? What if that application code assumes >>>> >>> org.apache.http as client and tries to obtain HttpSolrClient and even >>>> >>> org.apache.http objects based on CloudSolrClient? The code would fail >>>> >>> since the contract is broken. >>>> >> >>>> >> >>>> >> If the client/user truly assumes org.apache.http well clearly they will >>>> >> be disrupted by this change. You want to call that a "contract" -- >>>> >> shrug; I call it an implementation detail that can change :-). They >>>> >> may be calling getLbClient and may be using the LBHttpSolrClient >>>> >> subclass of LBSolrClient, perhaps. Or similarly specifying builder >>>> >> options relating to this advanced option. It's possible and it's >>>> >> undeniable _some_ clients will be impacted. We can only hypothesize >>>> >> _why_ a client is dependent in the first place (vs. perhaps an >>>> >> accidental dependency/assumption e.g. in dependency management). >>>> >> Perhaps to tweak/tune advanced options, timeouts. Perhaps to >>>> >> instrument mTLS details; although I know from experience it can be done >>>> >> without calling special methods on builders; it can be done via setting >>>> >> special system properties referring to one's own classes that are >>>> >> called in certain ways. If you do that (and we have), the way to do it >>>> >> for the Apache based client differs from Jetty; we've done it for both >>>> >> because Solr uses both internally. Anyway, this is off the beaten path >>>> >> of most users. >>>> >> >>>> >>> >>>> >>> >>>> >>> With the current pure deprecation and switch to CloudHttp2SolrClient, >>>> >>> existing users' code would continue to work.. >>>> >> >>>> >> >>>> >> Hey, this is a major release; let's not hold ourselves to a standard >>>> >> that is too onerous for us to maintain. We can make our intentions >>>> >> clear in upgrade notes. >>>> >> >>>> >> ~ David >>>> >> >>>> >>> >>>> >>> Jan >>>> >>> >>>> >>> >>>> >>> 14. mar. 2022 kl. 15:40 skrev David Smiley <[email protected] >>>> >>> <mailto:[email protected]>>: >>>> >>> >>>> >>> I want to bring an important SolrJ decision to the dev list. >>>> >>> >>>> >>> There's a JIRA issue https://issues.apache.org/jira/browse/SOLR-15223 >>>> >>> <https://issues.apache.org/jira/browse/SOLR-15223> "Deprecate >>>> >>> HttpSolrClient and friends in 9.0" >>>> >>> >>>> >>> Sounds great by the title -- we want to transition over time to the >>>> >>> Jetty client instead. Jan submitted a PR to deprecate CloudSolrClient >>>> >>> and some others, and I approved it because these classes intimately >>>> >>> assume the Apache HttpClient. It's merged. >>>> >>> >>>> >>> But I have serious doubts now and wish to discuss it with the dev >>>> >>> list. Copying my last message on the issue: >>>> >>> >>>> >>>> Now that I'm "seeing" the results of this in my IDE, seeing the >>>> >>>> cross-through of deprecated usage on innocent looking classes like >>>> >>>> CloudSolrClient in particular, I have doubts on the approach. >>>> >>>> "CloudSolrClient" is an intuitive/obvious name to a user that wants >>>> >>>> to talk to SolrCloud. The particulars of which HTTP protocol or >>>> >>>> wether the client is using whatever HTTP library is all an >>>> >>>> implementation detail. Ideally such decisions would be done in the >>>> >>>> builder, either a common builder or if not then a builder specific to >>>> >>>> those libraries if needed (less nice but acceptable IMO). >>>> >>>> >>>> >>>> The easiest way to get there is to rename CloudSolrClient to >>>> >>>> CloudHttp1SolrClient in one commit (merge it) and then rename >>>> >>>> BaseCloudSolrClient to simply CloudSolrClient in the next. Then add a >>>> >>>> Builder to this class that is the one in Http2; subclass it or >>>> >>>> something (details TBD). >>>> >>>> >>>> >>>> WDYT? >>>> >>>> >>>> >>>> Of course, today they are separated by their classes. Maybe we should >>>> >>>> simply convey the deprecation intent in the upgrade notes as an >>>> >>>> advanced warning, but not deprecate CloudSolrClient in particular. >>>> >>> >>>> >>> >>>> >>> Jan replied: >>>> >>> >>>> >>>> Since we did not deprecate these in 8.x, we still have a back-compat >>>> >>>> promise to keep these classes around in 9.x, and thus also the old >>>> >>>> http client. But perhaps we are breaking that promise already in >>>> >>>> SOLR-16061, so maybe we can change even more >>>> >>>> >>>> >>>> I don't like the CloudHttp2SolrClient naming either, would prefer the >>>> >>>> Http client to be abstracted away so that it could be swapped out as >>>> >>>> an impl detail, but it was not designed that way. I fear that >>>> >>>> re-using the same class name but with slightly different contract is >>>> >>>> harder to explain than a clear deprecation message in the IDE >>>> >>>> pointing you to the replacement. >>>> >>>> >>>> >>>> Perhaps the one client to rule them all in 10.0 should be >>>> >>>> ClusterSolrClient? And aim for it to be constructed with either a >>>> >>>> Jetty client or JDK8-HttpClient as backbone through different >>>> >>>> factories/builder? >>>> >>> >>>> >>> >>>> >>> How is the contract between CloudSolrClient & BaseCloudSolrClient >>>> >>> different Jan? I suspect if there's breakage, it'd be relatively >>>> >>> obscure options on the builder. >>>> >>> >>>> >>> ~ David Smiley >>>> >>> Apache Lucene/Solr Search Developer >>>> >>> http://www.linkedin.com/in/davidwsmiley >>>> >>> <http://www.linkedin.com/in/davidwsmiley> >>>> >>> >>>> >>> >>>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe, e-mail: [email protected] >>>> <mailto:[email protected]> >>>> For additional commands, e-mail: [email protected] >>>> <mailto:[email protected]> >>>> >>> >> >
