Enable setting default chunk encoding by config file (#2077) #4086
Conversation
ChinYing-Li
force-pushed
the
issue_2077
branch
4 times, most recently
from
1434b99 to
8a64704
Compare
|
Can you please rebase against master to pull in #4137 and move the changelog entry to the top? |
pkg/chunk/encoding/factory.go
Outdated
| New: func() Chunk { | ||
| return newVarbitChunk(varbitZeroEncoding) | ||
| }, | ||
| }, | ||
| Bigchunk: { | ||
| Name: "Bigchunk", | ||
| Name: "big-chunk", |
There was a problem hiding this comment.
question: If anyone is currently using the existing flag will this be a breaking change? Maybe it'd be best to keep the values for encoding types as is.
| func (Config) RegisterFlags(f *flag.FlagSet) { | ||
| f.Var(&DefaultEncoding, "ingester.chunk-encoding", "Encoding version to use for chunks.") | ||
| func (cfg *Config) RegisterFlags(f *flag.FlagSet) { | ||
| f.StringVar(&cfg.EncodingName, "encoding.chunk-encoding", "BigChunk", "Encoding version to use for chunks.") |
There was a problem hiding this comment.
Unfortunately we can't change CLI flag names or default values, otherwise we break our backward compatibility guarantee (see https://cortexmetrics.io/docs/configuration/v1guarantees/).
There was a problem hiding this comment.
Thank you for reminding!
There was a problem hiding this comment.
@pracucci Is it allowed to set a value using different names in CLI and json?
E.g. set DefaultEnconding using the CLI flag ingester.chunk-encoding, but set it as
encoding:
chunk_encoding: BigChunk in yaml config files?
|
This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions. |
|
Chunk storage has now been deprecated: #4268 |
What this PR does:
Adds a field to
Encoding.Configto allow parsing the string value from config files.DefaultEncodingis later set infunc Newatcortex.go.Please let me know if some tests should be introduced.
Any suggestion is appreciated!
Which issue(s) this PR fixes:
Fixes #2077
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]