Skip to content

Change default compression from Snappy to LZ4#14818

Closed
pdillinger wants to merge 2 commits into
facebook:mainfrom
pdillinger:lz4_default
Closed

Change default compression from Snappy to LZ4#14818
pdillinger wants to merge 2 commits into
facebook:mainfrom
pdillinger:lz4_default

Conversation

@pdillinger

Copy link
Copy Markdown
Contributor

Summary:
The historical default block compression kSnappyCompression dates to when Snappy was the obvious fast/cheap choice. On modern server CPUs LZ4 matches or beats Snappy on compression ratio while decompressing substantially cheaper, so it is a better default. This changes the default ColumnFamilyOptions::compression to kLZ4Compression, with a runtime fallback of LZ4 -> Snappy -> none depending on what is compiled into the binary (new GetDefaultCompressionType() in util/compression.h). Only column families that do not explicitly set compression are affected (including compaction output when
CompactionOptions::compression == kDisableCompressionOption), and only newly written SST files; existing data is read as before. Doc comments, the Java bindings, and the sorted_run_builder example are updated to describe the new default.

Test Plan:
Adjusted two unit tests that implicitly depended on the old Snappy default to pin compression = kSnappyCompression: db_iterator_test's ReadAhead (readahead byte thresholds assume Snappy-sized files) and compaction_service_test's CustomFileChecksum (LSM shape after auto-compaction determined whether a manual CompactRange had work to do).

This change is primarily validated by performance testing. Added a compressreject db_bench benchmark (output buffer sized just below the predicted compressed size) to measure the cost of attempting then declining compression, alongside compress/uncompress. Both db_bench and sst_dump compress/decompress a modest block at a time, as in a real workload.

sst_dump on SST files from production workloads (4 files, 16KB blocks, single thread) on recent server-class AMD, Intel, and ARM CPUs, LZ4 vs Snappy:

  • Compression ratio: comparable; LZ4 is slightly smaller on the more compressible files (up to ~8%) and within ~2% on the rest.
  • Compression (write) CPU: a wash, within ~2% either direction.
  • Decompression (read) CPU: the clear win -- Snappy costs ~1.2x-1.5x as much as LZ4, i.e. LZ4 saves ~25-30% read CPU, consistently across AMD, Intel, and ARM.

db_bench synthetic workload (100-byte values), at 1 / 12 / 160 threads, LZ4 vs Snappy:

  • Compression throughput: LZ4 ~10-30% higher.
  • Decompression throughput: LZ4 much higher, and the advantage grows with core count -- from ~+12% at 12 threads to ~+45-50% at 160 threads, i.e. better multi-core scaling.
  • Rejection (insufficient ratio) path: comparable; LZ4 ~15-18% faster on compressible-but-rejected blocks and Snappy within ~10% on barely-compressible blocks. No meaningful regression, confirming incompressible data is still efficiently detected and stored raw.
Summary:
The historical default block compression `kSnappyCompression` dates to
when Snappy was the obvious fast/cheap choice. On modern server CPUs LZ4
matches or beats Snappy on compression ratio while decompressing
substantially cheaper, so it is a better default. This changes the
default `ColumnFamilyOptions::compression` to `kLZ4Compression`, with a
runtime fallback of LZ4 -> Snappy -> none depending on what is compiled
into the binary (new `GetDefaultCompressionType()` in util/compression.h).
Only column families that do not explicitly set `compression` are
affected (including compaction output when
`CompactionOptions::compression == kDisableCompressionOption`), and only
newly written SST files; existing data is read as before. Doc comments,
the Java bindings, and the sorted_run_builder example are updated to
describe the new default.

Test Plan:
Adjusted two unit tests that implicitly depended on the old Snappy
default to pin `compression = kSnappyCompression`: db_iterator_test's
ReadAhead (readahead byte thresholds assume Snappy-sized files) and
compaction_service_test's CustomFileChecksum (LSM shape after
auto-compaction determined whether a manual CompactRange had work to do).

This change is primarily validated by performance testing. Added a
`compressreject` db_bench benchmark (output buffer sized just below the
predicted compressed size) to measure the cost of attempting then
declining compression, alongside `compress`/`uncompress`. Both db_bench
and sst_dump compress/decompress a modest block at a time, as in a real
workload.

sst_dump on SST files from production workloads (4 files, 16KB blocks,
single thread) on recent server-class AMD, Intel, and ARM CPUs,
LZ4 vs Snappy:
  - Compression ratio: comparable; LZ4 is slightly smaller on the more
    compressible files (up to ~8%) and within ~2% on the rest.
  - Compression (write) CPU: a wash, within ~2% either direction.
  - Decompression (read) CPU: the clear win -- Snappy costs ~1.2x-1.5x as
    much as LZ4, i.e. LZ4 saves ~25-30% read CPU, consistently across
    AMD, Intel, and ARM.

