I have too little time to do more so the current state is it for me. ~ David Smiley Apache Lucene/Solr Search Developer http://www.linkedin.com/in/davidwsmiley
On Wed, Mar 23, 2022 at 9:05 AM Jan Høydahl <[email protected]> wrote: > 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 > > > On Tue, Mar 22, 2022 at 10:16 AM Jan Høydahl <[email protected]> > wrote: > >> Hi all, >> >> Please help close this last(?) 9.0 blocker by reviewing >> https://github.com/apache/solr/pull/750 >> >> Jan >> >> 18. mar. 2022 kl. 13:51 skrev David Smiley <[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 >> >> >> On Fri, Mar 18, 2022 at 12:28 AM David Smiley <[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 >>> >>> >>> On Thu, Mar 17, 2022 at 12:17 PM Jan Høydahl <[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]>: >>>> >>>> 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]> >>>> 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]>: >>>>> >>>>> "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 >>>>> >>>>> >>>>> On Wed, Mar 16, 2022 at 4:53 PM Jason Gerlowski <[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]> 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]> >>>>>> wrote: >>>>>> >> >>>>>> >> On Tue, Mar 15, 2022 at 8:47 AM Jan Høydahl <[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]>: >>>>>> >>> >>>>>> >>> 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 "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 >>>>>> >>> >>>>>> >>> >>>>>> >>>>>> --------------------------------------------------------------------- >>>>>> To unsubscribe, e-mail: [email protected] >>>>>> For additional commands, e-mail: [email protected] >>>>>> >>>>>> >>>>> >>>> >> >
