[
https://issues.apache.org/jira/browse/SOLR-18600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18136975#comment-18136975
]
Nick Shanin commented on SOLR-18600:
------------------------------------
[~dsmiley], [~epugh]: flagging this at low priority. It is not blocking
anything of mine.
This came up while I was checking the SOLR-17968 client refactoring for
downstream binary compatibility, as a prerequisite question for design work
around the HTTP client layer. A small consumer compiled against released 10.0.0
fails on published 10.1.0 with {{NoSuchMethodError}}: removing
{{HttpSolrClientBase}} and {{HttpSolrClientBuilderBase}} changed declared types
across the Cloud and LB clients.
The tradeoffs as I see them: on one side, it breaks the promise that a minor
release keeps binary compatibility, and that promise is why people upgrade a
minor release without a second thought. On the other side, nobody else has
reported it, the failure only bites bytecode that references the removed types
directly (the builder base classes and the changed Cloud and LB signatures),
ordinary client code recompiles and runs unchanged, and a recompile cures
anyone who does hit it. The removed classes are plumbing most user code never
names. So the practical damage so far looks like none, and the open question is
whether the principle is worth acting on.
Two things are prepared, if either is wanted:
* A restoration branch on branch_10x that puts the released declared types
back:
[solr-18600-http-abi-restore-submit|https://github.com/nick-boss-tech/solr/tree/solr-18600-http-abi-restore-submit].
The consumer passes on the branch jar and fails on 10.1.0.
* A japicmp comparison of the SolrJ and solr-api jars against the previous
release. Pointed at 10.0.0 it flags this break exactly (both classes show as
removed); at the restored head it passes.
The comparison does not have to live in the build. It could just as well be a
pre-release checklist item: run it against the previous release before a
release is cut, and treat any binary-incompatible line as a release decision
instead of a build failure. I am not bothered either way; I am posting because
the testing turned it up.
🤖 _AI text above_ 🤖 _(reviewed by Nick Shanin)_
> SolrJ 10.1.0 binary incompatibility: HttpSolrClientBuilderBase removal breaks
> pre-compiled builder consumers with NoSuchMethodError
> -----------------------------------------------------------------------------------------------------------------------------------
>
> Key: SOLR-18600
> URL: https://issues.apache.org/jira/browse/SOLR-18600
> Project: Solr
> Issue Type: Bug
> Components: SolrJ
> Affects Versions: 10.1
> Reporter: Nick Shanin
> Priority: Major
> Labels: binary-compatibility, solrj
>
> h2. Summary
> SolrJ 10.1.0 removed the public classes
> {{org.apache.solr.client.solrj.impl.HttpSolrClientBase}} and
> {{org.apache.solr.client.solrj.impl.HttpSolrClientBuilderBase}} (SOLR-17968).
> Code that was compiled against SolrJ 10.0.0 and uses only the ordinary
> {{HttpJdkSolrClient.Builder}} methods fails at runtime on 10.1.0 with a
> {{NoSuchMethodError}} on the first builder call, before any HTTP request is
> made. Recompiling the same source against 10.1.0 fixes it, so source
> compatibility holds; binary compatibility does not. The 10.1 upgrade notes
> mention the new base class but do not mention the removal or the need to
> recompile.
> h2. Minimal reproducer
> {code}
> import java.util.concurrent.TimeUnit;
> import org.apache.solr.client.solrj.impl.HttpJdkSolrClient;
> public class Consumer {
> public static void main(String[] args) throws Exception {
> try (HttpJdkSolrClient client =
> new HttpJdkSolrClient.Builder("http://127.0.0.1:8983/solr";)
> .withConnectionTimeout(10, TimeUnit.SECONDS)
> .withRequestTimeout(60, TimeUnit.SECONDS)
> .build()) {
> System.out.println("BUILT " + client.getClass().getName());
> }
> System.out.println("CONSUMER OK");
> }
> }
> {code}
> The source names no base type and uses no internal API. Compiled against
> 10.0.0, {{javap}} shows the fluent calls carry descriptors naming the removed
> class, for example
> {{HttpJdkSolrClient$Builder.withConnectionTimeout:(JLjava/util/concurrent/TimeUnit;)Lorg/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase;}},
> because the methods are declared on the removed base class and javac emits
> its erased descriptor.
> h2. Tested coordinates
> * {{org.apache.solr:solr-solrj:10.0.0}} (jar sha256
> {{59178b49e43ffed3d4845b73c0b3302bd77e1b0e3c10c5c4550ba8499bdb3fac}}):
> contains both classes.
> * {{org.apache.solr:solr-solrj:10.1.0}} (jar sha256
> {{115fcfbe1ed8ce04f7da8e200d20ce9105a567b40d6ec33727fcbd0667118bb4}}):
> contains neither class; first published release with the removal.
> * Consumer class compiled with {{javac --release 17}} against 10.0.0, sha256
> {{a3bb85e5a58d39e226627df1340844d758471e48b7514ac137364cdea485801f}}. JDK
> used for the runs: Temurin 21.0.12.1. Runtime classpath: solrj, solr-api,
> slf4j-api. No server is needed; the failure precedes any HTTP operation, and
> the Jackson change in SOLR-18325 is not required to reproduce it.
> h2. Expected versus observed
> * Expected: bytes compiled against 10.0.0 run against 10.1.0, or the removal
> is called out in the 10.1 upgrade notes as requiring recompilation.
> * Observed on 10.0.0: {{BUILT
> org.apache.solr.client.solrj.impl.HttpJdkSolrClient}} and {{CONSUMER OK}},
> exit 0.
> * Observed on 10.1.0, identical bytes: exit 1 with
> {noformat}
> java.lang.NoSuchMethodError:
> 'org.apache.solr.client.solrj.impl.HttpSolrClientBuilderBase
> org.apache.solr.client.solrj.impl.HttpJdkSolrClient$Builder.withConnectionTimeout(long,
> java.util.concurrent.TimeUnit)'
> at Consumer.main(Consumer.java:8)
> {noformat}
> * The identical source recompiled against 10.1.0 compiles and runs correctly,
> which isolates the break to binary compatibility. A control consumer that
> uses only {{Builder.withHttpClient(...)}} (whose descriptors name only
> concrete types) runs unchanged on both versions.
> h2. Context and open question
> SOLR-17968 and [PR #3829|https://github.com/apache/solr/pull/3829] renamed
> the classes deliberately (main {{8d3f995c6a}}, branch_10x {{e5340a954b}}).
> Neither the ticket, the PR discussion, nor the upgrade notes mention the
> effect on already-compiled consumers. In 10.0.0, {{HttpSolrClientBase}} is
> marked {{@lucene.internal}} in its Javadoc; {{HttpSolrClientBuilderBase}},
> the class ordinary bytecode actually names, is not. Whether that makes the
> break acceptable in a minor release is the policy question this ticket poses;
> the runtime behavior above is the observed defect either way.
> Possible directions, kept separate from the defect report: restore the two
> classes as compatibility types so the released descriptors resolve, or
> document the removal and the recompile requirement in the 10.1 upgrade notes
> if the break is accepted.
> h2. Prepared fix
> A fix is prepared on branch
> [solr-18600-http-abi-restore-submit|https://github.com/nick-boss-tech/solr/tree/solr-18600-http-abi-restore-submit]
> at commit
> [c40665af8d6|https://github.com/nick-boss-tech/solr/commit/c40665af8d6bd23c85fef565f93de5004a7b2ca4],
> based on branch_10x.
> It restores {{HttpSolrClientBase}} and {{HttpSolrClientBuilderBase}} as
> top-level classes with the released fields, constructor and members;
> {{HttpSolrClient}} and its nested {{BuilderBase}} extend them; the released
> declared types are restored on the neighboring classes (CloudSolrClient,
> CloudHttp2SolrClient, HttpClusterStateProvider, LBSolrClient.Builder,
> LBAsyncSolrClient, ConcurrentUpdateBaseSolrClient.Builder); and the
> {{SolrRequest.processWithBaseUrl(HttpSolrClientBase, ...)}} overload is
> re-added.
> Verified at that head: the new HttpSolrClientBinaryCompatTest and
> HttpJettySolrClientBinaryCompatTest pass, as do HttpJdkSolrClientTest,
> HttpJettySolrClientTest and the Cloud, LB and ConcurrentUpdate suites, and
> :solr:solrj:check and :solr:solrj-jetty:check pass. The consumer bytes
> compiled against SolrJ 10.0.0 that fail on 10.1.0 with the NoSuchMethodError
> above run correctly against the patched build. A PR against branch_10x can be
> opened on request.
> 🤖 _AI text above_ 🤖 _(reviewed by Nick Shanin)_
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