db_bench synthetic workload (100-byte values), at 1 / 12 / 160 threads,
LZ4 vs Snappy:
  - Compression throughput: LZ4 ~10-30% higher.
  - Decompression throughput: LZ4 much higher, and the advantage grows
    with core count -- from ~+12% at 12 threads to ~+45-50% at 160
    threads, i.e. better multi-core scaling.
  - Rejection (insufficient ratio) path: comparable; LZ4 ~15-18% faster
    on compressible-but-rejected blocks and Snappy within ~10% on
    barely-compressible blocks. No meaningful regression, confirming
    incompressible data is still efficiently detected and stored raw.
@pdillinger pdillinger requested a review from joshkang97 June 4, 2026 16:15
@meta-cla meta-cla Bot added the CLA Signed label Jun 4, 2026
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

✅ clang-tidy: No findings on changed lines

Completed in 432.2s.

@meta-codesync

meta-codesync Bot commented Jun 4, 2026

Copy link
Copy Markdown

@pdillinger has imported this pull request. If you are a Meta employee, you can view this in D107536490.

@meta-codesync

meta-codesync Bot commented Jun 4, 2026

Copy link
Copy Markdown

@pdillinger has imported this pull request. If you are a Meta employee, you can view this in D107536490.

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🟡 Codex Code Review

Auto-triggered after CI passed — reviewing commit 126d671


Codex review failed before producing findings.

WARNING: proceeding, even though we could not update PATH: Refusing to create helper binaries under temporary dir "/tmp" (codex_home: AbsolutePathBuf("/tmp/codex-home"))
error: the argument '--base <BRANCH>' cannot be used with '[PROMPT]'

Usage: codex exec review --commit <SHA> --base <BRANCH> --title <TITLE> --model <MODEL> --config <key=value> --dangerously-bypass-approvals-and-sandbox --output-last-message <FILE> [PROMPT]

For more information, try '--help'.

ℹ️ About this response

Generated by Codex CLI.
Review methodology: claude_md/code_review.md

Limitations:

  • Codex may miss context from files not in the diff
  • Large PRs may be truncated
  • Always apply human judgment to AI suggestions

Commands:

  • /codex-review [context] — Request a code review
  • /codex-query <question> — Ask about the PR or codebase
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

✅ Claude Code Review

Auto-triggered after CI passed — reviewing commit 126d671


Summary

Well-motivated change to switch the default compression from Snappy to LZ4. The core implementation is correct: GetDefaultCompressionType() properly implements the LZ4 → Snappy → NoCompression fallback chain, and the two constructor call sites are updated consistently. SST file format stores compression type per-block, so existing data is unaffected. No high-severity issues found.

High-severity findings (0):
No high-severity findings.

Full review (click to expand)

Findings

🔴 HIGH

None.

🟡 MEDIUM

