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

sql: enable tenant testing for zone tests #141735

Merged

Conversation

shubhamdhama
Copy link
Contributor

@shubhamdhama shubhamdhama commented Feb 19, 2025

sql: enable tenant testing for `zone_test.go`

Updating TestValidSetShowZones to use
createTestServerParamsAllowTenants instead of createTestServerParams
required some investigation. It turned out simpler than expected. Zone
settings for meta ranges aren't supported for secondary tenants, so this
change simply exclude them from setup and validating.

Informs: #140446
Epic: CRDB-48357
Release note: None

sql: enable tenant testing for `zone_config_test`

Since GetSpanConfigForKey is only available in system tenant, it is not
used when running under secondary tenants.

@shubhamdhama shubhamdhama requested review from a team as code owners February 19, 2025 16:18
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@shubhamdhama shubhamdhama changed the title sql: enable tenant testing for tests in zone_config_test sql: enable tenant testing for tests in zone tests Feb 20, 2025
@shubhamdhama shubhamdhama changed the title sql: enable tenant testing for tests in zone tests sql: enable tenant testing for zone tests Feb 20, 2025
Updating `TestValidSetShowZones` to use
`createTestServerParamsAllowTenants` instead of `createTestServerParams`
required some investigation. It turned out simpler than expected. Zone
settings for meta ranges aren't supported for secondary tenants, so this
change simply exclude them from setup and validating.

Informs: cockroachdb#140446
Epic: CRDB-38970
Release note: None
Since `GetSpanConfigForKey` is only available in system tenant, it is not
used when running under secondary tenants.
@shubhamdhama shubhamdhama force-pushed the sql-tenant-testing-zone-config-tests branch from c922210 to 344c262 Compare March 12, 2025 11:20
@shubhamdhama shubhamdhama requested review from rafiss and removed request for a team March 12, 2025 11:23
Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

lgtm! thank you

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@shubhamdhama
Copy link
Contributor Author

TFTR!

bors r=rafiss

craig bot pushed a commit that referenced this pull request Mar 13, 2025
141735: sql: enable tenant testing for zone tests r=rafiss a=shubhamdhama

`` sql: enable tenant testing for `zone_test.go` ``

Updating `TestValidSetShowZones` to use
`createTestServerParamsAllowTenants` instead of `createTestServerParams`
required some investigation. It turned out simpler than expected. Zone
settings for meta ranges aren't supported for secondary tenants, so this
change simply exclude them from setup and validating.

Informs: #140446
Epic: CRDB-48357
Release note: None

`` sql: enable tenant testing for `zone_config_test` ``

Since `GetSpanConfigForKey` is only available in system tenant, it is not
used when running under secondary tenants.

142186: kvserver: mark replica as unavailable if leaderless for a long time r=iskettaneh a=iskettaneh

This PR marks the replica as unavailable in if it has been leaderless for a duration above:
`kv.replica_raft.leaderless_unavailable_threshold`. This helps requests to bail early on unavailable ranges without relying on the replica circuit breaker to trip. This has multiple benefits:

1) Faster reaction time than the replica circuit breaker: If two nodes fail (assuming R=3), many ranges will become unavailable. With the replica circuit breaker, a scan query will need to trip the circuit
breaker for multiple replicas, causing added delays. However, with this approach, the replica will basically become unavailable on the tick path (rather than on the request path). Meaning that a scan query wouldn't need to trip one circuit breaker after the other.

2) Lighter weight: Instead of relying on the replica circuit breaker to test the replication pipeline before it marks the range as
available again, this approach relies on the Raft signal to know when there is a leader again, indicating that the range is
available again.

3) With leader leases, a replica won't propose a lease if it's not the leader. This means that with leader leases, the replica circuit breaker might not trip if the range have lost quorum. However, with this commit, the replica will eventually forget who the leader was, and eventually the leaderlessWatcher would mark it as unavailable.

Fixes: #139638

Release note: None

142489: row, sql: implement mutations on vector indexes r=mw5h a=mw5h

This patch plumbs the output of the vector search operators into rowenc for encoding into vector indexes. The output of these vector search operators is included in the row values for mutation operators after partial index values and are plumbed into a new vector index update helper, which tracks the column values until they're needed by rowenc.

While we're here, we homogonize how
pkg/sql/{delete,insert,update,upsert}.go consume row values, hopefully improving legibility.

Epic: CRDB-42943
Release note: None

142829: sql/catalog: fix object renames for PCR reader catalogs r=fqazi a=fqazi

Previously, the PCR reader catalog would only delete namespace entries for a descriptor if it was not modified. This meant the reader catalog logic could leave behind stale entries in the system.namespace table after a object was renamed. To address this, this patch detects if an object is renamed, and allows the old namespace entry to be deleted.

Fixes: #142828

Release note (bug fix): PCR reader catalogs could have orphan rows in system.namespace after a object is renamed.

142836: kvserver: deflake TestLeasePreferencesDuringOutage r=kvoli a=arulajmani

There was a race here, when heartbeating node liveness epochs, where another node could increment our epoch. This patch retries in such situations.

Fixes #142795

Release note: None

Co-authored-by: Shubham Dhama <[email protected]>
Co-authored-by: Ibrahim Kettaneh <[email protected]>
Co-authored-by: Matt White <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Arul Ajmani <[email protected]>
@craig
Copy link
Contributor

craig bot commented Mar 13, 2025

Build failed (retrying...):

@craig craig bot merged commit f0a4f11 into cockroachdb:master Mar 13, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants