tsdb/chunkenc: add XOR2 chunk encoding and docs#18238
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
9fd90a2 to
ce2b180
Compare
| a.writeVDelta(v) | ||
| default: | ||
| tDelta = uint64(t - a.t) | ||
| dod := int64(tDelta - a.tDelta) |
There was a problem hiding this comment.
nit: Probably tDod would be bit more clear
| dod := int64(tDelta - a.tDelta) | |
| tDod := int64(tDelta - a.tDelta) |
|
ce2b180 to
433513d
Compare
|
/prombench restart main |
|
⏱️ Welcome (again) to Prometheus Benchmarking Tool. ⏱️ Compared versions: After successful deployment (check status here), the benchmarking results can be viewed at: Available Commands:
|
|
/prombench cancel |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
I will try to reduce a bit more the variance in prombench to asses the CPU difference. While 5% is not a 15% gain, I have seen workloads where we reach the 15%. On the 40 workloads I have found, none had regressions in disk size with this algorithm, so I am fairly confident it will benefit every user. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
XOR2 is based on #18238 With additional ST support. Signed-off-by: György Krajcsovits <[email protected]>
|
⏱️ Welcome (again) to Prometheus Benchmarking Tool. ⏱️ Compared versions: After successful deployment (check status here), the benchmarking results can be viewed at: Available Commands:
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
6df4aab to
bb3fdd6
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
96987f6 to
c8ee650
Compare
…float sample Signed-off-by: Julien Pivotto <[email protected]>
c8ee650 to
4d2ecae
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
XOR2Chunk uses a sync.Pool (xor2ChunkPool) so that mmap'd chunks can donate their grown backing slice to the next head chunk. The contract is that ReleaseXOR2Chunk() is called only after the chunk is fully written to disk and no longer referenced. ChunkDiskMapper violated this contract in two places: - Async queue path: WriteChunk() returns before the background writer has consumed the chunk, so ReleaseXOR2Chunk() could recycle the chunk while the queue still held its pointer. NewXOR2Chunk() would then reuse the pointer and overwrite the bytes the writer was about to flush, producing corrupt on-disk data and an eventual panic in NumSamples() when the empty/overwritten stream was later read back. - Synchronous path (default): writeChunk() stores the chunk pointer directly in chunkBuffer. ReleaseXOR2Chunk() followed by NewXOR2Chunk() can then overwrite the buffered chunk's bytes before a reader retrieves it via Chunk(). Fix both by cloning the XOR2 chunk bytes into an independent copy (via cloneRetainedXOR2Chunk) before the chunk is handed to either the queue or the buffer. ReleaseXOR2Chunk() can then safely recycle the original at any point after WriteChunk() returns. Add regression tests for both paths that block or sequence writes, release and reuse the original chunk, then assert the stored data matches the original samples. Signed-off-by: Julien Pivotto <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
… path Remove the sync.Pool introduced in the previous commit along with all clone-on-write machinery it required (cloneRetainedXOR2Chunk, ReleaseXOR2Chunk, xor2MaxPooledCapacity). The pool added complexity and a corruption risk that outweighs the allocation savings. In its place, add two targeted optimisations: - Cache the sample count in xor2Appender.num so each Append call no longer reads the two header bytes via binary.BigEndian.Uint16. - Add a 3-bit fast path in xor2Iterator.readValue that decodes the full control prefix in one integer operation when the bit buffer holds at least 3 bits, avoiding per-bit branching on the common hot path. Signed-off-by: Julien Pivotto <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
…te GC pressure XOR2Chunk previously embedded an xor2Appender struct which contains a *bstream pointer field. After Appender() is called, app.b points back to &c.b within the same allocation, creating an intra-object self-pointer. The GC must call runtime.findObject for every live XOR2Chunk to resolve that pointer, which is O(log N) in heap size. With millions of active head chunks this dominates GC scan time. Production CPU profiles confirm the overhead: runtime.findObject at 5.79% flat and runtime.scanobject at 9.71% cumulative in XOR2 vs essentially zero in XOR. Allocate xor2Appender on the heap in Appender() instead, matching the XOR approach. Appenders are short-lived (alive only during ingestion) while chunks live for minutes, so the allocation cost is negligible. Signed-off-by: Julien Pivotto <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
|
⏱️ Welcome (again) to Prometheus Benchmarking Tool. ⏱️ Compared versions: Custom benchmark version: After successful deployment (check status here), the benchmarking results can be viewed at: Available Commands:
|
…ncoding` (#18062) * feat(tsdb/chunkenc): add float chunk format with start timestamp support Signed-off-by: György Krajcsovits <[email protected]> * optimize code path and layout Signed-off-by: György Krajcsovits <[email protected]> * make new format usable in head Signed-off-by: György Krajcsovits <[email protected]> * fix issue with seeking to last sample again Signed-off-by: György Krajcsovits <[email protected]> * fix iterator benchmark for chunks not supporting ST Signed-off-by: György Krajcsovits <[email protected]> * reduce footprint of the xoroptst chunk iterator object It was 80 bytes with a lot of padding compared to the 56 bytes of the original xor chunk iterator. Made it 64 bytes, tightly packed. Signed-off-by: György Krajcsovits <[email protected]> * Fix benchmark expectations on ST in interator Signed-off-by: György Krajcsovits <[email protected]> * add inclusive delta test case Signed-off-by: György Krajcsovits <[email protected]> * make testcases independent of order Signed-off-by: György Krajcsovits <[email protected]> * drop unused code Signed-off-by: György Krajcsovits <[email protected]> * Drop commented out line Signed-off-by: György Krajcsovits <[email protected]> * documentation Signed-off-by: György Krajcsovits <[email protected]> * Small simplification in the doc Signed-off-by: György Krajcsovits <[email protected]> * Add delta st inclusive test case for random vt Signed-off-by: György Krajcsovits <[email protected]> * Switch to delta of difference of st to prev t from delta of delta of st. Signed-off-by: György Krajcsovits <[email protected]> * Write ST after T and V so we can write a single bit on the second sample Signed-off-by: György Krajcsovits <[email protected]> * verify chunk sample len function Signed-off-by: György Krajcsovits <[email protected]> * Reduce size of first st stored a little Signed-off-by: György Krajcsovits <[email protected]> * test the case where st equals the t Signed-off-by: György Krajcsovits <[email protected]> * add st equal t to bechmarks Signed-off-by: György Krajcsovits <[email protected]> * test(chunkenc): test that appender can contonue chunks Test that initializing a chunk appender from an existing chunk works correctly. Signed-off-by: György Krajcsovits <[email protected]> * fix(chunkenc): bug in initializing appender on existing chunk Signed-off-by: György Krajcsovits <[email protected]> * Add cases with jitter in the start time as well Signed-off-by: György Krajcsovits <[email protected]> * tsdb: ST-in-WAL: Counter implementation and benchmarks (#17671) Initial implementation of #17790. Only implements ST-per-sample for Counters. Tests and benchmarks updated. Note: This increases the size of the RefSample object for all users, whether st-per-sample is turned on or not. Signed-off-by: Owen Williams <[email protected]> * refactor: sed enableStStorage/enableSTStorage Signed-off-by: bwplotka <[email protected]> * feat[scrape]: add ST parsing support to scrape AppenderV2 flow (#18103) Signed-off-by: bwplotka <[email protected]> * feat(tsdb): change head opt EnableSTStorage to atomic (#18107) In downstream projects this needs to be set dynamically per tenant. Signed-off-by: György Krajcsovits <[email protected]> * Merge pull request #18108 from prometheus/bwplotka/fix scrape: add tests for ST appending; add warnings for ST feature flag users around _created drop * refact(tsdb): trivial rename (#18109) Signed-off-by: György Krajcsovits <[email protected]> * fix(tsdb): missing passing head option to wal/wbl write (#18113) Signed-off-by: György Krajcsovits <[email protected]> * feat(tsdb): allow using ST capable XOR chunks - retain format on read (#18013) * feat(tsdb): allow appending to ST capable XOR chunk optionally Only for float samples as of now. Supports for in-order and out-of-order samples. Make sure that on readout the ST capable chunks are returned automatically. When the chunks are returned as is, this is trivially true. When a chunk needs to be re-coded due to deletion (tombstone) markers, we take the encoding of the original chunk. When a chunk needs to be created from overlapping chunks, we observe whether ST is zero or not and create the new chunk based on that. Signed-off-by: György Krajcsovits <[email protected]> * fix test after merge Signed-off-by: bwplotka <[email protected]> * feat: RW2 sending ST support Signed-off-by: bwplotka <[email protected]> tmp Signed-off-by: bwplotka <[email protected]> * tests: test ST in a cheapest way possible Signed-off-by: bwplotka <[email protected]> * tests: add bench CLI recommended invokations Signed-off-by: bwplotka <[email protected]> * fixed tests after rebase Signed-off-by: bwplotka <[email protected]> * feat(chunkenc): replace xoroptst chunk encoding with xor2 XOR2 is based on #18238 With additional ST support. Signed-off-by: György Krajcsovits <[email protected]> * feat: add compliance RW sender test for agent Signed-off-by: bwplotka <[email protected]> * feat(agent): add support for appending ST Signed-off-by: bwplotka <[email protected]> * replace stray xoroptst words Signed-off-by: György Krajcsovits <[email protected]> * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> Signed-off-by: Bartlomiej Plotka <[email protected]> * post merge conflict fixes Signed-off-by: bwplotka <[email protected]> * feat(tsdb): register st_storage in feature API Register the st-storage feature flag in the feature registry via the TSDB options, consistent with how other TSDB features like exemplar_storage and delayed_compaction are registered. Signed-off-by: György Krajcsovits <[email protected]> Coded with Claude Sonnet 4.6. * Document xor2-encoding feature flag Signed-off-by: Carrie Edwards <[email protected]> * Add xor2-encoding feature flag Signed-off-by: Carrie Edwards <[email protected]> * Update CHANGELOG Signed-off-by: Carrie Edwards <[email protected]> * Fix linting Signed-off-by: Carrie Edwards <[email protected]> * Remove setting of xor2 encoding option in db open Signed-off-by: Carrie Edwards <[email protected]> * Fix tests Signed-off-by: Carrie Edwards <[email protected]> * Fix linting Signed-off-by: Carrie Edwards <[email protected]> * Update feature flag description Signed-off-by: Carrie Edwards <[email protected]> * Update comments and feature flag description Signed-off-by: Carrie Edwards <[email protected]> * Update documentation for st-storage feature Signed-off-by: Carrie Edwards <[email protected]> * st: disconnect st-storage with xor2-encoding given planned experiments (#18316) * st: disconnect st-storage with xor2-encoding given planned experiments Signed-off-by: bwplotka <[email protected]> * Update docs/feature_flags.md Co-authored-by: George Krajcsovits <[email protected]> Signed-off-by: Bartlomiej Plotka <[email protected]> * Update docs/feature_flags.md Co-authored-by: George Krajcsovits <[email protected]> Signed-off-by: Bartlomiej Plotka <[email protected]> * Update docs/feature_flags.md Co-authored-by: George Krajcsovits <[email protected]> Signed-off-by: Bartlomiej Plotka <[email protected]> * Update docs/feature_flags.md Co-authored-by: George Krajcsovits <[email protected]> Signed-off-by: Bartlomiej Plotka <[email protected]> --------- Signed-off-by: bwplotka <[email protected]> Signed-off-by: Bartlomiej Plotka <[email protected]> Co-authored-by: George Krajcsovits <[email protected]> --------- Signed-off-by: György Krajcsovits <[email protected]> Signed-off-by: Ganesh Vernekar <[email protected]> Signed-off-by: Bryan Boreham <[email protected]> Signed-off-by: Aleksandr Smirnov <[email protected]> Signed-off-by: Mohammad Abbasi <[email protected]> Signed-off-by: matt-gp <[email protected]> Signed-off-by: Ian Kerins <[email protected]> Signed-off-by: SuperQ <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Arve Knudsen <[email protected]> Signed-off-by: ffgan <[email protected]> Signed-off-by: Patryk Prus <[email protected]> Signed-off-by: Owen Williams <[email protected]> Signed-off-by: bwplotka <[email protected]> Signed-off-by: 3Juhwan <[email protected]> Signed-off-by: Sammy Tran <[email protected]> Signed-off-by: Casie Chen <[email protected]> Signed-off-by: Dan Cech <[email protected]> Signed-off-by: kakabisht <[email protected]> Signed-off-by: Jeanette Tan <[email protected]> Signed-off-by: Divyansh Mishra <[email protected]> Signed-off-by: Varun Chawla <[email protected]> Signed-off-by: Martin Valiente Ainz <[email protected]> Signed-off-by: Kyle Eckhart <[email protected]> Signed-off-by: Julien Pivotto <[email protected]> Signed-off-by: Matthieu MOREL <[email protected]> Signed-off-by: Linas Medziunas <[email protected]> Signed-off-by: Björn Rabenstein <[email protected]> Signed-off-by: beorn7 <[email protected]> Signed-off-by: Sayuru <[email protected]> Signed-off-by: Matt <[email protected]> Signed-off-by: Bartlomiej Plotka <[email protected]> Signed-off-by: Carrie Edwards <[email protected]> Co-authored-by: Bartlomiej Plotka <[email protected]> Co-authored-by: Ganesh Vernekar <[email protected]> Co-authored-by: Bryan Boreham <[email protected]> Co-authored-by: Sasha <[email protected]> Co-authored-by: Mohammad Abbasi <[email protected]> Co-authored-by: matt-gp <[email protected]> Co-authored-by: Ian Kerins <[email protected]> Co-authored-by: SuperQ <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Arve Knudsen <[email protected]> Co-authored-by: Julien <[email protected]> Co-authored-by: ffgan <[email protected]> Co-authored-by: Patryk Prus <[email protected]> Co-authored-by: Ganesh Vernekar <[email protected]> Co-authored-by: Joe Adams <[email protected]> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Owen Williams <[email protected]> Co-authored-by: 3Juhwan <[email protected]> Co-authored-by: Casie Chen <[email protected]> Co-authored-by: Dan Cech <[email protected]> Co-authored-by: hridyesh bisht <[email protected]> Co-authored-by: zenador <[email protected]> Co-authored-by: Divyansh Mishra <[email protected]> Co-authored-by: Varun Chawla <[email protected]> Co-authored-by: Martin Valiente Ainz <[email protected]> Co-authored-by: Kyle Eckhart <[email protected]> Co-authored-by: Matthieu MOREL <[email protected]> Co-authored-by: Linas Medžiūnas <[email protected]> Co-authored-by: Björn Rabenstein <[email protected]> Co-authored-by: beorn7 <[email protected]> Co-authored-by: Sayuru <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Carrie Edwards <[email protected]>
|
/prombench cancel |
|
Benchmark cancel is in progress. |
Which issue(s) does the PR fix:
Does this PR introduce a user-facing change?