Conversation
|
Is there a design document for introducing the concept of subtenants? This seems like a huge departure from how Mimir has operated until this point. |
juliusmh
force-pushed
the
jmh/subtenant_limits
branch
from
1509b31 to
31bb980
Compare
pkg/mimir/runtime_config.go
Outdated
| type runtimeConfigValues struct { | ||
| TenantLimits map[string]*validation.Limits `yaml:"overrides"` | ||
|
|
||
| // SubtenantLimits provides limits for subtenant types. |
There was a problem hiding this comment.
I think terminology is confusing throughout this change. With "subtenant" we sometimes refer to a full "abc:xyz" string, and sometimes just to the "xyz" part. Here we refer to the latter, but also call it "subtenant type".
How about we stick to:
- Tenant ID: what comes before the
:. This makes it backwards-compatible, and consistent with components that don't know anything about subtenant IDs. - Subtenant type: what comes after the
:. - Subtenant ID: tenant ID +
:+ subtenant type
(I think we may also drop "subtenant" as a term altogether, I think it's somewhat misleading. But I don't have a better idea at the moment.)
pkg/mimir/runtime_config.go
Outdated
|
|
||
| // BySubtenantID returns limits specific to a subtenant type, or nil if none are configured. | ||
| // This implements the validation.SubtenantLimitsProvider interface. | ||
| func (l *runtimeConfigSubtenantLimits) BySubtenantID(subtenantID string) *validation.Limits { |
There was a problem hiding this comment.
To remove ambiguities and let subtenant overrides actually be "overrides", I think this should explicitly take tenantID, subtenantType, and return:
limits[tenantID].
overrideWith(limits[subtenantType]).
overrideWith(limits[tenantID:subtenantType])Also, I think this logic would be a better for util/validation.
juliusmh
force-pushed
the
jmh/subtenant_limits
branch
2 times, most recently
from
be8f379 to
41c03a4
Compare
juliusmh
force-pushed
the
jmh/subtenant_limits
branch
from
41c03a4 to
e1e6629
Compare
pkg/util/validation/limits.go
Outdated
| } | ||
| return overrides.MaxGlobalSeriesPerUser | ||
| func (o *Overrides) MaxActiveOrGlobalSeriesPerUser(orgID string) int { | ||
| return getOverride(o, orgID, func(l *Limits) int { |
There was a problem hiding this comment.
To avoid trouble, maybe we should only do this for limits that actually should be overridable for subtenants. Which is only max_active_series at the moment (but I think should include at least ingestion_rate too; anything enforced by the distributor and that could interfere with the main tenant).
juliusmh
force-pushed
the
jmh/subtenant_limits
branch
from
a741bcc to
5f3a73d
Compare
3 participants
What this PR does
This PR introduces the concept of subtenants. a subtenant ID can be specified by passing it after the main tenant ID in the org ID header, e.g. "123123:test".
Subtenants can have specific limits, aka "SubtenantLimits". Mimir, whenever it encounters a request for a specific subtenant, can ignore the main tenant's limits
and use the ones for the subtenant instead.
Based on grafana/dskit#901
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]. If changelog entry is not needed, please add thechangelog-not-neededlabel to the PR.about-versioning.mdupdated with experimental features.