-
Notifications
You must be signed in to change notification settings - Fork 49
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
Cross-build for Elasticsearch 7.x and 8.x #391
Comments
Is there any update on cross-building to 7.17? The latest Elasticsearch release in the 7.17 series is 7.17.7, which is currently not supported by this plugin. The latest plugin release for the 7.17 series is 7.17.6.0. We are a bit blocked until there is a plugin release for 7.17.7 because Elastic Cloud only allows deploying the latest patch release for any given minor version. That is, it is no longer possible to deploy 7.17.6. But we cannot deploy 7.17.7 because there is no release of elastiknn for this version. As a quick fix, do you think we could instead create a branch starting from the 7.17.6.0 and do a quick release for 7.17.7? Any other suggestions? Thanks! |
Yeah, if you can't upgrade to 8.x, I would recommend doing this branch approach:
It's a bit hacky, but will work. I'd like to get back to the cross-building soon, but no immediate plans. It's a tricky feature. Also open to reviewing PRs if someone else wants to try it out. |
I have created a branch in my Github fork with support for ES 7.17.7. I used the 7.17.6 tag as a basis. However, I'm not sure how to create a PR. AFAIK, a PR must be made against a branch in the main repository, it is not possible to make it against a tag. I cannot do it against the main branch and I think there is no other branch that I could use. Maybe it is worth to create a 7_17 branch starting from the 7.17.6 tag? I guess this will not be the last time we need to make a new release for the 7.17 series. Do you have other suggestions? |
I made a branch: https://github.com/alexklibisz/elastiknn/tree/cross-build-7.17.6 Can you try making a PR into that branch? |
Thanks, it worked: #456 |
Hi @alexklibisz and @nfsantos , |
No progress on this. We will have to do a workaround like we used in #456. I can probably do that this weekend. Or feel free to take a pass at it. The build situation can be tricky. This cross-build is still something I would like to work on. I just completed a big move across the country, so life is approaching steady-state again. Out of curiosity, why not upgrade to 8.x? Stability? Priority? Something else? |
Thanks, @alexklibisz for the prompt reply! :) |
Hey all, I've made some progress here: #465. I decided to just start a new branch, elasticsearch-7x, to maintain releases for the latest Elasticsearch 7.x. Keeping it all on the main branch would have been my preference, but it really complicates the build. I'll try to merge the 7x branch and release for 7.17.8 today or tomorrow. |
Hey all, here is the 7.17.8 release: https://github.com/alexklibisz/elastiknn/releases/tag/7.17.8.0 Pleaase open a new ticket if you see any functionality issues w/ that release. Some more context: I decided to create a new branch, elasticsearch-7x, which will be used to maintain releases for Elasticsearch 7.x. I documented this more here: https://github.com/alexklibisz/elastiknn/blob/elasticsearch-7x/developer-guide.md I think this could get quite tricky, but I'll do my best to stay on top of it and use it as an opportunity to practice my git skills :) Also, as releases to 7.x come out, please feel free to make a PR to that branch with the new version. I usually upgrade within a week or so of each Elasticsearch release, but sometimes other things come up and it takes longer. |
Some more context on why I decided against cross-building on a single branch (or maybe for myself so I don't go down this rabbithole again): Each version of Elasticsearch depends on a potentially different version of Lucene. A new major version of ES typically depends on a new major version of Lucene. I thought about abstracting out the "core" functionality of Elastiknn into a package that can compile against both sets of versions. However, this would require a lot of refactoring, especially in the Lucene code. Beyond the complexity of it, this would probably have a non-trivial performance cost, and it would be impossible to guarantee that it would continue working over time, as I have no control over the types of breaking changes that occur across major versions of ES and Lucene. So after trying to do the single-branch approach for a couple hours, I decided it's simpler/saner to just keep two branches, and try to keep them in-sync when it comes to builds and testing. Doing that still took quite a bit of effort, since I had switched from Gradle to SBT shortly after the 8.0.0 release. I had to backport all of those build changes. It ended up being simpler to just take the 8.6.0 release, w/ the most recent build setup, and revert it back to Elasticsearch 7.17.8. That was #465. |
Background
This has been requested a few times in the past. It didn't seem pertinent as Elastiknn started on Elasticsearch 7.x. Now it's a bit of a blocker for upgrading to 8.x. I tried it before with Gradle and couldn't get it working. I want to see if I can get it working in SBT, as I'm much more interested in and familiar with SBT. If I can get this work, maybe publishing for Opensearch is an option in the future.
Deliverables
Related Issues
The text was updated successfully, but these errors were encountered: