SOLR-15733: Separate out a solrj-streaming module #1099

joel-bernstein · 2022-10-21T15:40:28Z

https://issues.apache.org/jira/browse/SOLR-15733

joel-bernstein · 2022-10-21T15:41:53Z

First step of migrating Streaming Expressions to solrj-streaming.

joel-bernstein · 2022-10-21T17:12:54Z

The latest commit allows Solr to assemble and build the new solrj-streaming jar. The distribution starts and streaming expressions run. So it's getting closer. Next I'll dig into the tests.

solr/solrj/build.gradle

solr/solrj-streaming/build.gradle

joel-bernstein · 2022-10-21T18:43:23Z

solrj-streaming tests are passing now. Checking the entire test suite, there will be failures in places that expect to find Streaming Expressions in solrj.

solr/solrj-streaming/build.gradle

joel-bernstein · 2022-10-21T19:18:56Z

The full test suit is passing except for the following which fails everytime and seems to be unrelated.

2> 32414 INFO (TEST-PerReplicaStatesIntegrationTest.testPerReplicaStateCollection-seed#[D7017DB2FFB75346]) [] o.a.s.SolrTestCaseJ4 ###Ending testPerReplicaStateCollection

org.apache.solr.client.solrj.impl.BaseHttpSolrClient$RemoteSolrException: Error from server at http://127.0.0.1:65178/solr: Invalid replica : core_node8 in shard/collection : shard1/perReplicaState_test available replicas are core_node2,core_node6,core_node10
    at __randomizedtesting.SeedInfo.seed([D7017DB2FFB75346:FD3CD6F05F17894B]:0)
    at app//org.apache.solr.client.solrj.impl.HttpSolrClient.executeMethod(HttpSolrClient.java:720)
    at app//org.apache.solr.client.solrj.impl.HttpSolrClient.request(HttpSolrClient.java:234)
    at app//org.apache.solr.client.solrj.impl.HttpSolrClient.request(HttpSolrClient.java:215)
    at app//org.apache.solr.client.solrj.impl.LBSolrClient.doRequest(LBSolrClient.java:405)
    at app//org.apache.solr.client.solrj.impl.LBSolrClient.request(LBSolrClient.java:371)
    at app//org.apache.solr.client.solrj.impl.CloudSolrClient.sendRequest(CloudSolrClient.java:1174)
    at app//org.apache.solr.client.solrj.impl.CloudSolrClient.requestWithRetryOnStaleState(CloudSolrClient.java:880)
    at app//org.apache.solr.client.solrj.impl.CloudSolrClient.request(CloudSolrClient.java:807)
    at app//org.apache.solr.client.solrj.SolrRequest.process(SolrRequest.java:234)
    at app//org.apache.solr.client.solrj.SolrRequest.process(SolrRequest.java:249)
    at app//org.apache.solr.common.cloud.PerReplicaStatesIntegrationTest.testPerReplicaStateCollection(PerReplicaStatesIntegrationTest.java:91)
    at java.base@11.0.9/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base@11.0.9/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base@11.0.9/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base@11.0.9/java.lang.reflect.Method.invoke(Method.java:566)
    at app//com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1758)

risdenk · 2022-10-26T14:15:33Z

@joel-bernstein that error was being looked at. it should be fixed on main. I can help fix the merge conflicts since they are due to some of the changes I've made.

risdenk · 2022-10-26T17:26:23Z

FWIW on your branch ran into when running ./gradlew check -x test -Pvalidation.errorprone=true

* What went wrong:
Execution failed for task ':solr:solrj-streaming:analyzeTestClassesDependencies'.
> Dependency analysis found issues.
  usedUndeclaredArtifacts
   - com.carrotsearch.randomizedtesting:randomizedtesting-runner:2.8.1@jar
   - org.apache.lucene:lucene-test-framework:9.4.0@jar
   - org.apache.solr:solrj-streaming:10.0.0-SNAPSHOT@
  unusedDeclaredArtifacts
   - com.google.guava:guava:31.1-jre@jar
   - commons-io:commons-io:2.11.0@jar
   - org.apache.zookeeper:zookeeper:3.8.0@jar

risdenk · 2022-10-26T17:41:58Z

@joel-bernstein I merged main, fixed the dependency issues, and a ref guide link. ./gradlew check -x test -Pvalidation.errorprone=true now passes and ./gradlew -p solr/solrj-streaming check passes.

risdenk

I wonder if testRuntimeOnly "org.hsqldb:hsqldb" // runtime because via JDBC reflection is still needed in solr/solrj/build.gradle

risdenk · 2022-10-26T18:05:17Z

@joel-bernstein all tests pass too!

./gradlew clean
...
./gradlew check -Pvalidation.errorprone=true
...
BUILD SUCCESSFUL in 19m 13s
610 actionable tasks: 556 executed, 54 up-to-date

joel-bernstein · 2022-10-27T20:24:10Z

@risdenk, thanks for the commits on this! I pulled all the latest and things look good.

Does the solrj build.gradle need to refer to solrj-streaming so 9x users don't have a back compat issue?

risdenk · 2022-10-27T20:45:27Z

@joel-bernstein I'd do that separately on branch_9x only - similar to 8088917

joel-bernstein · 2022-10-27T21:31:39Z

I don't have the merge button. Let me see if I can figure out why.

joel-bernstein · 2022-10-27T21:53:12Z

I checked my account at id.apache.org and my github account is there. So this may take a jira to infra to sort out.

I have this branch squash merged to main locally and am running tests. I can push it out from my local repo or @risdenk you can merge from here.

joel-bernstein · 2022-10-27T22:00:32Z

I am also a member of the apache organization on github.

risdenk · 2022-10-28T02:22:00Z

@joel-bernstein I don't see you listed here https://github.com/orgs/apache/teams/solr-committers/members?query=joel - its a private list if you are on it you can see it. I see your username here https://whimsy.apache.org/roster/committer/jbernste

so not sure what is going on

dsmiley

solrj/build.gradle shouldn't need hsqldb anymore; right? And it shouldn't depend on 'solr:modules:sql' if the corresponding tests have been moved.

janhoy

I assume this is the PR that will stay in main branch, since there is no dependency added in solrj's build.gradle to depend on solrj-streaming?

Should there not be some refguide docs in streaming about the need to add solrj-streaming as a dependency to your app if you need the IO classes?

I also assume there will be a backport PR that adds the transitive dependency from solrj to solrj-zookeeper, perhaps along with a note in the Upgrade Notes for 9.2 that you can now opt out of solrj-streaming on client if you don't use it, simply by excluding the coordinates in your POM.

joel-bernstein · 2022-10-28T09:44:15Z

I'm not able to see https://github.com/orgs/apache/teams/solr-committers/members?query=joel which must be the reason I'm not seeing the merge button. Is this something that infra needs to fix or does the Solr pmc manage this list?

joel-bernstein · 2022-10-28T09:53:22Z

@dsmiley I believe Solrj no long needs the hsql dependency. The solrj-streaming dependency on solr:modules:sql I believe is only there to support tests for the JDBCStream and JDBC driver. Both of which could eventually be moved to solrj-jdbc.

dsmiley · 2022-10-28T12:30:10Z

Shouldn't those tests move out of solrj(core) now?

risdenk · 2022-10-28T13:32:54Z

I assume this is the PR that will stay in main branch, since there is no dependency added in solrj's build.gradle to depend on solrj-streaming?

@janhoy yes - will add to branch_9x like solrj-zookeeper did - see #1099 (comment). @dsmiley asked the same question on the Jira.

Should there not be some refguide docs in streaming about the need to add solrj-streaming as a dependency to your app if you need the IO classes?

good catch will need to update.

I also assume there will be a backport PR that adds the transitive dependency from solrj to solrj-zookeeper, perhaps along with a note in the Upgrade Notes for 9.2 that you can now opt out of solrj-streaming on client if you don't use it, simply by excluding the coordinates in your POM.

yup makes sense.

solrj/build.gradle shouldn't need hsqldb anymore; right? And it shouldn't depend on 'solr:modules:sql' if the corresponding tests have been moved.

@joel-bernstein I can help take another look at the dependencies.

risdenk · 2022-10-28T14:32:01Z

Added a note to solrj refguide for solrj-streaming.

git grep -F hsql | grep -Fv -e 'solr/solrj-streaming'
NOTICE.txt:HSQL Database (HSQLDB): http://hsqldb.org/
NOTICE.txt:License: http://hsqldb.org/web/hsqlLicense.html
NOTICE.txt:For work added by the HSQL Development Group (a.k.a. hsqldb_lic.txt):
solr/solrj/build.gradle:  testRuntimeOnly "org.hsqldb:hsqldb" // runtime because via JDBC reflection
versions.lock:org.hsqldb:hsqldb:2.4.0 (1 constraints: 08050136)
versions.props:org.hsqldb:hsqldb=2.4.0

hsqldb does not look to be used in solr/solrj anymore so will remove and run tests.

need to check on sql module still.

risdenk · 2022-10-28T14:40:55Z

removed hsqldb and sql module from solrj core. both solrj core and solrj streaming tests pass. Rerunning the whole test suite now.

risdenk · 2022-10-28T14:43:25Z

@dsmiley / @janhoy I think I addressed your concerns so far. The only one not addressed (yet) is only for branch_9x to add upgrade notes and opting out of solrj-streaming.

HoustonPutman · 2022-10-28T15:16:33Z

Looked through the gradle stuff and it looks good to me.

risdenk · 2022-10-28T15:21:30Z

./gradlew check -Pvalidation.errorprone=true passed with changes pushed so far.

joel-bernstein · 2022-10-28T18:07:45Z

The merge button has appeared for me. Anything more to do or should we merge?

risdenk · 2022-10-30T19:06:55Z

I think this is good to go. Just need to do follow up on branch_9x after this is merged and backported to branch_9x - #1099 (comment)

* SOLR-15733: Initial commit WIP * SOLR-15733: Move solrj.client.io * SOLR-15733: Fix directories * SOLR-15733: Move tests * SOLR-15733: Remove commons math from main solrj * SOLR-15733: Wire-in solrj-streaming * SOLR-15733: Get solrj-streaming test classes to compile * SOLR-15733: Get solrj-streaming tests passing * Fix dependency declarations * Fix ref guide to DaemonStream reference * Add solrj refguide note * Remove hsqldb test dependency from solr/solrj * Cleanup solrj sql reference Co-authored-by: Kevin Risden <krisden@apache.org>

risdenk · 2022-11-04T16:44:23Z

backport to branch_9x here: #1161

joel-bernstein added 2 commits October 21, 2022 11:17

SOLR-15733: Initial commit WIP

7b3a598

SOLR-15733: Move solrj.client.io

5909554

joel-bernstein marked this pull request as draft October 21, 2022 15:41

joel-bernstein added 4 commits October 21, 2022 11:50

SOLR-15733: Fix directories

048a3ea

SOLR-15733: Move tests

4a9562c

SOLR-15733: Remove commons math from main solrj

6387519

SOLR-15733: Wire-in solrj-streaming

2b1b25e

SOLR-15733: Get solrj-streaming test classes to compile

bc211e1

risdenk reviewed Oct 21, 2022

View reviewed changes

solr/solrj/build.gradle Show resolved Hide resolved

solr/solrj-streaming/build.gradle Outdated Show resolved Hide resolved

solr/solrj-streaming/build.gradle Outdated Show resolved Hide resolved

SOLR-15733: Get solrj-streaming tests passing

7942722

risdenk reviewed Oct 21, 2022

View reviewed changes

solr/solrj-streaming/build.gradle Outdated Show resolved Hide resolved

Merge branch 'main' into SOLR-15733-streaming

fc094c3

risdenk added 2 commits October 26, 2022 13:34

Fix dependency declarations

95852c4

Fix ref guide to DaemonStream reference

ab28bba

risdenk reviewed Oct 26, 2022

View reviewed changes

joel-bernstein marked this pull request as ready for review October 27, 2022 20:17

risdenk changed the title ~~SOLR-15733: Migrate streaming~~ SOLR-15733: Separate out a solrj-streaming module Oct 27, 2022

risdenk approved these changes Oct 27, 2022

View reviewed changes

risdenk requested review from dsmiley, HoustonPutman, gerlowskija and janhoy October 28, 2022 02:33

risdenk assigned joel-bernstein Oct 28, 2022

dsmiley reviewed Oct 28, 2022

View reviewed changes

janhoy reviewed Oct 28, 2022

View reviewed changes

risdenk added 2 commits October 28, 2022 10:03

Merge branch 'main' into SOLR-15733-streaming

8ee2f15

Add solrj refguide note

73f3138

risdenk added 2 commits October 28, 2022 10:32

Remove hsqldb test dependency from solr/solrj

fc0c0a4

Cleanup solrj sql reference

e3c645e

risdenk requested review from dsmiley and janhoy October 28, 2022 14:41

janhoy approved these changes Oct 30, 2022

View reviewed changes

joel-bernstein merged commit db28836 into apache:main Nov 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SOLR-15733: Separate out a solrj-streaming module #1099

SOLR-15733: Separate out a solrj-streaming module #1099

joel-bernstein commented Oct 21, 2022 •

edited by risdenk

joel-bernstein commented Oct 21, 2022

joel-bernstein commented Oct 21, 2022 •

edited

joel-bernstein commented Oct 21, 2022 •

edited

joel-bernstein commented Oct 21, 2022

risdenk commented Oct 26, 2022

risdenk commented Oct 26, 2022

risdenk commented Oct 26, 2022 •

edited

risdenk left a comment

risdenk commented Oct 26, 2022

joel-bernstein commented Oct 27, 2022

risdenk commented Oct 27, 2022

joel-bernstein commented Oct 27, 2022

joel-bernstein commented Oct 27, 2022

joel-bernstein commented Oct 27, 2022

risdenk commented Oct 28, 2022

dsmiley left a comment

janhoy left a comment

joel-bernstein commented Oct 28, 2022

joel-bernstein commented Oct 28, 2022

dsmiley commented Oct 28, 2022

risdenk commented Oct 28, 2022

risdenk commented Oct 28, 2022

risdenk commented Oct 28, 2022

risdenk commented Oct 28, 2022

HoustonPutman commented Oct 28, 2022

risdenk commented Oct 28, 2022

joel-bernstein commented Oct 28, 2022

risdenk commented Oct 30, 2022 •

edited

risdenk commented Nov 4, 2022

SOLR-15733: Separate out a solrj-streaming module #1099

SOLR-15733: Separate out a solrj-streaming module #1099

Conversation

joel-bernstein commented Oct 21, 2022 • edited by risdenk

joel-bernstein commented Oct 21, 2022

joel-bernstein commented Oct 21, 2022 • edited

joel-bernstein commented Oct 21, 2022 • edited

joel-bernstein commented Oct 21, 2022

risdenk commented Oct 26, 2022

risdenk commented Oct 26, 2022

risdenk commented Oct 26, 2022 • edited

risdenk left a comment

Choose a reason for hiding this comment

risdenk commented Oct 26, 2022

joel-bernstein commented Oct 27, 2022

risdenk commented Oct 27, 2022

joel-bernstein commented Oct 27, 2022

joel-bernstein commented Oct 27, 2022

joel-bernstein commented Oct 27, 2022

risdenk commented Oct 28, 2022

dsmiley left a comment

Choose a reason for hiding this comment

janhoy left a comment

Choose a reason for hiding this comment

joel-bernstein commented Oct 28, 2022

joel-bernstein commented Oct 28, 2022

dsmiley commented Oct 28, 2022

risdenk commented Oct 28, 2022

risdenk commented Oct 28, 2022

risdenk commented Oct 28, 2022

risdenk commented Oct 28, 2022

HoustonPutman commented Oct 28, 2022

risdenk commented Oct 28, 2022

joel-bernstein commented Oct 28, 2022

risdenk commented Oct 30, 2022 • edited

risdenk commented Nov 4, 2022

joel-bernstein commented Oct 21, 2022 •

edited by risdenk

joel-bernstein commented Oct 21, 2022 •

edited

joel-bernstein commented Oct 21, 2022 •

edited

risdenk commented Oct 26, 2022 •

edited

risdenk commented Oct 30, 2022 •

edited