support canceling ongoing CompactFiles

[virajthakur]

Add an atomic bool to CompactionOptions to cancel an ongoing CompactFiles() operation, in the same fashion we do for CompactRange().

Test Plan: ./db_test2 --gtest_filter=DBTest2.TestCancelCompactFiles

[facebook-github-bot]

@virajthakur has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[Copilot]

Pull Request Overview

Adds support for canceling ongoing manual CompactFiles() operations by introducing a cancellation flag, wiring it into the compaction logic, and exercising it in a new test.

• Introduce std::atomic<bool>* canceled in CompactionOptions with default initialization.
• Update DBImpl::CompactFilesImpl to check the new cancel flag alongside existing pause logic.
• Add TestCancelCompactFiles in db_test2.cc and fix minor typos/removals in test files.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
include/rocksdb/options.h	Added canceled flag to CompactionOptions and initialized it to nullptr.
db/db_impl/db_impl_compaction_flush.cc	Wired the cancel flag into CompactFilesImpl and registered a syncpoint callback.
db/db_test2.cc	Removed unused header, fixed typo, and added TestCancelCompactFiles.

Comments suppressed due to low confidence (4)

include/rocksdb/options.h:2229

• [nitpick] Using a raw pointer for the cancellation flag can lead to dangling pointers if CompactionOptions is copied or the referenced atomic goes out of scope. Consider owning an std::atomic<bool> directly or using a safer handle (e.g., std::optional<std::reference_wrapper<std::atomic<bool>>>) to manage lifetime.

std::atomic<bool>* canceled;

include/rocksdb/options.h:2229

• Document that the atomic<bool>* canceled pointer must remain valid for the entire duration of the compaction to avoid undefined behavior.

std::atomic<bool>* canceled;

db/db_test2.cc:5693

• [nitpick] Variable name disable_compaction is a bit generic; consider renaming to disable_manual_compaction for clarity.

bool disable_compaction = false;

db/db_impl/db_impl_compaction_flush.cc:1443

• Add a unit test where CompactionOptions.canceled remains nullptr to verify that compaction proceeds normally and no null-pointer dereference occurs.

(compact_options.canceled &&

[facebook-github-bot]

@virajthakur has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@virajthakur has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@virajthakur has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@virajthakur has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[jaykorean]

LGTM!

[jaykorean]

I see that we are ingesting files for this test. This seems to be the same as what TestCompactFiles do and this is totally fine.

Just FYI in case you want to just call PUTs, flush them and call CompactFiles() on those SST files without knowing the file names in advance.

rocksdb/db/db_with_timestamp_compaction_test.cc

Lines 298 to 320 in 2dcfc54

	std::vector<std::string> inputs;
	{
	std::vector<LiveFileMetaData> fmetas;
	db_->GetLiveFilesMetaData(&fmetas);
	bool included_one_l1 = false;
	for (const auto& meta : fmetas) {
	if (meta.level == 0) {
	inputs.emplace_back(meta.relative_filename);
	} else if (!included_one_l1) {
	inputs.emplace_back(meta.relative_filename);
	included_one_l1 = true;
	}
	}
	}
	ASSERT_EQ(static_cast<size_t>(3), inputs.size());
	{
	std::vector<std::string> output_file_names;
	CompactionJobInfo compaction_job_info;
	ASSERT_OK(db_->CompactFiles(CompactionOptions(), inputs, /*output_level=*/1,
	/*output_path_id=*/-1, &output_file_names,
	&compaction_job_info));
	ASSERT_EQ(kNumFiles * kKeysPerFile + 2, output_file_names.size());

[facebook-github-bot]

@virajthakur has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@virajthakur has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@virajthakur merged this pull request in 4bdfb7e.

[cbi42]

IIUC, this only cancels CompactFiles() before compaction starts running. We should be able to cancel an ongoing compaction similar to CompactRange():

rocksdb/db/db_impl/db_impl_compaction_flush.cc

Line 4214 in 4bdfb7e

is_manual ? manual_compaction->canceled