M1. OptimizeLevelStyleCompaction should use GetDefaultCompressionType()options/options.cc:688-691
  • Issue: OptimizeLevelStyleCompaction implements the exact same LZ4 → Snappy → None fallback hierarchy inline rather than calling the new centralized GetDefaultCompressionType(). This creates code duplication and a maintenance risk if the fallback logic ever changes.
  • Root cause: The PR introduces a centralization function but doesn't apply it to all existing instances of the same pattern.
  • Suggested fix: Replace lines 688-691 with compression_per_level[i] = GetDefaultCompressionType(); (the #include "util/compression.h" is already present in the file).
M2. db_bench --compression_type flag still defaults to "snappy"tools/db_bench_tool.cc:1434
  • Issue: The DEFINE_string(compression_type, "snappy", ...) flag and the static FLAGS_compression_type_e = kSnappyCompression on line 1436-1437 are not updated. Users running db_bench without --compression_type will still use Snappy, which is inconsistent with the new library default. The compress and uncompress benchmarks use FLAGS_compression_type_e, so the new LZ4 default is not exercised by default.
  • Root cause: db_bench has its own hardcoded default separate from ColumnFamilyOptions.
  • Suggested fix: This may be intentional (db_bench pins its own default for reproducibility), but if so, it should be documented. Otherwise, update the flag default to match GetDefaultCompressionType() or at least to "lz4".
M3. CMake builds default to WITH_LZ4=OFFCMakeLists.txt:99
  • Issue: option(WITH_LZ4 "build with lz4" OFF) means CMake users who don't explicitly enable LZ4 will get kSnappyCompression as the default (via fallback), which is the same as before but contradicts the stated new default. This is not a regression, but it means the advertised behavior change only takes effect for Make builds or CMake builds with -DWITH_LZ4=ON.
  • Root cause: The fallback chain correctly preserves Snappy, so this is not a correctness issue — just a documentation/expectation mismatch.
  • Suggested fix: Consider changing CMake default to option(WITH_LZ4 "build with lz4" ON), or document that the LZ4 default requires LZ4 to be enabled at build time. The release note should mention this.
M4. CompressReject benchmark may not always reject compression — tools/db_bench_tool.cc (new code)
  • Issue: The reject buffer size is computed as max(0.0, FLAGS_compression_ratio - 0.10) * input.size(). With the default FLAGS_compression_ratio = 0.5, reject_size = 0.4 * block_size. If the compressor achieves exactly 40% or better compression on the generated data, compression will succeed rather than be rejected. The benchmark detects this case and aborts with a helpful error message ("Compression unexpectedly not rejected; try a smaller compression_ratio"), which is good defensive design — but the default parameters could hit this edge case for some compressors.
  • Root cause: The CompressibleString generator produces data that compresses to approximately FLAGS_compression_ratio of its original size, but the actual ratio varies by algorithm. The 10% margin may be tight.
  • Suggested fix: Consider using a larger margin (e.g., 20%) or documenting the expected behavior more prominently. The current error message is adequate for users who encounter the issue.

🟢 LOW / NIT

L1. Performance data in doc comment is hardware-specific — include/rocksdb/options.h
  • Issue: The updated doc comment references "AMD EPYC-Genoa" specific performance numbers. The old comment referenced "Intel Core2 2.4GHz". While the new data is more relevant, hardware-specific benchmarks may become outdated.
  • Suggested fix: This is acceptable — the old comment was also hardware-specific. The numbers provide useful order-of-magnitude guidance.
L2. Uncompress benchmark per-iteration allocation — tools/db_bench_tool.cc (existing code)
  • Issue: The Uncompress benchmark allocates a fresh CacheAllocationPtr buffer every iteration via AllocateBlock(). With the increased 20GB limit, this means significantly more allocation/deallocation churn. The Compress benchmark efficiently reuses a single GrowableBuffer.
  • Suggested fix: This is pre-existing behavior that mirrors real-world decompression patterns (where the output buffer lifecycle is managed by the caller). Not a regression introduced by this PR, but the 20x increase in iterations amplifies the overhead. Could consider pre-allocating a reusable buffer for a purer decompression speed measurement, but this is a separate concern.
L3. Release note could mention build system requirements — unreleased_history/behavior_changes/default_compression_lz4.md
  • Issue: The release note correctly describes the behavior change and fallback chain, but doesn't mention that CMake builds default to LZ4 disabled.
  • Suggested fix: Add a sentence noting that the LZ4 default requires LZ4 to be compiled in (which it is for Make builds but requires -DWITH_LZ4=ON for CMake builds).
L4. Compress benchmark increases from 1GB to 3GB — tools/db_bench_tool.cc
  • Issue: The benchmark data volume increased 3x. This is reasonable for better statistical stability and is consistent with the Uncompress increase.
  • Suggested fix: None needed. The increase is proportional and practical.

Cross-Component Analysis

Context Does code execute? Assumptions hold? Action needed?
WritePreparedTxnDB YES (inherits CF options) YES (compression is orthogonal to txn visibility) Safe
ReadOnly DB NO (read-only, doesn't write SSTs) N/A Safe
CompactionService YES (uses OPTIONS file) YES (compression type in OPTIONS file) Safe, but remote worker needs LZ4
User-defined timestamps YES YES (compression independent of UDT) Safe
MemPurge NO (memtable-to-memtable) N/A Safe
BlobDB NO (blob_compression_type is separate) N/A Safe (separate config)
Old snapshots N/A (compression is per-write, not per-read) N/A Safe
FIFO/Universal compaction YES (inherits CF options) YES Safe

Fallback chain verification:

  • LZ4 compiled in, Snappy compiled in → kLZ4Compression
  • LZ4 not compiled in, Snappy compiled in → kSnappyCompression ✅ (no regression)
  • LZ4 compiled in, Snappy not compiled in → kLZ4Compression
  • Neither compiled in → kNoCompression ✅ (same as old behavior when Snappy unavailable)

Key debate resolution: Multiple review agents initially flagged the fallback chain as "critical" claiming it drops directly to kNoCompression when LZ4 is unavailable. This was incorrect — the actual GetDefaultCompressionType() implementation properly includes Snappy as an intermediate fallback: LZ4_Supported() ? kLZ4Compression : (Snappy_Supported() ? kSnappyCompression : kNoCompression). On systems without LZ4 but with Snappy, behavior is identical to the old default.

SST format safety: Compression type is stored per-block in the SST block trailer (1 byte at block_data[block_size]). Decompression reads this stored type, never the default. MANIFEST does not store compression type. Existing data is fully readable regardless of the default change.

Positive Observations

  • Centralizing the default via GetDefaultCompressionType() is good design — eliminates the scattered Snappy_Supported() ? ... pattern.
  • The fallback chain is well-designed: LZ4 → Snappy → None preserves backward compatibility.
  • Test pinning strategy (using kNoCompression with clear comments) is the right approach for test stability.
  • The CompressReject benchmark fills a genuine gap in db_bench's compression measurement capabilities.
  • The UNLIKELY macros in error paths of CompressReject follow existing conventions.
  • The release note in unreleased_history/behavior_changes/ is clear and covers the key points.
  • LZ4 has been supported in RocksDB since ~2015, so forward/backward compatibility is excellent.

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/code_review.md

Limitations:

  • Claude may miss context from files not in the diff
  • Large PRs may be truncated
  • Always apply human judgment to AI suggestions

Commands:

  • /claude-review [context] — Request a code review
  • /claude-query <question> — Ask about the PR or codebase
@meta-codesync

meta-codesync Bot commented Jun 4, 2026

Copy link
Copy Markdown

@pdillinger merged this pull request in 8053b94.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

1 participant