Add option to verify block checksums of output files by jaykorean · Pull Request #14103 · facebook/rocksdb

jaykorean · 2025-11-05T19:29:24Z

Summary

For all compactions, RocksDB performs a lightweight sanity check on output SST files before installation (in CompactionJob::VerifyOutputFiles()). However, this lightweight check may not catch corruption that is small enough to allow the SST files to still be opened.

There is an existing feature, paranoid_file_check, which opens the SST file, iterates through all keys, and checks the hash of each key. While this provides the ultimate level of data integrity checking, it comes at a high computational cost.

In this PR, we introduce a new mutable CF option, verify_output_flags. The verify_output_flags is a bitmask enum that allows users to select various verification types, including block checksum verification, full key iteration, and file checksum verification (to be added in subsequent PRs). Note that the existing paranoid_file_check option is equivalent to a full key iteration check. Block-level checksum verification is much lighter than the full key iteration check.

Please note that the previously deprecated verify_checksums_in_compaction option (removed in version 5.3.0) was for verifying the checksum of input SST files. RocksDB continues to perform this verification for both local and remote compactions, and this behavior remains unchanged. In contrast, this PR focuses on verifying the output SST files.

To follow up

File-level Checksum verification for output SST files
Deprecate paranoid_file_checks option in favor of the new option
Add to stress test / db_bench

Test Plan

New Unit Test added. The corruption is both detected by paranoid_file_check and various types of verification set by this new option, verify_output_flags

./compaction_service_test --gtest_filter="*CompactionServiceTest.CorruptedOutput*"

meta-codesync · 2025-11-05T23:33:17Z

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

include/rocksdb/advanced_options.h

db/compaction/compaction_job.cc

pdillinger · 2025-11-06T21:07:50Z

include/rocksdb/advanced_options.h

+  // Default: 0 (kVerifyNone)
+  //
+  // Dynamically changeable through SetOptions() API
+  uint32_t verify_output_option = VerifyOutputOptions::kVerifyNone;


I believe it's standard to use the enum type as the type of the field even if it's a bitwise enum. But probably has to be mutable as a uint32_t, which is probably worth mentioning in the "dynamically changeable" part with the change of type.

pdillinger · 2025-11-06T21:08:40Z

include/rocksdb/advanced_options.h

+// Bitmask enum for verify output options during compaction.
+// This allows fine-grained control over what verification is performed
+// on compaction output files and when it's enabled.
+enum VerifyOutputOptions : uint32_t {


Avoid enum (without class) unless it's already scoped in something limited already. rocksdb:: is a big scope.

See https://softwareengineering.stackexchange.com/a/204566 for how to avoid excessive casting.

pdillinger · 2025-11-06T21:09:56Z

options/cf_options.cc

          OptionTypeFlags::kMutable}},
