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]>: > > 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]> >>> >> >
