feat: add limits package #16489
feat: add limits package #16489
Conversation
grobinson-grafana
force-pushed
the
grobinson/add-limits-pkg
branch
6 times, most recently
from
5d08a8c to
56df031
Compare
This commit adds a new limits package for enforcing per-tenant limits.
grobinson-grafana
force-pushed
the
grobinson/add-limits-pkg
branch
from
56df031 to
bd84903
Compare
|
|
||
| // EncodeStreamMetadata encodes the stream metadata into a Kafka record | ||
| // using the tenantID as the key and partition as the target partition | ||
| func EncodeStreamMetadata(partition int32, topic string, tenantID string, streamHash uint64) *kgo.Record { |
There was a problem hiding this comment.
@periklis why don't we change it to (*kgo.Record, error)? Then we can return an error on line 215?
There was a problem hiding this comment.
Agree on that too, we should now hide the error
| // DecodeStreamMetadata decodes a Kafka record into a StreamMetadata. | ||
| // It returns the decoded metadata and any error encountered. | ||
| func DecodeStreamMetadata(record *kgo.Record) (*logproto.StreamMetadata, error) { | ||
| if record == nil { |
There was a problem hiding this comment.
@periklis Do we need these two checks? It should be expected that the caller passes a non-nil record that also has a non-nil record.Value?
There was a problem hiding this comment.
Agree with that we can drop them now.
|
|
||
| var ( | ||
| tenantPartitionDesc = prometheus.NewDesc( | ||
| constants.Loki+"_ingest_limits_partitions", |
There was a problem hiding this comment.
I believe this is no longer recommended, almost all of our new code adds loki as a string literal.
| metadata map[string]map[int32][]streamMetadata // tenant -> partitionID -> streamMetadata | ||
|
|
||
| // Track partition assignments | ||
| mtxAssingedPartitions sync.RWMutex |
There was a problem hiding this comment.
Need to remove this, we will just use mtx.
There was a problem hiding this comment.
I believe you have already removed that right?
There was a problem hiding this comment.
Yeah but it regressed somehow, need to remove it again.
What this PR does / why we need it:
This commit adds a new limits package for enforcing per-tenant limits. It is the first of a number of pull requests to move code from
feat/usage-trackerintomain.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Checklist
CONTRIBUTING.mdguide (required)featPRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.mddeprecated-config.yamlanddeleted-config.yamlfiles respectively in thetools/deprecated-config-checkerdirectory. Example PR