mirror of
https://github.com/facebook/rocksdb.git
synced 2026-10-23 08:09:02 +03:00
main
32 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Josh Kang
|
4b983da055 |
Spell out special member functions (rule of five) (#15387)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15387 ## Summary RocksDB's `.clang-tidy` enables `cppcoreguidelines-special-member-functions`, `cppcoreguidelines-virtual-class-destructor` and the noexcept-move checks, and clang warns `-Wdeprecated-copy-with-user-provided-dtor`. A full scan reports 476 findings. In most of them, a class declares a destructor (or copy operations) but leaves the rest of copy/move implicit. That has two costs. Readers can't tell whether a class may be copied or moved. And resource-owning classes keep an implicit copy that would double-free if anyone ever used it. This diff resolves 453 of the 476. Approach, picked per class from least to most invasive: 1. An empty user-provided destructor becomes `= default`. This covers most interface classes, such as `virtual ~X() {}` -> `= default`. If the class is still copied, that would only rename the deprecated-copy warning, so the destructor is removed instead when nothing needs it (`Customizable`, `TableFactory` and `InternalKeyComparator` inherit a virtual one; `IteratorWrapperBase` isn't polymorphic), and `MemTableRep::KeyComparator` gets defaulted copy and move. 2. Classes whose copy is already deleted get explicitly deleted move operations too. Move was already unavailable, so this only documents it. 3. Resource-owning classes that are never copied get deleted copy and move operations. Examples: the C API wrapper structs, which run a `destructor_` callback (edited in `tools/c_api_gen/c_base.cc` and regenerated into `db/c.cc`); `ShardedCache`, which frees its shards; iterator wrappers; and RAII handles. 4. Value types that are copied or moved get the missing operations as `= default`, which keeps their current behavior. 5. Exception specifications of existing functions are unchanged, except for `WriteBatch`'s move assignment (below): no other `noexcept` is added or removed. Only the move operations this diff newly declares (`Comparator`, `FileSampledStats` and the thread-local `Entry`) are `noexcept`; they copy only scalars or atomics. `WriteBatch`'s copy and move assignment destroyed `*this` and placement-new'ed a new `WriteBatch` over it. That is undefined behavior when `*this` is a subclass, and if the copy constructor threw, `*this` was left destroyed and was later destroyed again. Move assignment now assigns member-wise and ends in the same state as the move constructor, which leaves `has_key_with_ts_`, the timestamp-size tracking and the other fields it doesn't copy at their defaults. `rep_` is swapped through a temporary, because a plain string move-assignment can keep the old buffer (e.g. a large batch reset with `batch = WriteBatch()` would keep its memory). Nothing in it can throw, so it is now `noexcept`. Copy assignment copies into a temporary and moves it in, so a throwing copy leaves `*this` unchanged. Bugs this exposed and fixed: - `RestoreAfterCopyOrCreateWorkItem` (backup engine): the hand-written move constructor and assignment moved only some fields, so `from_file` and `to_file` were empty after `push_back(std::move(...))`. A restore that hit a checksum mismatch then reported "While restoring -> : ..." without file names. It now uses the compiler-generated moves, which are still `noexcept`. - `ExternalSstFileIngestionJob` lived in a `std::vector`, and its destructor frees raw-owned pointers, so the implicit copy used on vector reallocation would double-free. Its file batches also point into its own inline file storage, so neither a copy nor a move is safe after `Prepare()`. Copy and move are now deleted, and `DBImpl` holds jobs through `std::unique_ptr`. Compatibility: no public class loses an operation users can call. Public interfaces mostly only get `virtual ~X() = default;`. `WriteBatch`'s move assignment becomes `noexcept`, which is not part of the mangled name, so the ABI doesn't change. `SubcompactionJobInfo` and `CompactionJobInfo` intentionally stay C++20 aggregates, so users' aggregate and designated initializers keep compiling. `Configurable`, `CompareInterface`, `WriteBatchBase`, `ObjectLibrary::Entry` and `MemTableRep::KeyComparator` declare defaulted copy and move, and `Customizable` no longer declares a destructor. As a result, `Configurable` and `Customizable` subclasses that declare no destructor or copy operations of their own now move their registered-options list instead of copying it. In-tree, these are the mutable-options adapters, the table factories, the memory allocators and test classes; all are held through smart pointers or used in place, and `Clone()` copies, so none is moved by value. Not changed (23): `IODebugContext` (custom deep copy plus a `std::shared_mutex`), `DBWithColumnFamilies` in db_bench (stored in a `std::vector` with non-movable members), `Snapshot`'s protected virtual destructor (intended), `ManagedSnapshot`, the `SubcompactionJobInfo` and `CompactionJobInfo` aggregates, and the 16 noexcept-move-constructor and noexcept-swap findings on existing functions, including `autovector`'s move assignment (adding `noexcept` would change their exception specifications). Copying `ManagedSnapshot` already double-releases the snapshot. Fixing that needs a real move implementation and is a behavior change, so it is left for a separate diff. Reviewed By: rban1 Differential Revision: D123186309 fbshipit-source-id: 6f8e57e233e12199a007407130bb9a1f6d52e8d1 |
||
|
Josh Kang
|
da82001d81 |
Mark overriding methods and destructors override (#15366)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15366 ## Summary RocksDB's `.clang-tidy` enables `modernize-use-override`, and clang reports `-Winconsistent-missing-destructor-override` when a class marks its other overrides `override` but not its destructor. A full scan finds 190 `modernize-use-override`, 100 `inconsistent-missing-destructor-override` and 2 `suggest-override` findings in existing code. Each one comes back whenever a nearby line is edited. They also leave classes where it isn't obvious which methods are overrides and which introduce new virtuals. This diff marks every overriding member function and destructor `override` and drops the redundant `virtual` on them. It also changes `final override` to plain `final`, following the check's default. Methods that introduce a new virtual keep `virtual`. Most edits are clang-tidy's own fix-its, applied mechanically and then clang-formatted. Compatibility: no signatures change. In public headers the only effect is `override` on existing overrides (mostly destructors, plus `Cache::Name()` and `Cache::GetPrintableOptions()`, whose base declarations are unconditional), so user code that subclasses these types is unaffected. Every touched class now marks all of its overrides consistently. This matters because a partially-marked class would make clang emit `-Winconsistent-missing-override` in user builds. Reviewed By: rban1 Differential Revision: D123186308 fbshipit-source-id: 1f489ea32d938e3ec0faf9abc4f7ff49e042d37a |
||
Peter Dillinger
|
fcf382c95c |
Integrated and ratio-aware compression auto-skip (#15079)
Summary: Auto-skip compression (stop attempting compression on data that isn't compressing well, to save CPU) was previously an experimental CompressionManager wrapper (CreateAutoSkipCompressionManager) whose predictor lived in each per-thread working area. Besides being fragmented across parallel workers and reset on a tumbling window, it decided based on how OFTEN blocks failed to compress, ignoring how MUCH they compress -- so on mixed data it could give up substantial compression to save a little CPU (see performance results). This makes auto-skip a first-class, optional, ratio-aware feature of the block-based table builder, controlled by two new CompressionOptions fields: - auto_skip (default false): the on/off switch. There is no separate threshold knob; the decision reuses max_compressed_bytes_per_kb (the existing per-block "minimum compression worth keeping" bar) as the bar for whether attempting compression on the stream is worthwhile. - auto_skip_min_sample_every: sampling interval that bounds the local impact around a transition back to compressible data. 0 selects an internal default (currently 64 skipped data blocks between forced samples). The emit thread makes the skip/attempt decision once per data block (serial and parallel paths alike; data blocks only), so a skipped block is never dispatched to a compression worker. It is driven by a continuously updated, shared estimate q_hat of the aggregate stored/uncompressed ratio the builder would achieve by attempting -- each block counted at its achieved ratio, or 1.0 when stored raw: attempt while q_hat is at or below the bar, otherwise skip. Because the estimate is payoff-weighted, a stream whose blocks each save little -- or where only a few blocks compress well -- does not clear the bar and is skipped, while a stream that compresses substantially is attempted. The compression given up per KB is bounded by (1024 - max_compressed_bytes_per_kb). q_hat uses an asymmetric exponentially weighted moving average (EWMA), slightly faster toward better compression than toward worse; the fast side is deliberately gentle so a single marginally-compressible sample does not flip a whole file back to attempting. It is kept on its own cache line to avoid false sharing. Under parallel compression q_hat is updated with relaxed atomics, and the emit thread can decide from a value that lags the learning it drives by the in-flight depth -- tolerable concessions of accuracy and responsiveness for a coarse heuristic. The first data block of each file is always attempted (the sample countdown starts at 0), and while skipping the builder keeps sampling (one block after a randomized gap) so a return to compressible data is detected. q_hat is carried across the files a flush/compaction thread emits via thread_local state keyed by preferred compression type, so a sustained region does not re-learn on every file. The old CompressionManager-based AutoSkip is removed (CostAware is retained); db_stress, db_bench, and db_crashtest are migrated to the new option. db_bench is extended so it can generate the relevant regimes: --compressible_data_fraction and --compressible_data_run_length produce a mix of compressible/incompressible blocks -- i.i.d. per value (run length 1) or in alternating regions (large run length). The default (fraction 1.0) reproduces the historical single-pool behavior. Pull Request resolved: https://github.com/facebook/rocksdb/pull/15079 Test Plan: New/updated unit tests in `util/compression_test.cc` (`DBAutoSkipTest`): - skips sustained incompressible data; - never skips compressible data; - disabled by default never bypasses a block; - 4-thread parallel compression round-trips all data across alternating compressible/incompressible regions. **Performance** (`db_bench fillseq`, release, zstd level 3; 100k values of 16 KiB each, roughly one value per data block; WAL and auto-compaction off; median of 5 runs on a quiet host). Auto-skip trades one cost for another, so each cell reports two costs, both lower-is-better: CPU cost as process CPU time (user+sys) per GB of uncompressed input (s/GB; ~1.64 GB uncompressed here), and storage cost as the resulting DB size. Auto-skip does not appear to change parallelizability, so wall-clock s/GB tracks CPU s/GB (the two agree here); CPU s/GB is reported. Percentages are relative to the no-auto-skip baseline: negative CPU = cheaper, positive size = more storage (shown only where storage grows). | Workload | baseline (off) | (a) old wrapper | (b) draft | (c) this version | |---|---|---|---|---| | Incompressible | 2.33 s/GB, 1568 MB | 0.96 (-59%), 1568 MB | 0.82 (-65%), 1568 MB | 0.82 (-65%), 1568 MB | | Compressible | 3.55 s/GB, 710 MB | 3.54 (~0%), 710 MB | 3.56 (~0%), 710 MB | 3.55 (~0%), 710 MB | | Marginal-payoff mix | 3.80 s/GB, 1376 MB | 1.54 (-59%), 1523 MB (+11%) | 3.81 (~0%), 1376 MB | 1.43 (-62%), 1531 MB (+11%) | | High-payoff mix | 2.20 s/GB, 929 MB | 1.14 (-48%), 1417 MB (+53%) | 2.16 (~0%), 929 MB | 2.17 (~0%), 929 MB | A strategy wins by cutting CPU cost without paying much storage cost. On incompressible data all three cut CPU ~60% at no storage cost; on compressible data none skip (within run-to-run noise). The two mixes (50% incompressible, 50% compressible per block) separate the strategies: - Marginal-payoff mix: the compressible half barely clears the per-block bar, so the stream is not worth compressing -- skipping is right. (c) and (a) cut CPU ~60% for ~11% more storage. That extra storage is compression (c) intentionally declines because its aggregate benefit is below the configured worthwhile threshold (max_compressed_bytes_per_kb, ~12.5% per KB); it stays within the (1024 - max_compressed_bytes_per_kb) bound. The ratio-unaware draft (b) cannot see the low payoff, keeps compressing, and saves no CPU. - High-payoff mix: the compressible half compresses hard, so compressing is right. (c) and (b) keep full compression at baseline storage; the old wrapper (a) cuts CPU 48% but inflates storage +53% -- far beyond that bound -- giving up worthwhile compression. Only (c) makes the right call on both mixes: it never gives up worthwhile compression (unlike (a)) and does not waste CPU on low-payoff data (unlike (b)). Reviewed By: xingbowang Differential Revision: D115444567 Pulled By: pdillinger fbshipit-source-id: 05e122d25de95ddcd0edeb0dd07c9d25bead7293 |
||
|
Peter Dillinger
|
6d4a8144e0 |
Unify LZ4 and LZ4HC compression levels (#14819)
Summary: kLZ4Compression and kLZ4HCCompression share the same on-disk format and decompressor, but historically kLZ4Compression only honored negative (acceleration) levels while kLZ4HCCompression only honored positive levels. This unifies them so `compression_opts.level` alone selects the variant: level <= 0 uses LZ4 fast (acceleration = -level) and level >= 1 uses LZ4HC (1..12), regardless of which of the two types is configured. The configured type now only determines the default compression level (LZ4: acceleration 1, equivalent to level -1; LZ4HC: level 9). For code simplicity, the recorded per-block type comes from the compression type derived from the level, which could differ from the configured type. To preserve the originally configured choice for debugging/tracking, it is recorded as a `_type=<decimal>` pseudo-option in the rocksdb.compression_options SST table property. Out-of-range non-default levels are clamped to the nearest effective value (LZ4 acceleration capped at 65537, which also avoids signed overflow negating INT_MIN; LZ4HC level capped at 12). The cost-aware (auto-tune) compressor's LZ4 level grid is changed to negative accelerations so it actually exercises fast LZ4 (positive levels now route to LZ4HC). Related inclusion: The ZSTD library has a discontinuity at level=0, which maps to level 3, which is more aggressive than levels 1 and 2, which are more aggressive than levels -1, -2, etc. For better friendliness to auto-tuning (etc.), we now map level 0 to be the same as level -1, so that increasing compression level numbers have non-decreasing aggressiveness. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14819 Test Plan: New unit tests in compression_test.cc: - UnifiedLZ4LZ4HCLevels: for a representative set of non-default levels, both configured types produce identical output and the same recorded type (selected by the level), levels that clamp to the same effective parameter compress identically, and each round-trips; plus per-type default-level behavior. - ConfiguredCompressionTypeRecordedInProperties: the `_type=` pseudo- option appears in the SST table property for each configured type. - ZSTDLevelZeroMapsToMinusOne: level 0 behaves like -1, not like 3. Reviewed By: joshkang97 Differential Revision: D107536580 Pulled By: pdillinger fbshipit-source-id: a1281956f70a75d0620cb73d0bfb9ad76c52cca3 |
||
|
Peter Dillinger
|
01084e7e8e |
Disable parallel compression for fast built-in compressors (#14841)
Summary: Use Compressor::GetRecommendedParallelThreads() overrides to disable parallel compression for Snappy, LZ4 (accelerated, not LZ4HC), and for accelerated levels of ZSTD (level < 0). These do not generally benefit from parallel compression. Also add --verify_compression option to sst_dump for some basic "recompress" benchmarking with that option. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14841 Test Plan: unit test updated. In planning the scope of this change, I manually tested some production SST files with release build sst_dump --command=recompress and various settings for --compression_parallel_threads, --compression_types, and --compression_level, while I had the CPUs mostly busy using cache_bench in the background. Here's an example, before the change to override parallel_threads: ``` $ for PT in 1 8; do /usr/bin/time ./sst_dump --command=recompress --compression_parallel_threads=$PT --block_size=16384 --compression_types=kLZ4Compression --verify_compression=1 test.sst; done ... Compression: kLZ4Compression Block Size: 16384 Threads: 1 Cx level: 32767 Cx size: 168501634 Uncx size: 791721664 Ratio: 4.698599 Write usec: 2345894 Read usec: 429022 Cx count: 48661 (100.0%) Not cx for ratio: 0 ( 0.0%) Not cx otherwise: 0 ( 0.0%) 2.54user 0.29system 0:02.84elapsed 99%CPU (0avgtext+0avgdata 460816maxresident)k ... Compression: kLZ4Compression Block Size: 16384 Threads: 8 Cx level: 32767 Cx size: 168501634 Uncx size: 791721664 Ratio: 4.698599 Write usec: 2476459 Read usec: 439464 Cx count: 48661 (100.0%) Not cx for ratio: 0 ( 0.0%) Not cx otherwise: 0 ( 0.0%) 3.95user 0.33system 0:02.98elapsed 143%CPU (0avgtext+0avgdata 455504maxresident)k ``` Here as in many of the cases I'm changing, it actually takes longer to compress with parallel, despite the added parallel opportunity of verify_compression. And overall CPU is much higher from 2.54 `CPU*s` to 3.95 `CPU*s`. The difference disappears with the change, because both use single-threaded SST construction. Reviewed By: joshkang97 Differential Revision: D108075938 Pulled By: pdillinger fbshipit-source-id: 5d1fc77ddbccf9f3b24a4f0b20b2b3c43074e89d |
||
|
Peter Dillinger
|
959fa315b1 |
Use unique_ptr for StreamingCompress and StreamingUncompress (#14665)
Summary: Replace raw owning pointers with std::unique_ptr for StreamingCompress/Uncompress in Create functions and log::Writer/Reader, eliminating manual delete calls in destructors. Also update the StreamingCompressionTest to use unique_ptr for its local StreamingCompress, StreamingUncompress, and MemoryAllocator objects. Bonus: improve error message for missing/broken ruby (new build requirement) Pull Request resolved: https://github.com/facebook/rocksdb/pull/14665 Test Plan: ./log_test - all 211 tests pass. Reviewed By: xingbowang Differential Revision: D102381893 Pulled By: pdillinger fbshipit-source-id: 02806b46c8371474a969d6fa3012fff5eb7886c3 |
||
Yoshinori Matsunobu
|
3cdf942192 |
Make ParseCompressionNameForDisplay manager-aware (#14658)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14658 Follow up to D101463511. Add a hook and a manager-aware overload of so custom CompressionManagers can provide human-readable names for custom compression types while preserving the existing generic fallback when no compatible manager is available. Reviewed By: pdillinger Differential Revision: D102201365 fbshipit-source-id: 0c7456bb9db2e54927a4349d12c035fc8b5ad562 |
||
|
Peter Dillinger
|
b7395b3fa4 |
Allow Compressor to override parallel_threads for SST building (#14580)
Summary:
Add Compressor::GetRecommendedParallelThreads() virtual method so that the compression subsystem can influence the parallel compression thread count used when building SST files. The base Compressor returns 0 (no opinion), while built-in compressors (via CompressorBase) return the parallel_threads value from their CompressionOptions. This gives CompressionManager a clean mechanism to override parallel_threads by customizing the CompressionOptions passed to GetCompressor() in its GetCompressorForSST() implementation.
The table builder now reads the thread count from the compressor after creation, rather than only from CompressionOptions directly. Hard structural constraints (partition_filters without decoupled mode, user_defined_index_factory) still force single-threaded compression regardless of the compressor's recommendation.
Also adds a sync point in MaybeStartParallelCompression for test observability, and compression_test coverage for the new functionality.
Bonus: extends CompressionManagerCustomCompression test to cover re-opening a DB at format_version=7 with a non-default compression manager that uses the built-in CompatibilityName ("BuiltinV2"). Verifies that data is readable after re-opening without the original manager, and that neither GetId() nor CompatibilityName() resolves to the custom manager via CreateFromString.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14580
Test Plan:
New unit tests in compression_test:
- GetRecommendedParallelThreads: verifies built-in compressors return the parallel_threads from their CompressionOptions for all supported compression types.
- CompressionManagerOverridesParallelThreads: end-to-end test with a custom CompressionManager that overrides parallel_threads from 1 to 4 in GetCompressorForSST, verified via sync point that parallel compression actually activates with the overridden thread count, plus data readback verification.
Existing parallel compression tests (DBCompressionTestMaybeParallel) continue to pass.
Reviewed By: xingbowang
Differential Revision: D99896343
Pulled By: pdillinger
fbshipit-source-id: 6b3a30856a78641714d33ec7ba2099f33533d3af
|
||
|
Peter Dillinger
|
672389fd8c |
Remove obsolete compression code and some .h->.cc movement (#14325)
Summary: In follow-up to https://github.com/facebook/rocksdb/issues/14315 Remove obsolete code replaced by new Compressor/Decompressor interface: * OLD_CompressData and OLD_UncompressData * Individual compression/decompression functions (Snappy_*, Zlib_*, BZip2_*, LZ4_*, LZ4HC_*, XPRESS_*, ZSTD_Compress, ZSTD_Uncompress) * CompressionInfo and UncompressionInfo classes * UncompressionDict class * compression::PutDecompressedSizeInfo and GetDecompressedSizeInfo The only small refactoring in this change that is not pure code removal or movement is in blob_file_builder_test.cc. Move some function implementations etc. from compression.h to compression.cc: * CompressionTypeToString, CompressionTypeFromString, CompressionOptionsToString * ZSTD_TrainDictionary (both overloads), ZSTD_FinalizeDictionary * DecompressorDict::Populate * Most compression library includes Also cleaned up other includes of compression.h, which caused some other files to need new includes. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14325 Test Plan: existing tests Reviewed By: hx235 Differential Revision: D93120580 Pulled By: pdillinger fbshipit-source-id: ab5c50db7379c0387a8c0e379642c9ea2799eae5 |
||
|
Peter Dillinger
|
d8b1893c9d |
DROP support for block-based SST format_version < 2 (#14315)
Summary: ... and remove some old code and tech debt in the process. This is arguably a great milestone and precendent in RocksDB history as for the first time we are explicitly dropping support for the ability to read source-of-truth data in old formats. (We previously dropped support for reading some old bloom filters, but those are performance optimizers not source-of-truth. https://github.com/facebook/rocksdb/issues/10184) However, DBs written with default settings since release 4.6.0, which is very nearly 10 years ago, can still be read. And by using compaction with intermediate versions, there's an upgrade path going back to (AFAIK) early releases of LevelDB (from which RocksDB was forked). Some detail: * The magic number for LevelDB SST files (0xdb4775248b80fb57, most recently called kLegacyBlockBasedTableMagicNumber) now only exists in the code to provide a good error message and to test that good error message. * There is some notable refactoring and renaming around format_version handling. This is a bit of a messy area of code because the footer code being shared between different table formats (block-based, plain, cuckoo) means format_version in the footer is in ways tied to all of them, but in other ways is just tied to block-based table where we have been making updates. Hopefully code comments keep this clear. * Now that there are old format_versions we can't read (and can't write authoritatively in tests), I've needed to split out kMinSupportedFormatVersion into a constant for reads and for writes, currently the same at format_version=2. Comments describe how to update these in the future. * The idea of versioning the compression format is basically going away, though we're keeping BuiltinV2 in places just because it's already there. There's lots of room in the BuiltinV2 schema to expand to new built-in compression types, or new ways of handling existing compression algorithms. CompressionManager with CompatibilityName gives users the power to customize compression without the need for versions tied to format_version. Immediate follow-up: * Clean up compression loose ends like OLD_Compress, OLD_Uncompress Suggested follow-up: * Update plain table builder to migrate to new footer version so that we can drop support for legacy footer. We have to be careful that the (likely untested) forward compatibility path I put in place a while back works (or fix it and wait a while) before dropping support for plain table with legacy footer. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14315 Test Plan: * Some tests updated / added * A couple tests are obsolete: removed * Also updated format compatible test, which now doesn't need to dig as far back into history building RocksDB. Reviewed By: hx235 Differential Revision: D92577766 Pulled By: pdillinger fbshipit-source-id: a23be846189d901ce087af4ca9a99cef18445cb7 |
||
|
Peter Dillinger
|
6a79e02ebd |
Support pre-defined compression dictionaries (#14253)
Summary: ... in addition to those derived from samples. This could be useful when trade-offs favor an offline trained dictionary that's good for the whole work load, which can involve heavy-weight training, vs. on-the-fly training on samples for each file, which has limitations. This involves some breaking changes to some deeper parts of the new compression API. I'm not concerned about performance because this doesn't touch the per-block parts of the API, just the per-file parts. Bonus: change to CompressionManagerWrapper::FindCompatibleCompressionManager to implement what is likely the preferred behavior. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14253 Test Plan: unit test included Reviewed By: hx235 Differential Revision: D91082208 Pulled By: pdillinger fbshipit-source-id: 1442db65e15c9435437204c19787c96f7a40a207 |
||
|
Peter Dillinger
|
a6af317476 |
Use format_version=7 by default, fix perf bug (#14239)
Summary: Since it's been > 6 months and we have production uses, migrate to fv=7 by default. One unit test needed an update for the change to table properties with fv=7. On making this change, PresetCompressionDictTest tests detected extra memory usage by decompressing LZ4 with dictionary compression. This turned out to be a bug in `std::find` usage that led to using the ZSTD-optimized decompressor (with digested dictionary usage) in cases where it is not needed. I've fixed the bug and improved the unit tests that found the bug. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14239 Test Plan: existing tests, including format compatible CI job (updated, and run locally with SHORT_TEST=1) Reviewed By: hx235 Differential Revision: D90728697 Pulled By: pdillinger fbshipit-source-id: 8f1a0e9ca59a88c18eaa4cdfdea00309175ce30a |
||
|
Peter Dillinger
|
57036b68d9 |
Migrate blob handling to new compression APIs (#14234)
Summary: as part of the effort to get rid of OLD_CompressData and OLD_UncompressData and the old implementations in compression.h. It's unfortunate the the existing blob file schema doesn't allow storing blobs uncompressed when the compressed version is larger, so we have to work around that. Note that use of GrowableBuffer in place of std::string is intended to avoid the potential performance overhead of zeroing out memory before overwriting it. Also includes some cleanup of includes Pull Request resolved: https://github.com/facebook/rocksdb/pull/14234 Test Plan: some unit test updates as needed. Crash test covers integrated blob support. I'm not too concerned about performance, as until a future schema change, this code is committing the grave performance error of storing compressed data larger than uncompressed. Reviewed By: mszeszko-meta, hx235 Differential Revision: D90544049 Pulled By: pdillinger fbshipit-source-id: 2f2ed16de63990b797cc06c8dad36b5869dac302 |
||
|
Peter Dillinger
|
9c2c8f54fa |
Fix AutoSkipCompressorWrapper with new logic (#14150)
Summary: ... from https://github.com/facebook/rocksdb/issues/14140. The assertion in the default implementation of CompressorWrapper::MaybeCloneSpecialized() could fail because this wrapper wasn't overriding it when it should. (See the NOTE on that implementation.) Because this release already has a breaking modification to the Compressor API (adding Clone()), I took this opportunity to add 'const' to MaybeCloneSpecialized(). Also marked some compression classes as 'final' that could be marked as such. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14150 Test Plan: unit test expanded to cover this case (verified failing before). Audited the rest of our CompressorWrappers. Reviewed By: archang19 Differential Revision: D87793987 Pulled By: pdillinger fbshipit-source-id: 61c4469b84e4a47451a9942df09277faeeccfe63 |
||
|
Peter Dillinger
|
35148aca91 |
Improve distinct compression for index and data blocks (#14140)
Summary: This change enables a custom CompressionManager / Compressor to adopt custom handling for data and index blocks. In particular, index blocks for format_version >= 4 use a distinct variant of the block format. Thus, a potentially format-aware compression algorithm such as OpenZL should be told which kind of block we are compressing. (And previously I avoided passing block type in CompressBlock for efficient handling of things like dictionaries but also avoiding checks on every CompressBlock call.) Most of the change is in BlockBasedTableBuilder to call MaybeCloneSpecialized for both kDataBlock and for kIndexBlock. But I also needed some small tweaks/additions to the public API also: * Require a Clone() function from Compressors, to support proper implementations of MaybeCloneSpecialized() in wrapper Compressors. * Assert that the default implementation of CompressorWrapper::MaybeCloneSpecialized() is only used in allowable cases. * Convenience function Compressor::CloneMaybeSpecialized() This also fixes a serious bug/oversight in ManagedPtr for (ManagedWorkingArea) that somehow wasn't showing up before. It probably doesn't need a release note because CompressionManager stuff is still considered experimental. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14140 Test Plan: Greatly expanded DBCompressionTest.CompressionManagerWrapper to make sure the distinction between data blocks and index blocks is properly communicated to a custom CompressionManager/Compressor. The test includes processing the expected structure of data and index blocks, to serve as a tested example for structure-aware compressors. Reviewed By: hx235 Differential Revision: D87600019 Pulled By: pdillinger fbshipit-source-id: 252ef78910073a0e45f2c81dd45ac87ff8a41fc6 |
||
|
Peter Dillinger
|
9d3afcf543 |
Fix regression in LZ4 compression performance since 10.6 (#14017)
Summary: In RocksDB 10.6 with https://github.com/facebook/rocksdb/issues/13805, due to inaccurate testing of an async system, it went undetected at the time that LZ4 compression was using more CPU despite making a change to reuse stream objects which dramatically improved LZ4HC compression efficiency. This change switches to using a basic LZ4 compress API which appears to be faster than all of these: * Legacy behavior of creating LZ4_stream_t for each compression * 10.6-10.7 behavior of re-using streams between compressions for the same file (with stream-as-WorkingArea) * using LZ4's extState APIs without streams (with extState-as-WorkingArea) (data not shown in below results) Also in this PR: more improvements to sst_dump --recompress, which is arguably the best SST construction benchmark right now since db_bench seems to be so noisy due to backgroun flush+compaction, even with no compaction (FIFO). Streamlined some output and added a SST read time test, mostly for decompression performance. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14017 Test Plan: Performance test using sst_dump --recompress with newer sst_dump back-ported to 10.5: ``` ./sst_dump --command=recompress --compression_types=kLZ4Compression test5.sst --compression_level_from=-6 --compression_level_to=-1 ``` and with default compression level. 10.5: ``` Cx level: -6 Cx size: 61608137 Write usec: 880404 Cx level: -5 Cx size: 60793749 Write usec: 840903 Cx level: -4 Cx size: 58134030 Write usec: 836365 Cx level: -3 Cx size: 55193773 Write usec: 857113 Cx level: -2 Cx size: 54013891 Write usec: 855642 Cx level: -1 Cx size: 50400393 Write usec: 865194 Cx level: 32767 Cx size: 50400393 Write usec: 886310 ``` Before this change (showing the regression, more time, from 10.6: ``` Cx level: -6 Cx size: 61608137 Write usec: 933448 Cx level: -5 Cx size: 60793749 Write usec: 893826 Cx level: -4 Cx size: 58134030 Write usec: 891138 Cx level: -3 Cx size: 55193773 Write usec: 898461 Cx level: -2 Cx size: 54013891 Write usec: 897485 Cx level: -1 Cx size: 50400393 Write usec: 936970 Cx level: 32767 Cx size: 50400393 Write usec: 958764 ``` After this change (faster than both the above): ``` Cx level: -6 Cx size: 63641883 Write usec: 874190 Cx level: -5 Cx size: 58860032 Write usec: 834662 Cx level: -4 Cx size: 57150188 Write usec: 832707 Cx level: -3 Cx size: 58791894 Write usec: 850305 Cx level: -2 Cx size: 53145885 Write usec: 839574 Cx level: -1 Cx size: 49809139 Write usec: 845639 Cx level: 32767 Cx size: 49809139 Write usec: 875199 ``` Similar tests with dictionary compression show essentially no difference (need to use stream APIs and reuse doesn't seem to matter). LZ4HC also unaffected (still improved vs. 10.5) Reviewed By: hx235 Differential Revision: D83722880 Pulled By: pdillinger fbshipit-source-id: 30149dd187686d5dd98321e6aa7d74bd7653a905 |
||
|
Peter Dillinger
|
0a169cea0e |
Compressor::CompressBlock API change and refactoring/improvement (#13805)
Summary: The main motivation for this change is to more flexibly and efficiently support compressing data without extra copies when we do not want to support saving compressed data that is LARGER than the uncompressed. We believe pretty strongly that for the various workloads served by RocksDB, it is well worth a single byte compression marker so that we have the flexibility to save compressed or uncompressed data when compression is attempted. Why? Compression algorithms can add tens of bytes in fixed overheads and percents of bytes in relative overheads. It is also an advantage for the reader when they can bypass decompression, including at least a buffer copy in most cases, after reading just one byte. The block-based table format in RocksDB follows this model with a single-byte compression marker, and at least after https://github.com/facebook/rocksdb/pull/13797 so does CompressedSecondaryCache. (Notably, the blob file format DOES NOT. This is left to follow-up work.) In particular, Compressor::CompressBlock now takes in a fixed size buffer for output rather than a `std::string*`. CompressBlock itself rejects the compression if the output would not fit in the provided buffer. This also works well with `max_compressed_bytes_per_kb` option to reject compression even sooner if its ratio is insufficient (implemented in this change). In the future we might use this functionality to reduce a buffer copy (in many cases) into the WritableFileWriter buffer of the block based table builder. This is a large change because we needed to (or were compelled to) * Update all the existing callers of CompressBlock, sometimes with substantial changes. This includes introducing GrowableBuffer to reuse between calls rather than std::string, which (at least in C++17) requires zeroing out data when allocating/growing a buffer. * Re-implement built-in Compressors (V2; V1 is obsolete) to efficiently implement the new version of the API, no longer wrapping the `OLD_CompressData()` function. The new compressors appropriately leverage the CompressBlock virtual call required for the customization interface and no rely on `switch` on compression type for each block. The implementations are largely adaptations of the old implementations, except * LZ4 and LZ4HC are notably upgraded to take advantage of WorkingArea (see performance tests). And for simplicity in the new implementation, we are dropping support for some super old versions of the library. * Getting snappy to work with limited-size output buffer required using the Sink/Source interfaces, which appear to be well supported for a long time and efficient (see performance tests). * Replace awkward old CompressionManager::GetDecompressorForCompressor with Compressor::GetOptimizedDecompressor (which is optional to implement) * Small behavior change where we treat lack of support for compression closer to not configuring compression, such as incompatibility with block_align. This is motivated by giving CompressionManager the freedom of determining when compression can be excluded for an entire file despite the configured "compression" type, and thus only surfacing actual incompatibilities not hypothetical ones that might be irrelevant to the CompressionManager (or build configuration). Unit tests in `table_test` and `compact_files_test` required update. * Some lingering clean up of CompressedSecondaryCache and a re-optimization made possible by compressing into an existing buffer. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13805 Test Plan: for correctness, existing tests ## Performance Test As I generally only modified compression paths, I'm using a db_bench write benchmark, with before & after configurations running at the same time. vc=1 means verify_compression=1 ``` USE_CLANG=1 DEBUG_LEVEL=0 LIB_MODE=static make -j100 db_bench SUFFIX=`tty | sed 's|/|_|g'`; for CT in zlib bzip2 none snappy zstd lz4 lz4hc none snappy zstd lz4 bzip2; do for VC in 0 1; do echo "$CT vc=$VC"; (for I in `seq 1 20`; do BIN=/dev/shm/dbbench${SUFFIX}.bin; rm -f $BIN; cp db_bench $BIN; $BIN -db=/dev/shm/dbbench$SUFFIX --benchmarks=fillseq -num=10000000 -compaction_style=2 -fifo_compaction_max_table_files_size_mb=1000 -fifo_compaction_allow_compaction=0 -disable_wal -write_buffer_size=12000000 -format_version=7 -compression_type=$CT -verify_compression=$VC 2>&1 | grep micros/op; done) | awk '{n++; sum += $5;} END { print int(sum / n); }'; done; done ``` zlib vc=0 524198 -> 524904 (+0.1%) zlib vc=1 430521 -> 430699 (+0.0%) bzip2 vc=0 61841 -> 60835 (-1.6%) bzip2 vc=1 49232 -> 48734 (-1.0%) none vc=0 1802375 -> 1906227 (+5.8%) none vc=1 1837181 -> 1950308 (+6.2%) snappy vc=0 1783266 -> 1901461 (+6.6%) snappy vc=1 1799703 -> 1879660 (+4.4%) zstd vc=0 1216779 -> 1230507 (+1.1%) zstd vc=1 996370 -> 1015415 (+1.9%) lz4 vc=0 1801473 -> 1943095 (+7.9%) lz4 vc=1 1799155 -> 1935242 (+7.6%) lz4hc vc=0 349719 -> 1126909 (+222.2%) lz4hc vc=1 348099 -> 1108933 (+218.6%) (Repeating the most important ones) none vc=0 1816878 -> 1952221 (+7.4%) none vc=1 1813736 -> 1904622 (+5.0%) snappy vc=0 1794816 -> 1875062 (+4.5%) snappy vc=1 1789363 -> 1873771 (+4.7%) zstd vc=0 1202592 -> 1225164 (+1.9%) zstd vc=1 994322 -> 1016688 (+2.2%) lz4 vc=0 1786959 -> 1971518 (+10.3%) lz4 vc=1 1829483 -> 1935871 (+5.8%) I confirmed manually that the new WorkingArea for LZ4HC makes the huge difference on that one, but not as much difference for LZ4, presumably because LZ4HC uses much larger buffers/structures/whatever for better compression ratios. Reviewed By: hx235 Differential Revision: D79111736 Pulled By: pdillinger fbshipit-source-id: 1ce1b14af9f15365f1b6da49906b5073a8cecc14 |
||
|
Peter Dillinger
|
60a0172096 |
Compression API clarifcations/minor fixes (#13775)
Summary: * A number of comments clarifying contracts, etc. * Make ReleaseWorkingArea public instead of protected because there are some limited cases where a wrapper implementation might want to call it directly * Check non-empty dictionary precondition on MaybeCloneForDict * Expand testing of wrapped WorkingAreas * Random documentation improvement in block_builder.cc Pull Request resolved: https://github.com/facebook/rocksdb/pull/13775 Test Plan: existing and expanded tests and assertions Reviewed By: hx235 Differential Revision: D78304550 Pulled By: pdillinger fbshipit-source-id: e5f064e8405a5a49be123ee13145cb3626bbbfbf |
||
|
Peter Dillinger
|
78c83ac1ec |
Publish/support format_version=7, related enhancements (#13713)
Summary: * Make new format_version=7 a supported setting. * Fix a bug in compressed_secondary_cache.cc that is newly exercised by custom compression types and showing up in crash test with tiered secondary cache * Small change to handling of disabled compression in fv=7: use empty compression manager compatibility name. * Get rid of GetDefaultBuiltinCompressionManager() in public API because it could cause unexpected+unsafe schema change on a user's CompressionManager if built upon the default built-in manager and we add a new built-in schema. Now must be referenced by explicit compression schema version in the public API. (That notion was already exposed in compressed secondary cache API, for better or worse.) * Improve some error messages for compression misconfiguration * Improve testing with ObjectLibrary and CompressionManagers * Improve testing of compression_name table property in BlockBasedTableTest.BlockBasedTableProperties2 * Improve some comments Pull Request resolved: https://github.com/facebook/rocksdb/pull/13713 Test Plan: existing and updated tests. Notably, the crash test has already been running with (unpublished) format_version=7 Reviewed By: mszeszko-meta, hx235 Differential Revision: D77035482 Pulled By: pdillinger fbshipit-source-id: 95278de8734a79706a22361bff2184b1edb230ca |
||
|
Peter Dillinger
|
fdc2970d37 |
Connect custom compression to crash test and ObjectLibrary (#13710)
Summary: Some pieces of follow-up to https://github.com/facebook/rocksdb/issues/13659. _Recommend hiding whitespace for review_ * Add support for instantiating CompressionManagers through CreateFromString/ObjectLibrary. * Pull CompressorCustomAlg and DecompressorCustomAlg out of db_test2, refactor/improvement them a bit, and put them in testutil.h for sharing with db_stress. Switched it from being built on snappy to being built on lz4 so that it can properly test dictionary compression. * Add a custom compression manager for db_stress that uses these, and add to crash test. This depends on the ObjectLibrary stuff because some invocations of db_stress will not be configured with the custom compression manager but will need to access it to read some existing SST files. * Remove some pieces where the concern of setting compression=kZSTD for compatibility purposes had leaked into configuring some tests and compression managers. After https://github.com/facebook/rocksdb/issues/13659 this compatibility concern is contained in the SST building code. * Fix BuiltinDecompressorV2SnappyOnly hiding the (ignored) compression dictionary. SST read logic expects the serialized dictionary to be returned by the decompressor even if it's effectively ignored. Updated DBBlockCacheTest.CacheCompressionDict to cover this case. For follow-up: * Combine custom compression and mixed compression types in a file (not clean/easy without duplicating or majorly refactoring the mixed/random compressor) Pull Request resolved: https://github.com/facebook/rocksdb/pull/13710 Test Plan: unit tests updated Reviewed By: hx235 Differential Revision: D76928974 Pulled By: pdillinger fbshipit-source-id: 772cf9cb048d737699b0e2887c624fb64a68aa8c |
||
|
Peter Dillinger
|
9d490593d0 |
Preliminary support for custom compression algorithms (#13659)
Summary: This change builds on https://github.com/facebook/rocksdb/issues/13540 and https://github.com/facebook/rocksdb/issues/13626 in allowing a CompressionManager / Compressor / Decompressor to use a custom compression algorithm, with a distinct CompressionType. For background, review the API comments on CompressionManager and its CompatibilityName() function. Highlights: * Reserve and name 127 new CompressionTypes that can be used for custom compression algorithms / schemas. In many or most cases I expect the enumerators such as `kCustomCompression8F` to be used in user code rather than casting between integers and CompressionTypes, as I expect the supported custom compression algorithms to be identifiable / enumerable at compile time. * When using these custom compression types, a CompressionManager must use a CompatibilityName() other than the built-in one AND new format_version=7 (see below). * When building new SST files, track the full set of CompressionTypes actually used (usually just one aside from kNoCompression), using our efficient bitset SmallEnumSet, which supports fast iteration over the bits set to 1. Ideally, to support mixed or non-mixed compression algorithms in a file as efficiently as possible, we would know the set of CompressionTypes as SST file open time. * New schema for `TableProperties::compression_name` in format_version=7 to represent the CompressionManager's CompatibilityName(), the set of CompressionTypes used, and potentially more in the future, while keeping the data relatively human-readable. * It would be possible to do this without a new format_version, but then the only way to ensure incompatible versions fail is with an unsupported CompressionType tag, not with a compression_name property. Therefore, (a) I prefer not to put something misleading in the `compression_name` property (a built-in compression name) when there is nuance because of a CompressionManager, and (b) I prefer better, more consistent error messages that refer to either format_version or the CompressionManager's CompatibilityName(), rather than an unrecognized custom CompressionType value (which could have come from various CompressionManagers). * The current configured CompressionManager is passed in to TableReaders so that it (or one it knows about) can be used if it matches the CompatibilityName() used for compression in the SST file. Until the connection with ObjectRegistry is implemented, the only way to read files generated with a particular CompressionManager using custom compression algorithms is to configure it (or a known relative; see FindCompatibleCompressionManager()) in the ColumnFamilyOptions. * Optimized snappy compression with BuiltinDecompressorV2SnappyOnly, to offset some small added overheads with the new tracking. This is essentially an early part of the planned refactoring that will get rid of the old internal compression APIs. * Another small optimization in eliminating an unnecessary key copy in flush (builder.cc). * Fix some handling of named CompressionManagers in CompressionManager::CreateFromString() (problem seen in https://github.com/facebook/rocksdb/issues/13647) Smaller things: * Adds Name() and GetId() functions to Compressor for debugging/logging purposes. (Compressor and Decompressor are not expected to be Customizable because they are only instantiated by a CompressionManager.) * When using an explicit compression_manager, the GetId() of the CompressionManager and the Compressor used to build the file are stored as bonus entries in the compression_options table property. This table property is not parsed anywhere, so it is currently for human reading, but still could be parsed with the new underscore-prefixed bonus entries. IMHO, this is preferable to additional table properties, which would increase memory fragmentation in the TableProperties objects and likely take slightly more CPU on SST open and slightly more storage. * ReleaseWorkingArea() function from protected to public to make wrappers work, because of a quirk in C++ (vs. Java) in which you cannot access protected members of another instance of the same class (sigh) * Added `CompressionManager:: SupportsCompressionType()` for early options sanity checking. Follow-up before release: * Make format_version=7 official / supported * Stress test coverage Sooner than later: * Update tests for RoundRobinManager and SimpleMixedCompressionManager to take advantage of e.g. set of compression types in compression_name property * ObjectRegistry stuff * Refactor away old internal compression APIs Pull Request resolved: https://github.com/facebook/rocksdb/pull/13659 Test Plan: Basic unit test added. ## Performance ### SST write performance ``` SUFFIX=`tty | sed 's|/|_|g'`; for ARGS in "-compression_type=none" "-compression_type=snappy" "-compression_type=zstd" "-compression_type=snappy -verify_compression=1" "-compression_type=zstd -verify_compression=1" "-compression_type=zstd -compression_max_dict_bytes=8180"; do echo $ARGS; (for I in `seq 1 20`; do BIN=/dev/shm/dbbench${SUFFIX}.bin; rm -f $BIN; cp db_bench $BIN; $BIN -db=/dev/shm/dbbench$SUFFIX --benchmarks=fillseq -num=10000000 -compaction_style=2 -fifo_compaction_max_table_files_size_mb=1000 -fifo_compaction_allow_compaction=0 -disable_wal -write_buffer_size=12000000 -format_version=7 $ARGS 2>&1 | grep micros/op; done) | awk '{n++; sum += $5;} END { print int(sum / n); }'; done ``` Ops/sec, Before -> After, both fv=6: -compression_type=none 1894386 -> 1858403 (-2.0%) -compression_type=snappy 1859131 -> 1807469 (-2.8%) -compression_type=zstd 1191428 -> 1214374 (+1.9%) -compression_type=snappy -verify_compression=1 1861819 -> 1858342 (+0.2%) -compression_type=zstd -verify_compression=1 979435 -> 995870 (+1.6%) -compression_type=zstd -compression_max_dict_bytes=8180 905349 -> 940563 (+3.9%) Ops/sec, Before fv=6 -> After fv=7: -compression_type=none 1879365 -> 1836159 (-2.3%) -compression_type=snappy 1865460 -> 1830916 (-1.9%) -compression_type=zstd 1191428 -> 1210260 (+1.6%) -compression_type=snappy -verify_compression=1 1866756 -> 1818989 (-2.6%) -compression_type=zstd -verify_compression=1 982640 -> 997129 (+1.5%) -compression_type=zstd -compression_max_dict_bytes=8180 912608 -> 937248 (+2.7%) ### SST read performance Create DBs ``` for COMP in none snappy zstd; do echo $ARGS; ./db_bench -db=/dev/shm/dbbench-7-$COMP --benchmarks=fillseq,flush -num=10000000 -compaction_style=2 -fifo_compaction_max_table_files_size_mb=1000 -fifo_compaction_allow_compaction=0 -disable_wal -write_buffer_size=12000000 -compression_type=$COMP -format_version=7; done ``` And test ``` for COMP in none snappy zstd none; do echo $COMP; (for I in `seq 1 8`; do ./db_bench -readonly -db=/dev/shm/dbbench -7-$COMP --benchmarks=readrandom -num=10000000 -duration=20 -threads=8 2>&1 | grep micros/op; done ) | awk '{n++; sum += $5;} END { print int(sum / n); }'; done ``` Ops/sec, Before -> After (both fv=6) none 1491732 -> 1500209 (+0.6%) snappy 1157216 -> 1169202 (+1.0%) zstd 695414 -> 703719 (+1.2%) none (again) 1491787 -> 1528789 (+2.4%) Ops/sec, Before fv=6 -> After fv=7: none 1492278 -> 1508668 (+1.1%) snappy 1140769 -> 1152613 (+1.0%) zstd 696437 -> 696511 (+0.0%) none (again) 1500585 -> 1512037 (+0.7%) Overall, I think we can take the read CPU improvement in exchange for the hit (in some cases) on background write CPU Reviewed By: hx235 Differential Revision: D76520739 Pulled By: pdillinger fbshipit-source-id: e73bd72502ff85c8779cba313f26f7d1fd50be3a |
||
Sujit Maharjan
|
fccc881894 |
Implement MixedCompressor that Round robins on compression algorithm (#13647)
Summary: **Summary** This pull request introduces a mixed compressor, RoundRobinManager and RoundRobinCompressor, which selects algorithms in a loop. This implementation replaces the current hacky approach to round-robin compression in BuiltInCompressorV2. Additionally, it configures RocksDB to optionally utilize this customized compressor in the db stress test. **Testing** Testing was performed by verifying the stdout output from both RoundRobinCompressor and BuiltInCompressorV2. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13647 Reviewed By: pdillinger Differential Revision: D75921997 Pulled By: shubhajeet fbshipit-source-id: 8f42ac46f08ba982b2cd70241bd7dc13ff5a1225 |
||
|
Peter Dillinger
|
0c533e61bc |
Fix XPRESS compression and enable in CI (#13649)
Summary: Somehow this was previously not being tested in our Windows CI jobs so was accidentally broken in https://github.com/facebook/rocksdb/pull/13540 This fix will need to be backported to 10.3. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13649 Test Plan: CI Reviewed By: hx235 Differential Revision: D75655418 Pulled By: pdillinger fbshipit-source-id: a56bb213270904a1b7a13b905c2cc1919116df1c |
||
Hui Xiao
|
6efdbe85e3 |
Detailed comment about setting ZSTD compression type for mixed compression (#13653)
Summary: **Context/Summary:** .... to clarify things more explicitly Pull Request resolved: https://github.com/facebook/rocksdb/pull/13653 Test Plan: no code change Reviewed By: pdillinger Differential Revision: D75655419 Pulled By: hx235 fbshipit-source-id: d9ee2e669df15aacf7996a3122c382412b23229e |
||
|
Peter Dillinger
|
8dc3d77b59 |
Experimental, preliminary support for custom CompressionManager (#13626)
Summary: This exposes CompressionManager and related classes to the public API and adds `ColumnFamilyOptions::compression_manager` for tying a custom compression strategy to a column family. At the moment, this does not support custom/pluggable compression algorithms, just custom strategies around the built-in algorithms, e.g. which compression to use when and where. A large part of the change is moving code from internal compression.h to a new public header advanced_compression.h, with some minor changes: * `Decompressor::ExtractUncompressedSize()` is out-of-lined * CompressionManager inherits Customizable and some related changes to members of CompressionManager are made. (Core functionality of CompressionManager is unchanged.) This depends on a smart pointer I'm calling `ManagedPtr` which I'm adding to data_structure.h. Additionally, advanced_compression.h gets CompressorWrapper and CompressionManagerWrapper as building blocks for overriding aspects of compression strategy while leveraging existing compression algorithms / schemas. Some pieces needed to support the `compression_manager` option and rudimentary Customizable implementation are included. More work will be needed to make this general and well-behaved (see e.g. https://github.com/facebook/rocksdb/issues/8641; I still hit inscrutible problems every time I touch Customizable). I'll add a release note for the experimental feature once pluggable compression algorithms and more of the Customizable things are working. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13626 Test Plan: Added a unit test demonstrating how a custom compressor can "bypass" or "reject" compressions. Expected next follow-up (probably someone else): use a custom CompressionManager/Compressor to replace the internal hack for testing mixed compressions. Reviewed By: hx235 Differential Revision: D75028850 Pulled By: pdillinger fbshipit-source-id: 8565bb8ba4b5fa923b1e29e76b4f7bb4faa42381 |
||
|
Peter Dillinger
|
77af042413 |
Fix some compression-related assertion failures (#13621)
Summary: showing up in the crash test after https://github.com/facebook/rocksdb/issues/13540 * For an assertion `dict_samples.sample_data.size() <= opts_.max_dict_bytes` we needed to ensure that `zstd_max_train_bytes` only takes effect with kZSTD compression. * For an assertion with `r->table_options.verify_compression == (verify_decomp != nullptr)` we needed to ensure that `data_block_verify_decompressor` is set even when dictionary compression is attempted but not used. * Noticed along the way: finish an optimization in `CompressAndVerifyBlock` that was incomplete. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13621 Test Plan: Both failures were reproducible with hard-coding of some crash test params, and now not getting a failure. ``` --compression_type=zstd --compression_max_dict_bytes=16384 --compression_zstd_max_train_bytes=65536 --compression_max_dict_buffer_bytes=131071 --compression_use_zstd_dict_trainer=1 ``` Write performance test like in https://github.com/facebook/rocksdb/issues/13540 shows essentially no change, maybe slightly faster (+0.4%) with verify_compression. Reviewed By: virajthakur Differential Revision: D74939103 Pulled By: pdillinger fbshipit-source-id: 8bac8891bc08e1356eff52cc524e5bb409b0f86f |
||
|
Peter Dillinger
|
7c9b580681 |
Big refactor for preliminary custom compression API (#13540)
Summary: Adds new classes etc. in internal compression.h that are intended to become public APIs for supporting custom/pluggable compression. Some steps remain to allow for pluggable compression and to remove a lot of legacy code (e.g. now called `OLD_CompressData` and `OLD_UncompressData`), but this change refactors the key integration points of SST building and reading and compressed secondary cache over to the new APIs. Compared with the proposed https://github.com/facebook/rocksdb/issues/7650, this fixes a number of issues including * Making a clean divide between public and internal APIs (currently just indicated with comments) * Enough generality that built-in compressions generally fit into the framework rather than needing special treatment * Avoid exposing obnoxious idioms like `compress_format_version` to the user. * Enough generality that a compressor mixing algorithms/strategies from other compressors is pretty well supported without an extra schema layer * Explicit thread-safety contracts (carefully considered) * Contract details around schema compatibility and extension with code changes (more detail in next PR) * Customizable "working areas" (e.g. for ZSTD "context") * Decompression into an arbitrary memory location (rather than involving the decompressor in memory allocation; should facilitate reducing number of objects in block cache) Pull Request resolved: https://github.com/facebook/rocksdb/pull/13540 Test Plan: This is currently an internal refactor. More testing will come when the new API is migrated to the public API. A test in db_block_cache_test is updated to meaningfully cover a case (cache warming compression dictionary block) that was previously only covered in the crash test. SST write performance test, like https://github.com/facebook/rocksdb/issues/13583. Compile with CLANG, run before & after simultaneously: ``` SUFFIX=`tty | sed 's|/|_|g'`; for ARGS in "-compression_parallel_threads=1 -compression_type=none" "-compression_parallel_threads=1 -compression_type=snappy" "-compression_parallel_threads=1 -compression_type=zstd" "-compression_parallel_threads=1 -compression_type=zstd -verify_compression=1" "-compression_parallel_threads=1 -compression_type=zstd -compression_max_dict_bytes=8180" "-compression_parallel_threads=4 -compression_type=snappy"; do echo $ARGS; (for I in `seq 1 20`; do ./db_bench -db=/dev/shm/dbbench$SUFFIX --benchmarks=fillseq -num=10000000 -compaction_style=2 -fifo_compaction_max_table_files_size_mb=1000 -fifo_compaction_allow_compaction=0 -disable_wal -write_buffer_size=12000000 $ARGS 2>&1 | grep micros/op; done) | awk '{n++; sum += $5;} END { print int(sum / n); }'; done ``` Before (this PR and with https://github.com/facebook/rocksdb/issues/13583 reverted): -compression_parallel_threads=1 -compression_type=none 1908372 -compression_parallel_threads=1 -compression_type=snappy 1926093 -compression_parallel_threads=1 -compression_type=zstd 1208259 -compression_parallel_threads=1 -compression_type=zstd -verify_compression=1 997583 -compression_parallel_threads=1 -compression_type=zstd -compression_max_dict_bytes=8180 934246 -compression_parallel_threads=4 -compression_type=snappy 1644849 After: -compression_parallel_threads=1 -compression_type=none 1956054 (+2.5%) -compression_parallel_threads=1 -compression_type=snappy 1911433 (-0.8%) -compression_parallel_threads=1 -compression_type=zstd 1205668 (-0.3%) -compression_parallel_threads=1 -compression_type=zstd -verify_compression=1 999263 (+0.2%) -compression_parallel_threads=1 -compression_type=zstd -compression_max_dict_bytes=8180 934322 (+0.0%) -compression_parallel_threads=4 -compression_type=snappy 1642519 (-0.2%) Pretty neutral change(s) overall. SST read performance test (related to https://github.com/facebook/rocksdb/issues/13583). Set up: ``` for COMP in none snappy zstd; do echo $ARGS; ./db_bench -db=/dev/shm/dbbench-$COMP --benchmarks=fillseq,flush -num=10000000 -compaction_style=2 -fifo_compaction_max_table_files_size_mb=1000 -fifo_compaction_allow_compaction=0 -disable_wal -write_buffer_size=12000000 -compression_type=$COMP; done ``` Test (compile with CLANG, run before & after simultaneously): ``` for COMP in none snappy zstd; do echo $COMP; (for I in `seq 1 5`; do ./db_bench -readonly -db=/dev/shm/dbbench-$COMP --benchmarks=readrandom -num=10000000 -duration=20 -threads=8 2>&1 | grep micros/op; done) | awk '{n++; sum += $5;} END { print int(sum / n); }'; done ``` Before (this PR and with https://github.com/facebook/rocksdb/issues/13583 reverted): none 1495646 snappy 1172443 zstd 706036 zstd (after constructing with -compression_max_dict_bytes=8180) 656182 After: none 1494981 (-0.0%) snappy 1171846 (-0.1%) zstd 696363 (-1.4%) zstd (after constructing with -compression_max_dict_bytes=8180) 667585 (+1.7%) Pretty neutral. Reviewed By: hx235 Differential Revision: D74626863 Pulled By: pdillinger fbshipit-source-id: dc8ff3178da9b4eaa7c16aa1bb910c872afaf14a |
||
|
Peter Dillinger
|
7f271a3fa8 |
Require ZSTD >= 1.4.0, greatly simplifying related code (#13362)
Summary: Leading up to some compression code refactoring, we have a bit of an ifdef nightmare in compression.h relating to zstd support. With the major release RocksDB 10.0.0 coming up, it is a good time to clean up much of this tech debt by requiring zstd >= 1.4.0 (April 2019) if building RocksDB with ZSTD support. For example, Ubuntu 20, the first LTS version to properly support C++17 in its built-in gcc, comes with zstd version 1.4.4. This should not be a significant limitation. * Almost all of the `ZSTD_VERSION_NUMBER` checks are simplified to just `ZSTD`, though * `ROCKSDB_ZSTD_DDICT` still needs to be separate because of dependency on `ZSTD_STATIC_LINKING_ONLY` (added to fbcode_config_platform010.sh by the way) * Similar for ZDICT_finalizeDictionary, which is only generally available in >= 1.4.5 * Eliminate deprecated `kZSTDNotFinalCompression` * Reduce some cases of unnecessary copying definitions across `#if` branches (e.g. `ZSTDUncompressCachedData`) Pull Request resolved: https://github.com/facebook/rocksdb/pull/13362 Test Plan: minor unit test updates. `make check` on several build variants with/without zstd and with/without `ZSTD_STATIC_LINKING_ONLY` Also deflaked DBTest.DynamicLevelCompressionPerLevel which was flaky before this change but failed once in CI Reviewed By: cbi42 Differential Revision: D69129453 Pulled By: pdillinger fbshipit-source-id: ef0cbf9f0fea4e7684fa0999320aa170cfbec233 |
||
Changyu Bi
|
c2aad555c3 |
Add CompressionOptions::checksum for enabling ZSTD checksum (#11666)
Summary:
Optionally enable zstd checksum flag (
|
||
anand76
|
77b61abc7b |
Fix bug in WAL streaming uncompression (#11198)
Summary: Fix a bug in the calculation of the input buffer address/offset in log_reader.cc. The bug is when consecutive fragments of a compressed record are located at the same offset in the log reader buffer, the second fragment input buffer is treated as a leftover from the previous input buffer. As a result, the offset in the `ZSTD_inBuffer` is not reset. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11198 Test Plan: Add a unit test in log_test.cc that fails without the fix and passes with it. Reviewed By: ajkr, cbi42 Differential Revision: D43102692 Pulled By: anand1976 fbshipit-source-id: aa2648f4802c33991b76a3233c5a58d4cc9e77fd |
||
Siddhartha Roychowdhury
|
fec4403ff1 |
Integrate WAL compression into log reader/writer. (#9642)
Summary: Integrate the streaming compress/uncompress API into WAL compression. The streaming compression object is stored in the log_writer along with a reusable output buffer to store the compressed buffer(s). The streaming uncompress object is stored in the log_reader along with a reusable output buffer to store the uncompressed buffer(s). Pull Request resolved: https://github.com/facebook/rocksdb/pull/9642 Test Plan: Added unit tests to verify different scenarios - large buffers, split compressed buffers, etc. Future optimizations: The overhead for small records is quite high, so it makes sense to compress only buffers above a certain threshold and use a separate record type to indicate that those records are compressed. Reviewed By: anand1976 Differential Revision: D34709167 Pulled By: sidroyc fbshipit-source-id: a37a3cd1301adff6152fb3fcd23726106af07dd4 |
||
|
Siddhartha Roychowdhury
|
21345d2823 |
Streaming Compression API for WAL compression. (#9619)
Summary: Implement a streaming compression API (compress/uncompress) to use for WAL compression. The log_writer would use the compress class/API to compress a record before writing it out in chunks. The log_reader would use the uncompress class/API to uncompress the chunks and combine into a single record. Added unit test to verify the API for different sizes/compression types. Pull Request resolved: https://github.com/facebook/rocksdb/pull/9619 Test Plan: make -j24 check Reviewed By: anand1976 Differential Revision: D34437346 Pulled By: sidroyc fbshipit-source-id: b180569ad2ddcf3106380f8758b556cc0ad18382 |