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

roachtest: use updated update_tenant_resource_limits signature in 24.2.1+ #137137

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

DarrylWong
Copy link
Contributor

@DarrylWong DarrylWong commented Dec 10, 2024

The function signature of update_tenant_resource_limits was changed in v24.2.1 to deprecate the args "as_of" and "as_of_consumed_tokens". Since serverless currently only uses the tenant_id overload and not the tenant_name overload like we do in mixedversion, the args were removed completely from the latter.

This breaks backwards compatibility so this change selectively omits the removed args if the version is 24.2.1+.

Fixes: none
Epic: none
Release note: none

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@DarrylWong DarrylWong force-pushed the fix-tenant-limit-sql branch 3 times, most recently from 55fcf6b to e8064f2 Compare December 10, 2024 22:07
@DarrylWong
Copy link
Contributor Author

DarrylWong commented Dec 10, 2024

Tested by running tpcc/mixed-headroom locally bootstrapped on v24.2.0 and v24.2.1 and confirming the SQL statement works both ways. Trying to get the plan to generate that + have it be on separate-process is a pain though, I wish the framework let you force certain params regardless of the seed. Threw together something quick with env vars but I'll keep it as a future PR.

@DarrylWong DarrylWong changed the title roachtest: use updated update_tenant_resource_limits signature in 24.3+ roachtest: use updated update_tenant_resource_limits signature in 24.2.1+ Dec 10, 2024
…2.1+

The function signature of update_tenant_resource_limits was changed
in v24.2.1 to deprecate the args "as_of" and "as_of_consumed_tokens".
Since serverless currently only uses the `tenant_id` overload and
not the `tenant_name` overload like we do in mixedversion, the args
were removed completely from the latter.

This breaks backwards compatibility so this change selectively omits
the removed args if the version is 24.2.1+.
@DarrylWong DarrylWong marked this pull request as ready for review December 10, 2024 22:23
@DarrylWong DarrylWong requested a review from a team as a code owner December 10, 2024 22:23
@DarrylWong DarrylWong requested review from golgeek and csgourav and removed request for a team December 10, 2024 22:23
@DarrylWong
Copy link
Contributor Author

roachtest_test pkg timed out but is unrelated to this change.

TFTR!

bors r=srosenberg

@craig craig bot merged commit 57273c7 into cockroachdb:master Dec 10, 2024
21 of 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.

3 participants