Skip to content

roachprod: reduce start-up lag #137916

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

Merged
merged 3 commits into from
Jan 14, 2025
Merged

roachprod: reduce start-up lag #137916

merged 3 commits into from
Jan 14, 2025

Conversation

tbg
Copy link
Member

@tbg tbg commented Dec 23, 2024

On my machine (which is in Europe), this brings time roachprod --help from 1.56s down to to 0.06s under the following env vars:

ROACHPROD_DISABLE_UPDATE_CHECK=true
ROACHPROD_DISABLED_PROVIDERS=azure
ROACHPROD_SKIP_AWSCLI_CHECK=true

Under these env vars, my roachprod

  • no longer invokes aws --version on each start (python, ~400ms)
  • no longer inits azure, which is >1s for me
  • doesn't list the gs bucket to check for a newer roachprod binary (~800ms; doesn't exist for OSX anyway).

A better way (but one outside of my purview) for most of these would be to add caching for each of these and so to avoid the cost in the common case.

Azure is an exception, as the (wall-clock) profile below shows we're spending most of our time waiting for GetTokenFromCLIWithParams to return. It's not clear how to optimize this. (The AWS portion of the flamegraph is aws --version).

image

Epic: none

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg tbg force-pushed the rp-update-check branch 2 times, most recently from b2d78c1 to 9cc858a Compare December 23, 2024 14:23
@tbg tbg marked this pull request as ready for review December 23, 2024 14:24
@tbg tbg requested review from a team as code owners December 23, 2024 14:24
@tbg tbg requested review from herkolategan, vidit-bhat, golgeek and a team and removed request for a team December 23, 2024 14:24
Copy link
Collaborator

@rickystewart rickystewart left a comment

Choose a reason for hiding this comment

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

.

@tbg tbg force-pushed the rp-update-check branch from 9cc858a to 22c053e Compare January 6, 2025 11:53
@tbg tbg requested a review from rickystewart January 6, 2025 11:54
Copy link
Collaborator

@herkolategan herkolategan left a comment

Choose a reason for hiding this comment

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

Nice, we're under the Doherty threshold again with this!

@herkolategan
Copy link
Collaborator

StaticCheck is unhappy with:

=== RUN   TestLint/TestStaticCheck
    gen-lint_test.go:2115: 
        pkg/cmd/roachprod/cli/commands.go:1940:2: should use 'return goos == "linux"' instead of 'if goos == "linux" { return true }; return false' (S1008)
    --- FAIL: TestLint/TestStaticCheck (1405.22s)

tbg added 3 commits January 14, 2025 14:39
The update check takes >800ms where I am. `roachprod` is slow enough as
is, so allow folks to opt out and also opt out automatically in cases
where `roachprod update` doesn't work in the first place (osx).

Epic: none
Release note: None
Azure in particular takes a long time - sometimes well north of 1s.
@tbg tbg force-pushed the rp-update-check branch from 22c053e to 7f30eeb Compare January 14, 2025 13:39
@tbg
Copy link
Member Author

tbg commented Jan 14, 2025

TFTR!

Fixed the lint.

@herkolategan if your team wants to socialize anything about these env vars, please go ahead... after all, so far the only person they're going to benefit is yours truly.

bors r+

@craig
Copy link
Contributor

craig bot commented Jan 14, 2025

👎 Rejected by code reviews

@tbg
Copy link
Member Author

tbg commented Jan 14, 2025

bors r+

@craig craig bot merged commit 2857e2b into cockroachdb:master Jan 14, 2025
22 checks passed
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.

5 participants