Added a new flag for maximum retention bytes for thanos #6944
Conversation
|
@MichaHoffmann @GiedriusS @yeya24 Can you please take a look? |
cmd/thanos/receive.go
Outdated
| @@ -916,6 +918,8 @@ func (rc *receiveConfig) registerFlag(cmd extkingpin.FlagClause) { | |||
|
|
|||
| cmd.Flag("tsdb.allow-overlapping-blocks", "Allow overlapping blocks, which in turn enables vertical compaction and vertical query merge. Does not do anything, enabled all the time.").Default("false").BoolVar(&rc.tsdbAllowOverlappingBlocks) | |||
|
|
|||
| cmd.Flag("tsdb.max-block-bytes", "Maximum size for local TSDB blocks").Default("0").Int64Var(&rc.tsdbMaxBytes) | |||
There was a problem hiding this comment.
Can we use BytesVar instead? https://github.com/prometheus/prometheus/blob/main/cmd/prometheus/main.go#L327
Also please extend the flag description similar to the one in Prometheus. We probably also need to mention it is per tenant TSDB
Signed-off-by: Kartikay <120778728+kartikaysaxena@users.noreply.github.com>
|
Implemented BytesVar instead of Int64Var and updated the description. Also ran make docs for automatic doc update. |
|
@kartikaysaxena Can you help update the changelog |
Glad to help! |
Sure. |
Signed-off-by: Kartikay <kartikay_2101ce32@iitp.ac.in>
|
@yeya24 please PTAL |
| @@ -916,6 +919,8 @@ func (rc *receiveConfig) registerFlag(cmd extkingpin.FlagClause) { | |||
|
|
|||
| cmd.Flag("tsdb.allow-overlapping-blocks", "Allow overlapping blocks, which in turn enables vertical compaction and vertical query merge. Does not do anything, enabled all the time.").Default("false").BoolVar(&rc.tsdbAllowOverlappingBlocks) | |||
|
|
|||
| cmd.Flag("tsdb.max-retention-bytes", "Maximum number of bytes that can be stored for blocks. A unit is required, supported units: B, KB, MB, GB, TB, PB, EB. Ex: \"512MB\". Based on powers-of-2, so 1KB is 1024B.").Default("0").BytesVar(&rc.tsdbMaxBytes) | |||
There was a problem hiding this comment.
This probably should still mention that this is per tenant right?
There was a problem hiding this comment.
Yeah we can mention it explicitly for clarity. @kartikaysaxena
All the flags with tsdb. prefix are per-tenant though.
There was a problem hiding this comment.
Thats true! Lets issue a followup with a small note in the docs that this limit ( and all tsdb flags ) are per tenant. Just for clarity, but that should not block this PR then.
There was a problem hiding this comment.
Alright, I'll create an issue regarding this and raise a PR to fix this :)
|
Just wondered because of the description: Is this really the total amount of storage that all blocks together (per tenant) may occupy? Description could also be interpreted as if that were the maximum storage per block. |
|
I think its a limit for the space of a local tsdb ( per tenant ) |
|
btw: It doesn't matter that much for me personally, since I anyway have only one tenant... but the limit would IMO make more sense if it were a total limit over all tenants. |
Yes it is a bit unfortunate but a good start nonetheless i think. |
This PR is about the issue (#6921 )
Changes
Made a few changes in receive.go and ran
make docsandmake check-docs.Verification
Ran
receiveof thanos