Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SolrJ 9x vs 8x #62

Open
chatman opened this issue Mar 27, 2023 · 9 comments
Open

SolrJ 9x vs 8x #62

chatman opened this issue Mar 27, 2023 · 9 comments

Comments

@chatman
Copy link
Collaborator

chatman commented Mar 27, 2023

As I was looking to merge master into ishan/validations branch, I realized that the master branch no longer works (neither with Solr 8x nor 9x). I see a plethora of errors. All of those started ever since we upgraded SolrJ from 8.11 to 9.1 in PR #57.

I see that all active development is now happening at solr-8.x branch. An 8x version of SolrJ can handle talking to Solr 8x and 9x both, so I think that's the way to go forward to support both.

Should we abandon master branch and make solr-8.x branch the new master? @patsonluk @justinrsweeney @hiteshk25 .
That will allow us to upgrade the open source graphs (mostly.cool/graph.html) to use the latest solr-bench commits (esp. the metrics graphs in a modal window), also allow the merging of the validations branch.

@justinrsweeney
Copy link
Contributor

I don't think we want to just switch solr-8.x to be master. I think we'd want to fix whatever is needed to get master working again and merge that into master. I believe security tooling was what pushed the upgrade to 9.1 for solrj: d893b51. Can solrj-9.1 talk with both 8.x and 9.x clusters?

@chatman
Copy link
Collaborator Author

chatman commented Mar 27, 2023

Can solrj-9.1 talk with both 8.x and 9.x clusters?

No, SolrJ 9.1 can only talk to Solr 9x clusters.

I believe security tooling was what pushed the upgrade to 9.1 for solrj

I think upgrading 8.11.0 to 8.11.2 would've taken care of security concerns.

I think we'd want to fix whatever is needed to get master working again and merge that into master.

I'm looking at it at the moment.

@justinrsweeney
Copy link
Contributor

👍 Cool, I think we should just revert the upgrade to 9.1 in master and instead bump to 8.11.2, but let me know as you look at it if you have other ideas.

@chatman
Copy link
Collaborator Author

chatman commented Mar 27, 2023

+1 Cool, I think we should just revert the upgrade to 9.1 in master and instead bump to 8.11.2, but let me know as you look at it if you have other ideas.

Sure, I'm looking at it. :-)

@patsonluk
Copy link
Contributor

patsonluk commented Mar 27, 2023

Thanks @chatman . Here's a bit more context of why we made a new branch solr8.x
https://fullstory.atlassian.net/browse/SAI-4402?focusedCommentId=224587

Some work was done in #60 to fix those errors after the solrj update. The first attempt is to make solrj 9 both compatible with 8.x and 9.x. Most stuff worked but the blocker (at least one) is the removal of maxShardPerNode in 9 solrj . Therefore the #60 probably only works for 9.x solr servers

@chatman
Copy link
Collaborator Author

chatman commented Mar 27, 2023

Thanks for the additional context, Patson. Any reason not to continue using the SolrJ 8.11.x (in solr-bench) against the Solr 9x clusters, apart from the security concern? I think the security concern can be addressed with just 8.11.0 to 8.11.2 upgrade. Is there any newly introduced API in SolrJ 9x that we'll miss out on, if we stay on SolrJ 8.11.x?

@patsonluk
Copy link
Contributor

patsonluk commented Mar 27, 2023

@chatman I believe it was purely because of the synk update with the proposed PR #57.

When i first ran into the issues (i discovered it pretty much the same as how you did 😓 ), I thought it should be trivial to make it work for both 8.x and 9.x. And little did I know that it took me half day+ 🤣 , and then hit the max shard per node snag when I ran it using our FS setups (which does have more than 1 node per shard). So I discussed with @hiteshk25 of the path forward, hence the 2 branches now.

Though we did NOT consider just upgrading it to 8.11.2 during that conversation, as back at the beginning I thought 9.x would be an easy fix, and it's usually good to upgrade to the latest and greatest 😓.

I think my concerns of using 8.x version is similar as yours:

  1. I'm not aware of any solrj9 features that we NEED right now, but we might start finding things once we do start using solr 9 for real
  2. I'm wondering if synk would flag other things once we start using 8.11.2, even if it does not now, it might find something in the future, and it could be hard to keep up using 8.x client?
  3. How much longer do we want our master branch to remain 8.x compatible? I think eventually we will need to flip the switch? :)

@chatman
Copy link
Collaborator Author

chatman commented Mar 27, 2023

I'm not aware of any solrj9 features that we NEED right now, but we might start finding things once we do start using solr 9 for real

Until now (9.2), I don't think there are features that won't work with an 8.11 client. Coordinator node is a feature that's supported in 9x clients. But, easy to simulate it via HttpSolrClients.

I'm wondering if synk would flag other things once we start using 8.11.2, even if it does not now, it might find something in the future, and it could be hard to keep up using 8.x client?

IIUC, Synk flagged the 8.11.0 release because of the Log4J vulnerability, which is patched in 8.11.2

How much longer do we want our master branch to remain 8.x compatible? I think eventually we will need to flip the switch? :)

I think every release of 9x should be compatible with 8.11, for backcompat reasons. That's on a best effort basis, there are no automated tests, though.

@chatman
Copy link
Collaborator Author

chatman commented Mar 27, 2023

"master" is copied over to "solr_9.x"
"solr-8.x" is effectively brought into "master".
TODO: Work done here: #56 needs to be ported again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants