Skip to content

Commit

Permalink
Fix name and default of log burst size
Browse files Browse the repository at this point in the history
This parameter is a _size_, not a _rate_.  Name it correctly.
Also, I don't think we want processes to be able to output 25,000
lines instantaneously - that's what the rate limit was intended to avoid.
Changed the default to 1,000.
  • Loading branch information
bboreham committed Oct 10, 2023
1 parent 8a39aa1 commit a17fe90
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* [CHANGE] Ingester: changed the default value for the experimental configuration parameter `-blocks-storage.tsdb.early-head-compaction-min-estimated-series-reduction-percentage` from 10 to 15. #6186
* [CHANGE] Update Go version to 1.21.2. #6244
* [CHANGE] Ingester: `/ingester/push` HTTP endpoint has been removed. This endpoint was added for testing and troubleshooting, but was never documented or used for anything. #6299
* [CHANGE] Experimental setting `-log.rate-limit-logs-per-second-burst` renamed to `-log.rate-limit-logs-burst-size`. #6230
* [FEATURE] Query-frontend: add experimental support for query blocking. Queries are blocked on a per-tenant basis and is configured via the limit `blocked_queries`. #5609
* [FEATURE] Vault: Added support for new Vault authentication methods: `AppRole`, `Kubernetes`, `UserPass` and `Token`. #6143
* [ENHANCEMENT] Ingester: exported summary `cortex_ingester_inflight_push_requests_summary` tracking total number of inflight requests in percentile buckets. #5845
Expand Down
4 changes: 2 additions & 2 deletions cmd/mimir/help-all.txt.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -1415,10 +1415,10 @@ Usage of ./cmd/mimir/mimir:
Only log messages with the given severity or above. Valid levels: [debug, info, warn, error] (default info)
-log.rate-limit-enabled
[experimental] Use rate limited logger to reduce the number of logged messages per second.
-log.rate-limit-logs-burst-size int
[experimental] Burst size, i.e., maximum number of messages that can be logged at once, temporarily exceeding the configured maximum logs per second. (default 1000)
-log.rate-limit-logs-per-second float
[experimental] Maximum number of messages per second to be logged. (default 10000)
-log.rate-limit-logs-per-second-burst int
[experimental] Burst size, i.e., maximum number of messages that can be logged in a second, temporarily exceeding the configured maximum logs per second. (default 25000)
-max-separate-metrics-groups-per-user int
[experimental] Maximum number of groups allowed per user by which specified distributor and ingester metrics can be further separated. (default 1000)
-mem-ballast-size-bytes int
Expand Down
32 changes: 16 additions & 16 deletions cmd/mimir/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,17 @@ const (
var testMode = false

type mainFlags struct {
ballastBytes int `category:"advanced"`
mutexProfileFraction int `category:"advanced"`
blockProfileRate int `category:"advanced"`
useBufferedLogger bool `category:"deprecated"` // Deprecated: deprecated in Mimir 2.11, remove it in 2.13.
rateLimitedLogsEnabled bool `category:"experimental"`
rateLimitedLogsPerSecond float64 `category:"experimental"`
rateLimitedLogsPerSecondBurst int `category:"experimental"`
printVersion bool
printModules bool
printHelp bool
printHelpAll bool
ballastBytes int `category:"advanced"`
mutexProfileFraction int `category:"advanced"`
blockProfileRate int `category:"advanced"`
useBufferedLogger bool `category:"deprecated"` // Deprecated: deprecated in Mimir 2.11, remove it in 2.13.
rateLimitedLogsEnabled bool `category:"experimental"`
rateLimitedLogsPerSecond float64 `category:"experimental"`
rateLimitedLogsBurstSize int `category:"experimental"`
printVersion bool
printModules bool
printHelp bool
printHelpAll bool
}

func (mf *mainFlags) registerFlags(fs *flag.FlagSet) {
Expand All @@ -73,7 +73,7 @@ func (mf *mainFlags) registerFlags(fs *flag.FlagSet) {
fs.BoolVar(&mf.useBufferedLogger, "log.buffered", true, "Use a buffered logger to reduce write contention.")
fs.BoolVar(&mf.rateLimitedLogsEnabled, "log.rate-limit-enabled", false, "Use rate limited logger to reduce the number of logged messages per second.")
fs.Float64Var(&mf.rateLimitedLogsPerSecond, "log.rate-limit-logs-per-second", 10000, "Maximum number of messages per second to be logged.")
fs.IntVar(&mf.rateLimitedLogsPerSecondBurst, "log.rate-limit-logs-per-second-burst", 25000, "Burst size, i.e., maximum number of messages that can be logged in a second, temporarily exceeding the configured maximum logs per second.")
fs.IntVar(&mf.rateLimitedLogsBurstSize, "log.rate-limit-logs-burst-size", 1000, "Burst size, i.e., maximum number of messages that can be logged at once, temporarily exceeding the configured maximum logs per second.")
fs.BoolVar(&mf.printVersion, "version", false, "Print application version and exit.")
fs.BoolVar(&mf.printModules, "modules", false, "List available values that can be used as target.")
fs.BoolVar(&mf.printHelp, "help", false, "Print basic help.")
Expand Down Expand Up @@ -173,10 +173,10 @@ func main() {

reg := prometheus.DefaultRegisterer
cfg.Server.Log = util_log.InitLogger(cfg.Server.LogFormat, cfg.Server.LogLevel, mainFlags.useBufferedLogger, util_log.RateLimitedLoggerCfg{
Enabled: mainFlags.rateLimitedLogsEnabled,
LogsPerSecond: mainFlags.rateLimitedLogsPerSecond,
LogsPerSecondBurst: mainFlags.rateLimitedLogsPerSecondBurst,
Registry: reg,
Enabled: mainFlags.rateLimitedLogsEnabled,
LogsPerSecond: mainFlags.rateLimitedLogsPerSecond,
LogsBurstSize: mainFlags.rateLimitedLogsBurstSize,
Registry: reg,
})

var ballast = util.AllocateBallast(mainFlags.ballastBytes)
Expand Down
2 changes: 1 addition & 1 deletion docs/sources/mimir/configure/about-versioning.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ The following features are currently experimental:
- Rate limited logger support
- `log.rate-limit-enabled`
- `log.rate-limit-logs-per-second`
- `log.rate-limit-logs-per-second-burst`
- `log.rate-limit-logs-burst-size`
- Timeseries Unmarshal caching optimization in distributor (`-timeseries-unmarshal-caching-optimization-enabled`)
- Reusing buffers for marshalling write requests in distributors (`-distributor.write-requests-buffer-pooling-enabled`)
- Using a worker pool for handling GRPC requests (`-server.grpc.num-workers`)
Expand Down
10 changes: 5 additions & 5 deletions pkg/util/log/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ var (
)

type RateLimitedLoggerCfg struct {
Enabled bool
LogsPerSecond float64
LogsPerSecondBurst int
Registry prometheus.Registerer
Enabled bool
LogsPerSecond float64
LogsBurstSize int
Registry prometheus.Registerer
}

// InitLogger initialises the global gokit logger (util_log.Logger) and returns that logger.
Expand All @@ -41,7 +41,7 @@ func InitLogger(logFormat string, logLevel dslog.Level, buffered bool, rateLimit
if rateLimitedCfg.Enabled {
// use UTC timestamps and skip 6 stack frames if rate limited logger is needed.
logger = log.With(logger, "ts", log.DefaultTimestampUTC, "caller", log.Caller(6))
logger = dslog.NewRateLimitedLogger(logger, rateLimitedCfg.LogsPerSecond, rateLimitedCfg.LogsPerSecondBurst, rateLimitedCfg.Registry)
logger = dslog.NewRateLimitedLogger(logger, rateLimitedCfg.LogsPerSecond, rateLimitedCfg.LogsBurstSize, rateLimitedCfg.Registry)
} else {
// use UTC timestamps and skip 5 stack frames if no rate limited logger is needed.
logger = log.With(logger, "ts", log.DefaultTimestampUTC, "caller", log.Caller(5))
Expand Down
8 changes: 4 additions & 4 deletions pkg/util/log/log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ func ExampleInitLogger() {
cfg := server.Config{}
_ = cfg.LogLevel.Set("info")
rateLimitedCfg := log.RateLimitedLoggerCfg{
Enabled: true,
LogsPerSecond: 1,
LogsPerSecondBurst: 4,
Registry: prometheus.NewPedanticRegistry(),
Enabled: true,
LogsPerSecond: 1,
LogsBurstSize: 4,
Registry: prometheus.NewPedanticRegistry(),
}
cfg.Log = log.InitLogger(cfg.LogFormat, cfg.LogLevel, false, rateLimitedCfg)

Expand Down

0 comments on commit a17fe90

Please sign in to comment.