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

Support for Elasticsearch 8.4.1 #406

Closed
wants to merge 19 commits into from
Closed

Conversation

nfsantos
Copy link

Related Issue

Changes

  • Upgraded versions of Elasticsearch and Lucene libraries.
  • Fix several deprecations and API changes introduced in newer versions of ES.

Testing and Validation

The JVM tests passed. (task jvm:test)

@alexklibisz
Copy link
Owner

Thanks! I don't think Github will run the builds until the merge conflicts are resolved. Once the builds run, you can use a snapshot release. You can also just publish the zip file locally (./gradlew assembly IIRC, but look in the jvm taskfile) and upload it somewhere that you can access from your systems.

I'd rather not merge this yet. Maybe I'm being silly but it seems best to keep a continuation of versions rather than leaving a large gap. Maybe the cross building (#391) is also unnecessary, but it has come up several times (and it's a fun exercise, too). Open to opinions/input.

@alexklibisz
Copy link
Owner

cc @fabriziofortino

@nfsantos
Copy link
Author

I have resolved the merge conflicts, but since I'm a first time collaborator, GitHub is not running the test workflow. Could you approve the workflow, just so we have a better idea if everything is fine? I'm not sure if I broke something. I'm running the tests locally on my machine, but would be nice to have more testing.

We can use our fork with the upgrade to 8.4.1 directly, we are not blocked even if this PR is not merged to master. We just wanted to contribute back, as these fix may be useful for other users of the project.

@alexklibisz
Copy link
Owner

We just wanted to contribute back, as these fix may be useful for other users of the project.

Beautiful! Thank you!

I approved the workflows. The tests are running. If tests pass, there should be a snapshot release similar to this one: https://github.com/alexklibisz/elastiknn/releases/tag/8.0.0.0-PR347-SNAPSHOT

@alexklibisz
Copy link
Owner

It looks like something is preventing the cluster from starting correctly in the test suite:

task: [cluster:run] python3 cluster_ready.py
Traceback (most recent call last):
  File "/usr/lib/python3.8/urllib/request.py", line 1354, in do_open
    h.request(req.get_method(), req.selector, req.data, headers,
  File "/usr/lib/python3.8/http/client.py", line 1256, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/lib/python3.8/http/client.py", line 1302, in _send_request
    self.endheaders(body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.8/http/client.py", line 1251, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.8/http/client.py", line 1011, in _send_output
    self.send(msg)
  File "/usr/lib/python3.8/http/client.py", line 951, in send
    self.connect()
  File "/usr/lib/python3.8/http/client.py", line 922, in connect
    self.sock = self._create_connection(
  File "/usr/lib/python3.8/socket.py", line 808, in create_connection
    raise err
  File "/usr/lib/python3.8/socket.py", line 796, in create_connection
    sock.connect(sa)
ConnectionRefusedError: [Errno 111] Connection refused

I think this exception case needs to be added: https://github.com/alexklibisz/elastiknn/pull/347/files#diff-cc47ca20c973158dd5d8ea0a2fa706db4627c03d40afdf0440b5ff9ec6e2d8c1R12

@nfsantos
Copy link
Author

@alexklibisz I have merged the latest 8.0.0 branch in this branch, to include the merge you have made with the master, like this the two branches are closer. I have also made some additional fixes to the test scripts so i could run the tests on my local machine (a Mac with Apple silicon). See commit 18d3fb2
The Elastiknn tests passed. Our internal tests are also passing.

If the GitHub workflow runs successfully, we could use the resulting snapshot artifact as a temporary solution until these changes are pulled into master.

@alexklibisz
Copy link
Owner

@fabriziofortino @nfsantos I got 8.0.0 and 8.0.1 merged and released yesterday. I'll be off-grid for the weekend but In the next week or so I'd like to continue stepping through each of the releases up to 8.4.1 (one PR per version). Looks like about 15 more PRs according to: https://www.elastic.co/guide/en/elasticsearch/reference/current/es-release-notes.html. If either of you wants to start on that, I'll happily review once I'm back.

@alexklibisz
Copy link
Owner

Resolved by #426 . We should be all caught up now!

@nfsantos nfsantos deleted the es8.4.1 branch September 21, 2022 06:09
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

Successfully merging this pull request may close these issues.

None yet

2 participants