+        {"verify_output_option",
+         {offsetof(struct MutableCFOptions, verify_output_option),
+          OptionType::kInt32T, OptionVerificationType::kNormal,


pdillinger · 2025-11-06T21:12:12Z

db/compaction/compaction_job.cc

+        if (should_verify) {
+          if (verify_output_option &
+              VerifyOutputOptions::kVerifyBlockChecksum) {
+            s = table_reader_ptr->VerifyChecksum(


If we are also iterating, a lot of this work is redundant (but not filter blocks). Can we avoid it? Perhaps we can have a variant of VerifyChecksum that only verifies metadata blocks? (I'm not too concerned about double-verifying index blocks.)

pdillinger · 2025-11-06T21:21:07Z

include/rocksdb/advanced_options.h

+  // Default: 0 (kVerifyNone)
+  //
+  // Dynamically changeable through SetOptions() API
+  uint32_t verify_output_option = VerifyOutputOptions::kVerifyNone;


I feel like verify_output_flags and VerifyOutputFlags would be a better name. (Everything in an Options is an option.)

pdillinger · 2025-11-06T21:22:06Z

include/rocksdb/advanced_options.h

@@ -704,6 +724,13 @@ struct AdvancedColumnFamilyOptions {
  // Dynamically changeable through SetOptions() API
  bool paranoid_file_checks = false;


I think we should deprecate this option, talk about the equivalence in the new option, and how the two are reconciled until the removal of this one.

I agree that we should deprecate this option, but I'd like to do that as a follow up.

pdillinger · 2025-11-06T21:24:03Z

include/rocksdb/advanced_options.h

+
+  // First set of bits: type of verifications
+  kVerifyBlockChecksum = 1 << 0,  // Verify block checksums
+  kVerifyIteration = 1 << 1,      // Verify iteration (full key/value hashing)


I suggest more detail, like, "Verify iteration and full key/value hash matches one computed while generating the file."

jaykorean · 2025-11-07T15:39:54Z

db/compaction/compaction_service_test.cc

+        });
+    SyncPoint::GetInstance()->EnableProcessing();
+    const bool is_enabled_for_remote_compaction =
+        !!(verify_output_flags & VerifyOutputFlags::kEnableForRemoteCompaction);


Not sure if this !! is any better than doing != VerifyOutputFlags::kVerifyNone, but I just did it to make the line shorter...

It's fine ‼️

pdillinger

Thanks!

pdillinger · 2025-11-07T17:10:45Z

db/compaction/compaction_service_test.cc

+        });
+    SyncPoint::GetInstance()->EnableProcessing();
+    const bool is_enabled_for_remote_compaction =
+        !!(verify_output_flags & VerifyOutputFlags::kEnableForRemoteCompaction);


It's fine ‼️

pdillinger · 2025-11-07T17:16:57Z

include/rocksdb/advanced_options.h

+  kVerifyAll = 0xFFFFFFFF,
+};
+
+inline VerifyOutputFlags operator|(VerifyOutputFlags lhs,


I think ideally we would just see here something like DEFINE_OPERATORS_FOR_FLAGS_ENUM_CLASS(VerifyOutputFlags); and put the details in another header (data_structure.h?) to keep this one from getting bloated with boilerplate. Same with SizeApproximationFlags in db.h

Let me do this as a follow up.

pdillinger · 2025-11-07T17:22:55Z

db/compaction/compaction_job.cc

+              s = iter->status();
+            }
+            if (s.ok() && !validator.CompareValidator(output_file.validator)) {
+              s = Status::Corruption("Paranoid checksums do not match");


I know it's not yours, but I would prefer something like "Key-value checksum of compaction output doesn't match what was computed when written"

meta-codesync · 2025-11-07T22:25:12Z

@jaykorean merged this pull request in 5879f8b.

Summary: For all compactions, RocksDB performs a lightweight sanity check on output SST files before installation (in `CompactionJob::VerifyOutputFiles()`). However, this lightweight check may not catch corruption that is small enough to allow the SST files to still be opened. There is an existing feature, `paranoid_file_check`, which opens the SST file, iterates through all keys, and checks the hash of each key. While this provides the ultimate level of data integrity checking, it comes at a high computational cost. In this PR, we introduce a new mutable CF option, `verify_output_flags`. The `verify_output_flags` is a bitmask enum that allows users to select various verification types, including block checksum verification, full key iteration, and file checksum verification (to be added in subsequent PRs). Note that the existing `paranoid_file_check` option is equivalent to a full key iteration check. Block-level checksum verification is much lighter than the full key iteration check. Please note that the previously deprecated `verify_checksums_in_compaction` option (removed in version 5.3.0) was for verifying the checksum of **input SST files**. RocksDB continues to perform this verification for both local and remote compactions, and this behavior remains unchanged. In contrast, this PR focuses on verifying the **output SST files**. ## To follow up - File-level Checksum verification for output SST files - Deprecate `paranoid_file_checks` option in favor of the new option - Add to stress test / db_bench Pull Request resolved: #14103 Test Plan: New Unit Test added. The corruption is both detected by `paranoid_file_check` and various types of verification set by this new option, `verify_output_flags` ``` ./compaction_service_test --gtest_filter="*CompactionServiceTest.CorruptedOutput*" ``` Reviewed By: pdillinger Differential Revision: D86357924 Pulled By: jaykorean fbshipit-source-id: a9e04798f249c7e977231e179622a0830d6675fe

Summary: One of the follow ups from #14103. Users will have the option to verify file checksums for all compaction output files before they are installed. This feature helps prevent corrupted SST files from being added to the LSM tree. Pull Request resolved: #14433 Test Plan: Unit test added Reviewed By: archang19 Differential Revision: D95648000 Pulled By: jaykorean fbshipit-source-id: 512f3c1a7449b96a660865531f3537624c89a9cc

meta-cla bot added the CLA Signed label Nov 5, 2025

jaykorean requested review from hx235 and pdillinger November 5, 2025 20:15

jaykorean changed the title ~~[Remote Compaction] Add option to verify block checksums of output files~~ Add option to verify block checksums of output files Nov 5, 2025

jaykorean marked this pull request as ready for review November 5, 2025 23:32

jaykorean requested a review from xingbowang November 6, 2025 17:35

pdillinger requested changes Nov 6, 2025

View reviewed changes

jaykorean added 4 commits November 6, 2025 21:14

[Remote Compaction] Add option to verify block checksums of output files

6a000a9

Add the new option to options test

2a4f305

Use bitmask option

2a2a341

First set of comments

bb3746f

jaykorean force-pushed the verify_output_files branch from 9226130 to 6d56c04 Compare November 7, 2025 05:14

Second set of comments

14bc02f

jaykorean force-pushed the verify_output_files branch from 6d56c04 to 14bc02f Compare November 7, 2025 05:41

jaykorean requested a review from pdillinger November 7, 2025 15:35

jaykorean commented Nov 7, 2025

View reviewed changes

Make linter happy

521215f

pdillinger approved these changes Nov 7, 2025

View reviewed changes

Fix linter and Corruption message per feedback

5bf275f

jaykorean force-pushed the verify_output_files branch from 65e6e5c to 5bf275f Compare November 7, 2025 18:21

meta-codesync bot closed this in 5879f8b Nov 7, 2025

facebook-github-bot added the Merged label Nov 7, 2025

jaykorean deleted the verify_output_files branch November 10, 2025 18:07

jaykorean mentioned this pull request Mar 6, 2026

Add option to verify file checksum of output files #14433

Closed

		@@ -704,6 +724,13 @@ struct AdvancedColumnFamilyOptions {
		// Dynamically changeable through SetOptions() API
		bool paranoid_file_checks = false;

Conversation

jaykorean commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

To follow up

Test Plan

Uh oh!

meta-codesync bot commented Nov 5, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pdillinger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

meta-codesync bot commented Nov 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jaykorean commented Nov 5, 2025 •

edited

Loading