-
Notifications
You must be signed in to change notification settings - Fork 131
feat: Add paranoid file checks in RocksDB #3511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -805,6 +805,60 @@ fn open_rocksdb<P: AsRef<Path>>(path: P, opt_cfs: &[&str]) -> Arc<RocksDB> { | |
| open_cf(path, None, MetricConf::default(), opt_cfs).expect("failed to open rocksdb") | ||
| } | ||
|
|
||
| struct EnvVarGuard { | ||
| name: &'static str, | ||
| previous: Option<String>, | ||
| } | ||
|
|
||
| impl EnvVarGuard { | ||
| fn set(name: &'static str, value: &str) -> Self { | ||
| let previous = std::env::var(name).ok(); | ||
| // SAFETY: typed-store RocksDB tests use a process-wide mutex to serialize env mutation. | ||
| unsafe { | ||
| std::env::set_var(name, value); | ||
| } | ||
|
Comment on lines
+816
to
+819
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When this test runs in parallel on Unix, the custom mutex does not make Useful? React with 👍 / 👎. |
||
| Self { name, previous } | ||
| } | ||
| } | ||
|
|
||
| impl Drop for EnvVarGuard { | ||
| fn drop(&mut self) { | ||
| // SAFETY: the guard is held while the same process-wide test mutex is held. | ||
| unsafe { | ||
| if let Some(value) = &self.previous { | ||
| std::env::set_var(self.name, value); | ||
| } else { | ||
| std::env::remove_var(self.name); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn paranoid_file_checks_env_applies_to_opened_and_created_column_families() { | ||
| let _lock = global_test_lock(); | ||
| let _env = EnvVarGuard::set(ENV_VAR_DB_PARANOID_FILE_CHECKS, "true"); | ||
| let path = temp_dir(); | ||
| let cf_options = rocksdb::Options::default(); | ||
|
|
||
| let rocks = open_cf_opts( | ||
| &path, | ||
| None, | ||
| MetricConf::default(), | ||
| &[("existing_cf", cf_options.clone())], | ||
| ) | ||
| .expect("failed to open rocksdb with env options"); | ||
|
|
||
| rocks | ||
| .create_cf("created_cf", &cf_options) | ||
| .expect("failed to create column family with env options"); | ||
|
|
||
| assert_eq!( | ||
| read_bool_from_env(ENV_VAR_DB_PARANOID_FILE_CHECKS), | ||
| Some(true) | ||
| ); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_sampling() { | ||
| let sampling_interval = SamplingInterval::new(Duration::ZERO, 10); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When reopening a database after an unclean shutdown,
open_cf_descriptorscan replay the WAL and flush recovery SSTs before it returns. Becauseparanoid_file_checksis applied only afterward, those SST writes bypass the requested checksum verification. Apply the environment override to each column family'sOptionsbefore callingopen_cf_descriptorsso recovery and startup writes are covered too.Useful? React with 👍 / 👎.