mirror of
https://github.com/facebook/rocksdb.git
synced 2026-11-06 19:12:36 -05:00
zaidoon
8fd33c2fac
Summary:
**Why:** The `IndexFactory` series (https://github.com/meta/rocksdb/issues/14963, https://github.com/meta/rocksdb/issues/14965, https://github.com/meta/rocksdb/issues/14966) added five index modes and a per-read selector, but the stress and benchmark tools could reach only part of that surface. Its public API also shipped with no release note, and the `CompressionOptions::parallel_threads` comment still said a custom index forces single-threaded compression.
**Previous behavior:**
- `db_stress` had only `--use_udi_as_primary_index`, which selects `kStandardDefault` or `kCustomDefault`. `kCustomOnly`, `kStandardRequired`, and the `kBuiltin`/`kPreferCustom` read selectors were never stress tested. Reads chose the trie through the legacy `ReadOptions::table_index_factory` pointer, and the two read paths disagreed: after a reopen, the pointer was also set in primary mode.
- `db_bench --use_trie_index` could only build the trie as the secondary index, so the trie could not be benchmarked as the default read index or as the only index.
**New behavior:**
- `db_stress` and `db_bench` take `--index_mode` (0-4, defaulting to the library default `kStandardDefault`) and `--read_index` (-1 = auto, or `kDefault`/`kBuiltin`/`kPreferCustom`). Auto keeps today's behavior of reading through the trie when it is the secondary index. Both tools reject combinations that cannot work: modes 2-4 or a non-default selector without `--use_trie_index`, and `kBuiltin` with `kCustomOnly`. `--use_udi_as_primary_index` is removed from `db_stress`, and `--index_mode=2` replaces it.
- The crashtest draws `index_mode` once per DB together with `use_trie_index`, so every SST in a DB has matching table options, and draws `read_index` on every invocation. It disables only what cannot work in a mode:
- `kCustomDefault`/`kCustomOnly`: partitioned index and filters (rejected by option validation); backup, because the restored DB rebuilds options from strings that do not carry the factory; and the secondary instance, which main already disables for primary mode.
- `kCustomOnly`: `format_version` raised to 6, and `kBuiltin` reads dropped.
- After all other sanitizers, custom modes and selectors are reset when something (for example user-defined timestamps) has turned the trie off.
- New `docs/components/index_factory/` component doc, and two release notes: one for the `IndexFactory` API, one for parallel compression with a custom index.
## Changes since the previous revision
This branch now contains only https://github.com/meta/rocksdb/issues/14968's own work on top of main, which includes https://github.com/meta/rocksdb/issues/14966. I re-checked each piece for value and redundancy:
- **Dropped `--index_mode=-1` ("auto")** from both tools. With no factory, the default `kStandardDefault` already builds only the standard index, so auto behaved identically to the default and needed a helper in each tool.
- **Kept remote compaction on in the custom modes.** The previous revision disabled it because it was "not exercised yet", but main already runs primary mode with remote compaction, and remote workers rebuild the trie factory from its registered name. The crashtest runs below cover it in `kCustomOnly` and `kCustomDefault`.
- **Dropped disabling write faults for `cf_consistency`** in the custom modes. Main runs that combination in primary mode today, and the targeted `cf_consistency` + `kCustomOnly` + write-fault run below passed.
- **Kept backup and secondary on for `kStandardRequired`.** It has a full standard index, so a restored or secondary DB can always read it.
- **Release notes cut from eight to two.** Dropped one that duplicated main's existing user-defined-timestamp note, and one that described `kPreferCustom`'s fallback as a behavior change. Before https://github.com/meta/rocksdb/issues/14963, `table_index_factory` was only consulted for SSTs that had a custom index, so SSTs without one already used the standard index.
- Replaced eleven per-case crashtest tests with four table-driven ones, and dropped an unrelated `(void)argc` edit in `db_bench_tool_test.cc`.
## Safety and compatibility
- Tool defaults are unchanged: without `--use_trie_index`, both tools build only the standard index. With it, `db_bench` and `db_stress` still read through the trie as before.
- Removing `--use_udi_as_primary_index` from `db_stress` breaks any external command line that passes it. The in-tree crashtest is updated.
- The header and docs changes are comments only.
- The component doc notes known gaps rather than fixing them: `sst_dump`, and `ldb` without the DB's OPTIONS file, cannot open `kCustomOnly` files, and there is no Java API.
Pull Request resolved: https://github.com/meta/rocksdb/pull/14968
Test Plan:
All on this branch rebased onto main at a7ae6cd0a (macOS ARM64, Apple Clang, debug):
- `tools/db_crashtest_test.py`: 76 tests pass. Each of the four new tests fails when its sanitizer rule is removed: the `format_version` bump, the `kBuiltin` drop for `kCustomOnly`, the reset when the trie is off, and keeping remote compaction on (verified by re-adding the old disable).
- Flag validation: every invalid combination listed above exits with status 1 and a clear message in both tools.
- `db_bench` smoke runs: fillseq, compact, readrandom, and seekrandom with `--use_trie_index=1` in all 19 valid mode and selector combinations, with every key found.
- Blackbox crashtests (`db_crashtest.py blackbox`, 300 s, killed every 30 s, `db_stress` built with gflags). Runs A, B, and D predate the last `read_index` fix, so they always used auto. Each run completed 10 kill/restart cycles, and its final crash-recovery verification passed:
| Run | Index mode | What it adds | Result |
|---|---|---|---|
| A | `kCustomOnly` | Remote compaction in 4 of 11 runs, `format_version` 6-8, 1-16 compression threads | Passed |
| A2 | `kCustomOnly` | Write faults (1 in 128) in all 11 runs. Remote compaction in 7 runs: 289 remote jobs, none fell back to local. `read_index` auto, `kDefault`, `kPreferCustom` | Passed |
| D | `kCustomOnly` | `--cf_consistency` | Passed |
| D2 | `kCustomOnly` | `--cf_consistency` with write faults in all 11 runs. Remote compaction in 6 runs: 1,710 remote jobs; 18 fell back to local after the remote job failed (`db_stress` falls back only for injected faults or when configured to, and fails the run on any other remote error) | Passed |
| B | `kStandardRequired` | Remote compaction and backup in every run, write faults in 4 runs | Passed |
| B2 | `kStandardRequired` | Secondary instance in every run, 2 DBs, all four `read_index` values including `kBuiltin` | Passed |
| E | `kCustomDefault` | Remote compaction in 9 runs: 1,472 remote jobs, none fell back to local. All four `read_index` values | Passed |
- `make format-auto` and `make check-sources` pass.
Not run locally: `db_bench_tool_test` (this Mac's gflags setup pulls in a conflicting gtest; CI runs it), full `make check`, Linux, Windows, sanitizers, and a release build.
## Series
Builds on https://github.com/meta/rocksdb/issues/14963, https://github.com/meta/rocksdb/issues/14965, and https://github.com/meta/rocksdb/issues/14966, all on main. This is the last PR in the series.
Reviewed By: jaykorean
Differential Revision: D128807954
Pulled By: xingbowang
fbshipit-source-id: b4faa1e94959f63ea56fd8d69f64b1763c8aacce