Upgrade Elasticsearch client to 8.0.0

[danielmitterdorfer]

Note: This is not actionable at the moment but acts as a reference of things to consider when upgrading the Elasticsearch client to 8.0.0.

Looking at the Elasticsearch client roadmap a few items stand out but specifically elastic/elasticsearch-py#1680 will be tricky: The client will not allow to pass a body but rather requires to specify the body in a more structured manner. Initially using body will only be deprecated but we need to think about alternatives.

[pquentin]

I think it's fine to continue using body as it will be supported in 8.0, and possibly later. If it gets removed at some point, we can always start using **body for the cases where we pass a body around before submitting to the client.

[sethmlarson]

The first pre-release of 8.0.0 is now available on PyPI: https://github.com/elastic/elasticsearch-py/releases/tag/v8.0.0a1

[michaelbaamonde]

@pquentin

As discussed, I've closed the linked PR due to size and am instead going to break it into a sequence of smaller PRs, while trying to limit the potential for breakage as much as possible. This is a little tricky. Here's my thinking:

First, we organize the client-related code to make it more readable and separate the different concerns at play. I've issued a PR that does this by creating a new esrally.client package. This should help keep things clean as we make the bigger changes that we know we need.

Assuming that looks good and we merge it, we could do a few small PRs that add in utility code that we'll eventually need (e.g. the _ProductChecker class, utility functions (example), etc.) These should be transparent and have no effect on runtime behavior.

Now, we'd be at the point where things are more challenging, as we'd need to introduce a dependency on the new client version and cut over to using it. I see two ways that we could do this cutover gradually.

Option 1: Install two client versions side-by-side temporarily

We could phase most of the changes required for the upgrade by installing v8.2.0 (elasticsearch8[async]==8.2.0) as well as elastic-transport==8.1.2 in addition to 7.14.0 (via elasticsearch[async]==7.14.0.) We would import 7.14.0 as elasticsearch and use it at runtime as we do today, but have elasticsearch8 available for import, as well.

This would allow us to merge the core additions we need for the upgrade (e.g. our own subclasses of elasticsearch8.Elasticsearch, elastic_transport.Transport, etc.) to master in a piecemeal fashion. There would temporarily be separate code paths for the 7.14.0 client and the 8.2.0 client, meaning duplication and effectively unused code on master, but we would remove the 7.14.0-related code with the final PR that finishes the cutover to the new client version.

• In this scenario, all commits are purely additive except for the final one that removes the 7.14.0 client dependency and related code.

• The big downside is that master would temporarily contain code that's not actually used at runtime.

Note: We could go as far as to do this behind a feature flag. Could be overkill, but could also make testing easier.

Option 2: Feature branch

Another option could be to have a feature branch in Rally for the client upgrade and issue PRs against that branch gradually before doing one large merge to master once we're happy with the state of the feature branch.

• In this scenario, we'd have more of a mix of green and red diffs during review and commits would probably have to be larger, but master would remain clean until the final merge.

• The big downside is the inevitability of merge conflicts with master.

In either case, we'd have more atomic changes that should be easier to review and present less risk overall. What do you think?

[pquentin]

Hmm, those options are interesting, thanks. My preference here is to merge code to master that gets used immediately, like the rally and rally-tracks PRs we merged yesterday, they were validated in our nightly benchmarks.

And yes, this means at some point we'll have a bigger PR that makes the switch, but that's OK. (And I'm now more familiar with the problem so reviewing will be easier.)

So basically I would suggest the following PRs:

• Switch to 7.17 and run CI with compatibility mode per https://www.elastic.co/guide/en/elasticsearch/client/python-api/current/migration.html#migration-compat-mode (this will require a few performance tests)
• Add the plumbing to pass distribution_version to the relevant classes
• (not a PR) Agree on the strategy regarding version checks (we may not get away with doing a post-flight check due to concurrency) and exception handling
• Move to 8.2 (it could be multiple commits but only one PR I think)

[pquentin]

By the way thinking about this more I don't think we should have the 7.17 step, let's not increase the scope too much. Regarding the version check, since we already disable it in the async client that is used for the actual benchmarking measures, I think we should just check once when Rally starts and then disable it everywhere.