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.

Sign up for GitHub

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

Jump to bottom

Bazelisk 1.24 downloads over and over again #640

Closed
visglz opened this issue Nov 25, 2024 · 2 comments · Fixed by #641
Closed

Bazelisk 1.24 downloads over and over again #640

visglz opened this issue Nov 25, 2024 · 2 comments · Fixed by #641
Assignees
@fweikert
Labels
P1 I'll work on this now. (Assignee required) type:bug Something isn't working

Comments

@visglz
Copy link

visglz commented Nov 25, 2024

Hi,

I just updated to bazelisk 1.24. When no specific version is defined it downloads the latest version over and over again with every invocation.

It can be reproduced the following way under Linux (note, that there is no USE_BAZEL_VERSION in ~/.bazeliskrc:

# Downloads 7.4.1 on every invocation even if present in `/.cache/bazelisk`
./bazelisk-1.24 --version
./bazelisk-1.24 --version
# No download happens
./bazelisk-1.23 --version
USE_BAZEL_VERSION=7.x ./bazelisk-1.24 --version

There is a significant delay when no version is provided with 1.24. When running it via strace one can see quite some amount of network traffic.

Could it be that #636 introduced this behaviour?
@meteorcloudy
Copy link
Member

I also noticed the same issue, @fweikert

@meteorcloudy meteorcloudy added type:bug Something isn't working P1 I'll work on this now. (Assignee required) labels Nov 25, 2024
@meteorcloudy meteorcloudy mentioned this issue Nov 25, 2024
Merged
@fweikert fweikert self-assigned this Nov 25, 2024
fweikert added a commit to fweikert/bazelisk that referenced this issue Nov 25, 2024
@fweikert
Avoid unnecessary HTTP requests for latest/last_rc
2086490 
The refactoring in bazelbuild#631 introduced a performance regression: With latest and last_rc the code traversed *all* GCS buckets for *every* Bazel track.
This commit restores the previous, correct behavior: Traversal will be stopped as soon as a matching version as been found.

Moreover, this commit adds a test to prevent similar problems in the future.

Fixes bazelbuild#640

Drive-by fix: Replaced \"%s\" with %q.
fweikert added a commit to fweikert/bazelisk that referenced this issue Nov 25, 2024
@fweikert
Avoid unnecessary HTTP requests for latest/last_rc
2349c6d 
The refactoring in bazelbuild#631 introduced a performance regression: With latest and last_rc the code traversed *all* GCS buckets for *every* Bazel track.
This commit restores the previous, correct behavior: Traversal will be stopped as soon as a matching version has been found.

Moreover, this commit adds a test to prevent similar problems in the future.

Fixes bazelbuild#640

Drive-by fix: Replaced \"%s\" with %q.
fweikert added a commit to fweikert/bazelisk that referenced this issue Nov 25, 2024
@fweikert
Avoid unnecessary HTTP requests for latest/last_rc
74993c1 
The refactoring in bazelbuild#631 introduced a severe performance regression: With latest and last_rc the code traversed *all* GCS buckets for *every* Bazel track.
Since we send one HTTP request per bucket, this behavior led to a significant increase in HTTP requests ( >140 instead of 2-3 requests).

This commit restores the previous, correct behavior: Traversal will be stopped as soon as a matching version has been found.

Moreover, this commit adds a test to prevent similar regressions in the future.

Fixes bazelbuild#640

Drive-by fix: Replaced \"%s\" with %q.
fweikert added a commit to fweikert/bazelisk that referenced this issue Nov 25, 2024
@fweikert
Avoid unnecessary HTTP requests for latest/last_rc
58eb135 
The refactoring in bazelbuild#631 introduced a severe performance regression: With latest and last_rc the code traversed *all* GCS buckets for *every* existing Bazel version.
Since we send one HTTP request per bucket, this behavior led to a significant increase in HTTP requests ( >140 instead of 2-3 requests).

This commit restores the previous, correct behavior: Traversal will be stopped as soon as a matching version has been found.

Moreover, this commit adds a test to prevent similar regressions in the future.

Fixes bazelbuild#640

Drive-by fix: Replaced \"%s\" with %q.
@fweikert fweikert mentioned this issue Nov 25, 2024
Merged
@fweikert
Copy link
Member

The culprit is the refactoring in #631. The binary is actually downloaded only once, but the code sends way too many HTTP requests to GCS in order to determine the actual version.

meteorcloudy pushed a commit that referenced this issue Nov 25, 2024
@fweikert
Avoid unnecessary HTTP requests for latest/last_rc (#641)
801dd34 
* Avoid unnecessary HTTP requests for latest/last_rc

The refactoring in #631 introduced a severe performance regression: With latest and last_rc the code traversed *all* GCS buckets for *every* existing Bazel version.
Since we send one HTTP request per bucket, this behavior led to a significant increase in HTTP requests ( >140 instead of 2-3 requests).

This commit restores the previous, correct behavior: Traversal will be stopped as soon as a matching version has been found.

Moreover, this commit adds a test to prevent similar regressions in the future.

Fixes #640

Drive-by fix: Replaced \"%s\" with %q.

* Apply minimal indentation principle
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fweikert fweikert

Labels
P1 I'll work on this now. (Assignee required) type:bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Avoid unnecessary HTTP requests for latest/last_rc fweikert/bazelisk
3 participants
@meteorcloudy @fweikert @visglz