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

kvserver: disable the spanconfig limiter for in-memory tenants [CRDB-14537 followup] #93562

Closed
andreimatei opened this issue Dec 14, 2022 · 0 comments · Fixed by #99918
Closed

kvserver: disable the spanconfig limiter for in-memory tenants [CRDB-14537 followup] #93562

andreimatei opened this issue Dec 14, 2022 · 0 comments · Fixed by #99918
Assignees
@knz
Labels
A-multitenancy Related to multi-tenancy branch-master Failures and bugs on the master branch. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. GA-blocker T-multitenant Issues owned by the multi-tenant virtual team

Comments

@andreimatei
Copy link
Contributor

andreimatei commented Dec 14, 2022 •
edited by cockroach-jira-scripts
Loading

In-memory tenants come with a default limit of 5000 "spans" (spanconfig.tenant_limit cluster), which I think means that they can only create 5000 tables/partition in a GC interval of 25h. This would be a big regression for a UA cluster.

The small limit is inconvenient for tests and benchmarks that run into it. For example, pkg/bench does:

ALTER TENANT ALL SET CLUSTER SETTING "spanconfig.tenant_limit" = 10000000

There's been some slack discussion about this limit. It seems a bit unclear whether it serves any purpose.

Jira issue: CRDB-22403
@andreimatei andreimatei added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-multitenancy Related to multi-tenancy T-multitenant Issues owned by the multi-tenant virtual team labels Dec 14, 2022
@ajstorm ajstorm added branch-master Failures and bugs on the master branch. GA-blocker labels Mar 7, 2023
@stevendanna stevendanna mentioned this issue Mar 29, 2023
Merged
craig bot pushed a commit that referenced this issue Mar 30, 2023
@cucaroach @stevendanna
Merge #99847 #99918
56bb002 
99847: copy: enable copy_from_retries_enabled by default r=cucaroach a=cucaroach

This turned out to be necessary to get customers over the line with DMS
in 22.2.  Now that its had time to prove itself enable it by default.

Epic: none
Informs: #99327
Fixes: #99464


99918: server: ignore spanconfig limit for shared-process tenants r=knz a=stevendanna

This change installs a noop spanconfig limiter when starting shared-process tenants.  Since shared-process tenants are currently used for "unified architecture" in which the tenant is expected to behave as close as possible to the system tenant.

When we have a tenant-side capabilities reader, we should tie this to a capability instead.

This has an unfortunate side-effect of making any tenant-level override for the span limit a lie for shared-process tenants.

Fixes #93562. 
Fixes #93561.

Release note: None

Co-authored-by: Tommy Reilly <[email protected]>
Co-authored-by: Steven Danna <[email protected]>
@craig craig bot closed this as completed in 0a431d9 Mar 30, 2023
@blathers-crl blathers-crl bot mentioned this issue Mar 30, 2023
Merged
blathers-crl bot pushed a commit that referenced this issue Mar 30, 2023
@stevendanna
server: ignore spanconfig limit for shared-process tenants
c9090d7 
This change installs a noop spanconfig limiter when starting
shared-process tenants.  Since shared-process tenants are currently
used for "unified architecture" in which the tenant is expected to
behave as close as possible to the system tenant.

When we have a tenant-side capabilities reader, we should tie this to
a capability instead.

This has an unfortunate side-effect of making any tenant-level
override for the span limit a lie for shared-process tenants.

Fixes #93562

Release note: None
stevendanna added a commit that referenced this issue Apr 13, 2023
@stevendanna
server: ignore spanconfig limit for shared-process tenants
dbc329c 
This change installs a noop spanconfig limiter when starting
shared-process tenants.  Since shared-process tenants are currently
used for "unified architecture" in which the tenant is expected to
behave as close as possible to the system tenant.

When we have a tenant-side capabilities reader, we should tie this to
a capability instead.

This has an unfortunate side-effect of making any tenant-level
override for the span limit a lie for shared-process tenants.

Fixes #93562

Release note: None
@blathers-crl blathers-crl bot mentioned this issue Apr 13, 2023
Merged
blathers-crl bot pushed a commit that referenced this issue Apr 13, 2023
@stevendanna
server: ignore spanconfig limit for shared-process tenants
1be8a42 
This change installs a noop spanconfig limiter when starting
shared-process tenants.  Since shared-process tenants are currently
used for "unified architecture" in which the tenant is expected to
behave as close as possible to the system tenant.

When we have a tenant-side capabilities reader, we should tie this to
a capability instead.

This has an unfortunate side-effect of making any tenant-level
override for the span limit a lie for shared-process tenants.

Fixes #93562

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@knz knz

Labels
A-multitenancy Related to multi-tenancy branch-master Failures and bugs on the master branch. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. GA-blocker T-multitenant Issues owned by the multi-tenant virtual team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

server: ignore spanconfig limit for shared-process tenants stevendanna/cockroach
3 participants
@andreimatei @knz @ajstorm