Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,9 @@ The following patterns emerged as frequent sources of review feedback:
### Adding new option
Refer to claude_md/add_option.md

### Removing deprecated option
Refer to claude_md/remove_option.md
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this indirection keep from eating into the context budget when it's not relevant? I hope so because it's a lot of text.


### Metrics
* When adding a new feature, evaluate whether there is opportunity to add
metrics. Try to avoid causing performance regression on hot path when adding
Expand Down
47 changes: 47 additions & 0 deletions claude_md/remove_option.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Removing Deprecated Options from RocksDB

This document provides guidance on the proper process for removing deprecated options from RocksDB's public API. **Improper removal can break backwards compatibility with old option files.**

---

## Critical Rule: Never Remove from type_info Structures

**IMPORTANT:** When removing a deprecated option, you must **keep** the option entry in the `type_info` structures with `OptionVerificationType::kDeprecated`.

This is because:
1. Old option files (e.g., `OPTIONS-xxx` files) generated by previous RocksDB versions may still contain the deprecated option
2. When RocksDB reads these old option files, `GetColumnFamilyOptionsFromMap()` or `GetDBOptionsFromMap()` will fail with `InvalidArgument` if the option is not recognized
3. Using `kDeprecated` allows the option to be parsed and silently ignored

**Keep** the entry in the appropriate type_info map with `kDeprecated`. Refer to existing examples:
- For CF options: see `soft_rate_limit` in `options/cf_options.cc`
- For DB options: see `new_table_reader_for_compaction_inputs` in `options/db_options.cc`

---

## Checklist for Removing Deprecated Options

### Remove from these files:
- [ ] Public header (`include/rocksdb/advanced_options.h`, `include/rocksdb/options.h`)
- [ ] Internal options struct (`options/cf_options.h` or `options/db_options.h`)
- [ ] `options/options_helper.cc` - `UpdateColumnFamilyOptions()` if mutable CF option
- [ ] `options/options_settable_test.cc` - test strings
- [ ] `test_util/testutil.cc` - `RandomInitCFOptions()` or `RandomInitDBOptions()`
- [ ] `db_stress_tool/db_stress_common.h` - DECLARE macro
- [ ] `db_stress_tool/db_stress_gflags.cc` - DEFINE macro
- [ ] `db_stress_tool/db_stress_test_base.cc` - flag usage
- [ ] `tools/db_bench_tool.cc` - flag definition and usage
- [ ] `tools/db_crashtest.py` - randomized value entry
- [ ] `tools/benchmark.sh` - if referenced
- [ ] `examples/rocksdb_option_file_example.ini` - if referenced
- [ ] `db/c.cc` - C API implementation
- [ ] `include/rocksdb/c.h` - C API declaration
- [ ] Java files (`java/src/main/java/org/rocksdb/*.java`) - if present
- [ ] `include/rocksdb/convenience.h` - update examples if they reference the option

### Keep in these files:
- [ ] **KEEP** entry in type_info (`options/cf_options.cc` or `options/db_options.cc`) with `OptionVerificationType::kDeprecated`
- [ ] **KEEP** or add test in `options/options_test.cc` `OptionsOldApiTest::GetOptionsFromMapTest` for loading old option files

### Documentation:
- [ ] Add release note to `HISTORY.md` and `unreleased_history/`
Loading