feat(core): add write-to-pk-ut-table config to skip PkUT table writes - #2300
Open
sherali42 wants to merge 3 commits into
Open
feat(core): add write-to-pk-ut-table config to skip PkUT table writes#2300sherali42 wants to merge 3 commits into
sherali42 wants to merge 3 commits into
Conversation
Add an explicit, operator-controlled `write-to-pk-ut-table` boolean to StoreConfig (default true). The partkeysbyupdatetime (PkUT) table is only consumed by the downsampler's DSIndexJob, so when downsampling is disabled the per-flush write is wasted work. Operators can now set this false to skip it. Changes: - StoreConfig gains a `writeToPkUTTable` field (default true), parsed from the HOCON key `write-to-pk-ut-table`, included in `toConfig` for round-trip serialization, and present in the inline default config string. - `write-to-pk-ut-table = true` added (documented) to the store-config defaults block in filodb-defaults.conf. - TimeSeriesShard.writeDirtyPartKeys now passes storeConfig.writeToPkUTTable as the trailing arg to colStore.writePartKeys (previously relied on the method default of true). The flag is an explicit config, not derived from DownsampleConfig. Default behavior is unchanged: absent or true still writes the PkUT table exactly as before. The downsample path in DownsamplableOnDemandPagingShard is untouched (it still explicitly passes false). Added StoreConfigSpec covering the default-true, override-false, and toConfig round-trip cases (3 tests pass; run in-process since the env's JDK rejects the build's forked-test CMS VM options). Co-Authored-By: Claude Opus 4.8 (1M context) via Établi <noreply@anthropic.com>
The `write-to-pk-ut-table` flag controls whether the partkeysbyupdatetime (PkUT) table is written on the raw ingestion flush path (TimeSeriesShard.writeDirtyPartKeys). Commit f63e03b mistakenly added it to the `downsample-store-config` block in filodb-defaults.conf, but the downsample flush path (DownsamplableOnDemandPagingShard.writeDirtyPartKeys) always passes `false` explicitly, so that entry was never consumed — it was dead config with zero behavioral effect. The raw store-config default already lives in code as `StoreConfig.defaults` (`write-to-pk-ut-table = true`), and `StoreConfig.apply` resolves per-dataset `store {}` overrides via `withFallback(defaults)`. So no code change is needed: a dataset overrides the flag in its source-config `store {}` block and falls back to the code default when absent. Changes: - Remove the dead `write-to-pk-ut-table` entry and its comment from the `downsample-store-config` block in filodb-defaults.conf. - Document the per-dataset override with a commented example in the `store {}` block of conf/timeseries-dev-source.conf. - Add StoreConfigSpec proving an absent key falls back to true, a `store {}` override of false yields `writeToPkUTTable == false`, an explicit true is honored, and the value round-trips through `toConfig`. Default behavior is unchanged: absent key => true => PkUT table written exactly as before. Tests run in-process (`Test/fork := false`) because this env's JDK rejects the build's forked-test CMS VM options. Co-Authored-By: Claude Opus 4.8 (1M context) via Établi <noreply@anthropic.com>
Add explicit, operator-controlled toggles for writing the Cassandra
`ingestion_time_index` (write-time index) table on both the raw-ingestion
flush path and the downsample Spark job. Both default to true, so behavior is
unchanged unless explicitly disabled. Mirrors the existing
`write-to-pk-ut-table` flag.
Raw ingestion (per-dataset via StoreConfig):
- StoreConfig gains `writeToIngestionTimeIndex` (default true), parsed from the
HOCON key `write-to-ingestion-time-index`, added to `StoreConfig.defaults`
and the `toConfig` round-trip map (IngestionConfig.scala).
- `write(...)` on the ChunkSink/ColumnStore trait and all impls (NullColumnStore,
CassandraColumnStore) gains a `writeToIngestionTimeIndex: Boolean = true`
param; default keeps every caller compatible.
- CassandraColumnStore.write gates `writeIndices` on the flag: when false the
index write is skipped (yields Success in the for-comprehension) while chunks
are still written and the returned Response is unchanged.
- TimeSeriesShard.writeChunks passes `storeConfig.writeToIngestionTimeIndex`
into `colStore.write`.
- Per-dataset override is set in a dataset's source-config `store {}` block;
documented with a commented example in conf/timeseries-dev-source.conf. No
store-config block added to filodb-defaults.conf (raw store defaults live in
the code `StoreConfig.defaults`).
Downsample Spark job (DownsamplerSettings flag):
- DownsamplerSettings gains `writeToIngestionTimeIndex`, read from downsampler
config key `write-to-ingestion-time-index` (default true, added to the
`downsampler {}` block in filodb-defaults.conf where other downsampler
defaults live).
- BatchDownsampler.persistDownsampledChunks gates only the index write block
(indexTable / indexInsert / Await) on the flag; downsampled chunks are still
persisted. The flag is read once outside the Spark closure.
Safety: the raw dataset's ingestion_time_index is what the downsample job
queries (getChunksByIngestionTimeRange) to find chunks to downsample, so
disabling raw writes is only safe when downsampling does not consume it. The
downsampler flag governs the downsample dataset's index (used by
repair/ChunkCopier and re-downsampling). Both caveats are noted in code and
config comments.
Flags are explicit config, not derived from DownsampleConfig.
Tests:
- StoreConfigSpec extended with default-true, override-false, explicit-true, and
toConfig round-trip cases for write-to-ingestion-time-index (8 tests pass).
- New DownsamplerSettingsSpec covers default-true and override-false (2 pass).
- Tests run in-process (Test/fork := false) since this env's JDK rejects the
build's forked-test CMS VM options.
- `sbt core/compile cassandra/compile sparkJobs/compile` all succeed.
Co-Authored-By: Claude Opus 4.8 (1M context) via Établi <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) via Établi <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request checklist
Add an explicit, operator-controlled
write-to-pk-ut-tableboolean to StoreConfig (default true). The partkeysbyupdatetime (PkUT) table is only consumed by the downsampler's DSIndexJob, so when downsampling is disabled the per-flush write is wasted work. Operators can now set this false to skip it.Changes:
writeToPkUTTablefield (default true), parsed from the HOCON keywrite-to-pk-ut-table, included intoConfigfor round-trip serialization, and present in the inline default config string.write-to-pk-ut-table = trueadded (documented) to the store-config defaults block in filodb-defaults.conf.The flag is an explicit config, not derived from DownsampleConfig. Default behavior is unchanged: absent or true still writes the PkUT table exactly as before. The downsample path in DownsamplableOnDemandPagingShard is untouched (it still explicitly passes false).