Add ooo native histogram ingestion #6626
Conversation
SungJin1212
force-pushed
the
Add-ingester.enable-ooo-native-histograms
branch
3 times, most recently
from
3aeaf64 to
063ce02
Compare
SungJin1212
force-pushed
the
Add-ingester.enable-ooo-native-histograms
branch
from
063ce02 to
d596fc5
Compare
docs/configuration/v1-guarantees.md
Outdated
| - `-blocks-storage.tsdb.out-of-order-cap-max` (int) CLI flag | ||
| - `-ingester.out-of-order-time-window` (duration) CLI flag | ||
| - `out_of_order_time_window` (duration) field in runtime config file | ||
| - `-ingester.enable-ooo-native-histograms` (bool) field in runtime config file |
There was a problem hiding this comment.
Nit. -ingester.enable-ooo-native-histograms is a flag instead of a config
| EnableMemorySnapshotOnShutdown: i.cfg.BlocksStorageConfig.TSDB.MemorySnapshotOnShutdown, | ||
| OutOfOrderTimeWindow: time.Duration(oooTimeWindow).Milliseconds(), | ||
| OutOfOrderCapMax: i.cfg.BlocksStorageConfig.TSDB.OutOfOrderCapMax, | ||
| EnableOOONativeHistograms: i.limits.EnableOOONativeHistograms(userID), |
There was a problem hiding this comment.
We should add hot reload because a user can enable or disable ooo native histogram during runtime.
But EnableOOONativeHistograms config is only specified once when creating the TSDB? So I wonder if it makes sense to make EnableOOONativeHistograms an runtime config as there is nothing can be changed during runtime
There was a problem hiding this comment.
Yeah, the reloader is necessary to change those configs at runtime.
I intended to configure EnableOOONativeHistograms as per tenant, just like OutOfOrderTimeWindow.
There was a problem hiding this comment.
out of order time window is a hot reloadable config.
I guess it is probably the same reason why we didn't make enable native histogram a runtime config.
There was a problem hiding this comment.
Shall we go with the global config for now?
SungJin1212
force-pushed
the
Add-ingester.enable-ooo-native-histograms
branch
from
d596fc5 to
27ca75a
Compare
Signed-off-by: SungJin1212 <[email protected]>
SungJin1212
force-pushed
the
Add-ingester.enable-ooo-native-histograms
branch
from
27ca75a to
29eaff6
Compare
|
I am wondering if we should remove the flag as Prometheus is planning to mark it as stable. prometheus/prometheus#16207 |
|
@yeya24 |
|
@yeya24 |
This PR adds a
-ingester.enable-ooo-native-histogramsflag to enable out-of-order native histogram ingestion per tenant.Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]