LittDB compression#3769
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3769 +/- ##
==========================================
- Coverage 59.88% 58.97% -0.91%
==========================================
Files 2288 2203 -85
Lines 190023 180436 -9587
==========================================
- Hits 113786 106414 -7372
+ Misses 66091 64664 -1427
+ Partials 10146 9358 -788
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
PR SummaryHigh Risk Overview On-disk format: segment metadata bumps to v4 with a persisted compression byte; legacy v3 segments still load as uncompressed. Values are written through Semantics: secondary keys on compressed tables must be full-value aliases (no sub-range slicing). Iterator and keymap paths are updated accordingly; unflushed-cache batching tracks raw value bytes, not compressed on-disk sizes. OTel metrics cover compression latency and ratio. Extensive integration tests cover flush ordering, restart/toggle, recovery, GC, and iteration. Reviewed by Cursor Bugbot for commit dab46c8. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 23cfd53. Configure here.
| // CompressionS2 compresses each value with the S2 block codec (github.com/klauspost/compress/s2), | ||
| // a high-throughput, Snappy-compatible algorithm. The encoded block is self-describing: the | ||
| // decompressed length is recoverable from the block itself, so it is not stored separately. | ||
| CompressionS2 CompressionAlgorithm = 1 |
There was a problem hiding this comment.
Do we plan to do any benchmark here for other alternatives ? I saw that MinLZ might be even faster? What about traditional ones like lz4, gzip, zstd?
There was a problem hiding this comment.
I'm open to the idea of of considering alternate compression algorithms when we do formal benchmarking of compression.
Originally, I was planning on handing compression benchmarking off to Kartik. But since will be switching focus to integrations, it might be a little time until we get around to this. Opened a ticket so we don't forget about this. https://linear.app/seilabs/issue/STO-659/benchmark-littdb-compression
| compressed := make([][]byte, len(req.values)) | ||
| var uncompressedBytes uint64 | ||
| var compressedBytes uint64 | ||
| for i, kv := range req.values { |
There was a problem hiding this comment.
The compression loop processes one batch at a time on a single goroutine, which is necessary to preserve flush ordering. Under high write throughput with large batches, compression could become the pipeline bottleneck. The design is correct as-is, but if this becomes a problem, we could parallelize compression within a batch (each value is independent) while keeping inter-batch ordering serial.
There was a problem hiding this comment.
Added a godoc mentioning this, in case we end up getting bottlenecked here:
// Future work: if the compression thread ever becomes a bottleneck, consider the following tactics:
// - Fan out batches, handle normal work serially.
// Simple, but only helps with batch workloads.
// - Fan out all requests, have this thread only responsible for maintaining ordering.
// Complex but improves all worloads.
| case CompressionNone: | ||
| return src, nil | ||
| case CompressionS2: | ||
| decompressed, err := s2.Decode(nil, src) |
There was a problem hiding this comment.
Every Read on a compressed segment allocates a new buffer via s2.Decode(nil, src). For read-heavy workloads, this adds GC pressure. A sync.Pool of decompression buffers (sized to the expected decompressed length) would reduce allocations on the hot path.
There was a problem hiding this comment.
We don't copy the data out of the buffers, so it would be very tricky to reuse these buffers. We'd have to expose the sync.Pool via API to the calling context, and require that callers to the DB recycle their buffers after they are are done with them. Since this would add a lot of complexity, I'm not sure it's worth doing , absent benchmark/profiling results that suggest golang GC pressure is degrading performance.
Thoughts?

Describe your changes and provide context
Adds optional compression for LittDB.