Skip to content

Disable auto compaction at the 1st place to make sure purger blocking… #92

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.

Sign up for GitHub

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

Merged
githubzilla merged 1 commit into from
Sep 23, 2025
Merged

Disable auto compaction at the 1st place to make sure purger blocking… #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 2 additions & 1 deletion eloq_data_store_service/purger_sliding_window.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ void S3FileNumberUpdater::UpdateSmallestFileNumber(uint64_t file_number,

void S3FileNumberUpdater::BlockPurger(const std::string &epoch)
{
DLOG(INFO) << "Wrote 0 as file number to S3 to block purger";
DLOG(INFO) << "Wrote 0 as file number to S3 to block purger, epoch: "
<< epoch;
UpdateSmallestFileNumber(std::numeric_limits<uint64_t>::min(), epoch);
// Don't update last_published_smallest_ in sliding window,
// so that future smaller file number can still be updated
Expand Down
6 changes: 6 additions & 0 deletions eloq_data_store_service/rocksdb_cloud_data_store.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,9 @@ bool RocksDBCloudDataStore::OpenCloudDB(
options.compaction_filter =
static_cast<rocksdb::CompactionFilter *>(ttl_compaction_filter_.get());

// Disable auto compactions before blocking purger
options.disable_auto_compactions = true;

Comment on lines +610 to +612

@coderabbitai coderabbitai Bot Sep 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

🧩 Analysis chain

Verify scope: disable_auto_compactions here affects only default CF

disable_auto_compactions is a ColumnFamily option. Setting it on Options before open typically covers only the default CF; existing CFs opened from manifest may keep their own settings. If multiple CFs exist, auto compactions may still run for them until PauseBackgroundWork executes.

Consider:

  • If you have multiple CFs: open with explicit ColumnFamilyDescriptors, setting disable_auto_compactions = true per CF; or immediately after open, call SetOptions on every CF handle before any background work resumes.

Run this to locate CF usage and confirm single-CF assumption:

🏁 Script executed:

#!/bin/bash
# Scan for column family usage patterns
rg -n -C2 -g '!**/build/**' -P --type=cpp '\b(CreateColumnFamily|ListColumnFamilies|DefaultColumnFamily\(|ColumnFamilyHandle\b|GetColumnFamilyHandle\b)'

Length of output: 148

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "PWD: $(pwd)"
echo "Top-level listing:"
ls -la

TARGET="eloq_data_store_service/rocksdb_cloud_data_store.cpp"
echo
echo "Check target: $TARGET"
if [ -f "$TARGET" ]; then
  echo "FOUND $TARGET"
  echo
  echo "Print 1-220:"
  sed -n '1,220p' "$TARGET" || true
  echo "----"
  echo "Print 560-720 (area around line ~610):"
  sed -n '560,720p' "$TARGET" || true
else
  echo "MISSING $TARGET"
fi

echo
echo "rg: searching for RocksDB ColumnFamily usage and related symbols (forced, include ignored files)"
rg -uu -n -S 'ColumnFamilyHandle|CreateColumnFamily|ListColumnFamilies|GetColumnFamilyHandle|DefaultColumnFamily|ColumnFamilyDescriptor|ColumnFamilyOptions' || true

echo
echo "rg: searching for disable_auto_compactions occurrences"
rg -uu -n -S 'disable_auto_compactions' || true

Length of output: 21574

Apply disable_auto_compactions per-column-family (Options covers only default CF)

disable_auto_compactions is a ColumnFamily option — setting options.disable_auto_compactions before DB::Open only affects the default CF. This repo manages multiple CFs (rocksdb_handler.cpp), so other CFs can still perform compactions between Open and PauseBackgroundWork.

Action: either open with ColumnFamilyDescriptor(s) that set disable_auto_compactions=true for every CF, or immediately after Open and before ContinueBackgroundWork call SetOptions on every ColumnFamilyHandle to disable compactions (and re-enable later).

Locations: eloq_data_store_service/rocksdb_cloud_data_store.cpp (disable true ~line 611; SetOptions(false) ~line 691). Inspect rocksdb_handler.cpp for CF creation/listing.

🤖 Prompt for AI Agents 
In eloq_data_store_service/rocksdb_cloud_data_store.cpp around lines 610-612 and
691, disable_auto_compactions is being set only on the global Options (affects
default CF only) so non-default column families can still compact; update the
code to apply disable_auto_compactions to every CF: either construct
ColumnFamilyDescriptor entries for each column family (see rocksdb_handler.cpp
for CF names/creation) and set disable_auto_compactions=true on each descriptor
before DB::Open, or immediately after DB::Open and before ContinueBackgroundWork
call SetOptions on every returned ColumnFamilyHandle to disable compactions (and
ensure you later call SetOptions to re-enable when unpausing). Ensure this
change targets all CFs and preserves the existing re-enable logic around line
~691.

auto start = std::chrono::system_clock::now();
std::unique_lock<std::shared_mutex> db_lk(db_mux_);
rocksdb::Status status;
Expand Down Expand Up @@ -684,6 +687,9 @@ bool RocksDBCloudDataStore::OpenCloudDB(
// Resume background work
db_->ContinueBackgroundWork();

// Enable auto compactions after blocking purger
status = db_->SetOptions({{"disable_auto_compactions", "false"}});

Comment on lines 687 to +692

@coderabbitai coderabbitai Bot Sep 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: compactions may remain disabled — wrong SetOptions overload and ordering

  • SetOptions without a ColumnFamilyHandle targets DBOptions; disable_auto_compactions is a CF option, so this likely no-ops/fails.
  • Also enable compactions before resuming background work to avoid running with compactions disabled if SetOptions fails.

Apply this fix:

-    // Resume background work
-    db_->ContinueBackgroundWork();
-
-    // Enable auto compactions after blocking purger
-    status = db_->SetOptions({{"disable_auto_compactions", "false"}});
+    // Enable auto compactions after blocking purger (default column family)
+    status = db_->SetOptions(
+        db_->DefaultColumnFamily(),
+        {{"disable_auto_compactions", "false"}});
+    if (!status.ok())
+    {
+        LOG(ERROR) << "Fail to enable auto compactions, error: "
+                   << status.ToString();
+        // Avoid leaving background work paused; resume and fail fast.
+        db_->ContinueBackgroundWork();
+        return false;
+    }
+    // Resume background work
+    db_->ContinueBackgroundWork();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Resume background work
db_->ContinueBackgroundWork();
// Enable auto compactions after blocking purger
status = db_->SetOptions({{"disable_auto_compactions", "false"}});
// Enable auto compactions after blocking purger (default column family)
status = db_->SetOptions(
db_->DefaultColumnFamily(),
{{"disable_auto_compactions", "false"}});
if (!status.ok())
{
LOG(ERROR) << "Fail to enable auto compactions, error: "
<< status.ToString();
// Avoid leaving background work paused; resume and fail fast.
db_->ContinueBackgroundWork();
return false;
}
// Resume background work
db_->ContinueBackgroundWork();
🤖 Prompt for AI Agents 
In eloq_data_store_service/rocksdb_cloud_data_store.cpp around lines 687-692,
the code calls db_->SetOptions(...) with a DB-only overload after resuming
background work, which doesn't set the column-family-level
"disable_auto_compactions" and may leave compactions disabled; change to call
the ColumnFamily-aware SetOptions (db_->SetOptions(column_family_handle,
{{"disable_auto_compactions","false"}})) before calling ContinueBackgroundWork,
check the returned status and handle/log any error (and avoid resuming
background work if the SetOptions fails).

if (cloud_config_.warm_up_thread_num_ != 0)
{
db_->WarmUp(cloud_config_.warm_up_thread_num_);
Expand Down