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

Fix "Could not checkout. Do you have uncommitted changes?" error #1772

Merged
merged 3 commits into from
Aug 29, 2023

Conversation

b-deam
Copy link
Member

@b-deam b-deam commented Aug 29, 2023

In #1763 we added a new Bootstrap message handler to each Rally worker, which is responsible for ensuring the track and configuration is properly loaded on each worker before we attempt to execute the benchmark. Part of this work uncovered that there was a race condition if multiple workers tried to checkout the same underlying git repository in parallel, resulting in this error message:

2023-08-29 01:38:04,182 ActorAddr-(T|:58548)/PID:87195 esrally.utils.process INFO fatal: Unable to create '/Users/bradleydeam/perf/github.com/b-deam/rally-tracks/.git/index.lock': File exists.

Another git process seems to be running in this repository, e.g.
an editor opened by 'git commit'. Please make sure all processes
are terminated then try again. If it still fails, a git process
may have crashed in this repository earlier:
remove the file manually to continue.

To avoid this, we added a check as part of the repository loading process that skips the checkout if we're already on the correct revision/branch

# skip checkout if already on correct version. this is helpful in case of multiple actors loading simultaneously.
if not self.repo.correct_revision(repo_revision):
self.repo.checkout(repo_revision)

However, recent PRs to https://github.com/elastic/rally-tracks have been failing due to this race condition even with the above check in place. When we look at the CI logs for an example of the failure, we see that it's because the revision passed in contains whitespace at the end, meaning the correction_revision() function returns false, note the whitespace in the latter revision:

2023-08-24 11:21:15,408 ActorAddr-(T|:36487)/PID:3957 esrally.utils.repo INFO Checking current revision [96248077b03ae242a973b175244ac93eec0e0f9a] is equal to specified revision [96248077b03ae242a973b175244ac93eec0e0f9a
].

CI highlighted this bug, but it would be very confusing and difficult for any end users to debug.

This commit adds a function that strips whitespace from any subcommand
that takes a revision argument.

@b-deam b-deam added the bug Something's wrong label Aug 29, 2023
@b-deam b-deam requested a review from pquentin August 29, 2023 01:48
@b-deam b-deam self-assigned this Aug 29, 2023
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This looks good to me. Very nice catch.

esrally/rally.py Outdated Show resolved Hide resolved
@b-deam b-deam merged commit e713173 into elastic:master Aug 29, 2023
@pquentin pquentin added this to the 2.9.1 milestone Sep 14, 2023
@pquentin pquentin changed the title Strip whitespace from 'revision' args Fix "Could not checkout. Do you have uncommitted changes?" error Sep 14, 2023
@pquentin pquentin mentioned this pull request Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something's wrong
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants