fix: publish the file number guard from DBCloud

[thweetkomputer]

Problem

eloqkv main CI is red on all four ELOQDSS_ROCKSDB_CLOUD_S3 jobs. The server never starts:

Open rocksdb cloud error : Invalid argument: publish_file_number_guard must be
enabled for a writable DBCloud when cloud file deletion is delegated to the
purger (disable_cloud_file_deletion or run_purger), retrying ...
rocksdb_cloud_data_store.cpp:718 Unable to open db ... bucket eloqkv-test
data_store_service.cpp:404     Failed to create data store
redis_server.cpp:481           Failed to start DataSubstrate.

The rocksdb-cloud bump in d342a5c (to 8802542, "cloud: fix purger garbage leak and move file-number-guard writer in-repo") added a hard check in DBCloudImpl::Open (cloud/db_cloud_impl.cc:139): a writable DBCloud whose cloud file deletion is delegated to the purger must set publish_file_number_guard.

RocksDBCloudDataStore::StartDB hardcodes disable_cloud_file_deletion = true and defaults run_purger to true, so the condition always matches and every open fails. The log service is unaffected — it sets run_purger = false.

Fix

Enable the option. It does two things: satisfies the check, and opens the gate at db_cloud_impl.cc:256 that installs the in-repo FileNumberGuardPublisher + FileNumberGuardListener.

That publisher is precisely the writer rocksdb-cloud moved in-repo. We already had our own — PurgerEventListener + SlidingWindow — so this change removes ours rather than running both:

	ours (removed)	in-repo (now used)
window	SlidingWindow	FileNumberSlidingWindow
listener	PurgerEventListener	FileNumberGuardListener
upload	S3FileNumberUpdater	FileNumberGuardPublisher
block purging	BlockPurger() writes 0	writes 0 sentinel

Both push a listener into the same options.listeners and PUT to the same smallest_new_file_number-<epoch> key. Running both would let two publishers land PUTs out of order and reinstate a stale high watermark — the purger would then delete an SST that is still in flight. FileNumberGuardPublisher's publish_mutex_ only serializes PUTs within one publisher.

Why the in-repo one supersedes ours

• Sources the epoch from the cloud manifest at publish time, instead of being constructed with "" and patched via SetEpoch after open.
• Writes the 0 sentinel before recovery can flush, as part of Open.
• Gates SST uploads through ProtectFileUpload (cloud/cloud_storage_provider.cc:203): the watermark is lowered before the upload proceeds. Ours republished on a timer, leaving a window where an SST was already in S3 but the guard had not come down yet — the purger could delete it. This is one of the leaks the upstream commit fixes.

Compatibility

On-cloud protocol is unchanged — same key <object_path>/smallest_new_file_number-<epoch>, same ASCII uint64 value, same 0 / UINT64_MAX sentinels. The object is per-epoch and each writer node owns its epoch, so mixed-version nodes on one bucket do not collide.

One behavior change comes from the submodule bump itself, not this PR: the purger's require_guard_marker now defaults to true, so an epoch with no marker aborts the purge cycle instead of falling back to the MANIFEST high watermark. Effect on an existing bucket is that old markerless epochs stop being collected (storage grows); no data loss. --require_guard_marker=false is available for a staged rollout.

Verification

• rocksdb_cloud_data_store.cpp compiles clean (-Wall, -fsyntax-only) against the submodule's rocksdb-cloud headers under -DDATA_STORE_TYPE_ELOQDSS_ROCKSDB_CLOUD_S3.
• No remaining references to the removed files anywhere in the tree.
• Full local build/link/run could not be validated: this machine's /opt/eloq/third_party prebuilt rocksdb-cloud predates the bump and lacks both the option and the check. CI's prime-third-party rebuilds it from the submodule, so the S3 jobs are the real gate.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor
  • Simplified cloud database startup by removing legacy purger event-listener and sliding-window handling.
  • Enabled safeguards for publishing cloud file numbers.
  • Streamlined S3 and GCS cloud storage integration while retaining database epoch validation.

[coderabbitai]

Walkthrough

The change removes RocksDB cloud purger listener and sliding-window components from builds and database initialization. It adds the S3 downloader source and enables the cloud file-number publication guard while retaining database epoch validation.

Changes

Cloud purger update

Layer / File(s)	Summary
Remove cloud purger sources CMakeLists.txt, store_handler/eloq_data_store_service/CMakeLists.txt, store_handler/eloq_data_store_service/purger_*	The cloud build lists no longer include purger listener or sliding-window sources. The S3 build adds s3_file_downloader.cpp.
Update cloud database initialization store_handler/eloq_data_store_service/rocksdb_cloud_data_store.cpp	The database opening path removes PurgerEventListener setup and enables publish_file_number_guard. Epoch retrieval and validation remain. PurgerEventListener and its purger controls are deleted.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: liunyl

Poem

A rabbit hops through clouds so bright,
The purger fades from build tonight.
File numbers guard the stream,
Epoch checks remain the scheme.
S3 paths now download right.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the primary fix: enabling publication of the DBCloud file-number guard.
Description check	✅ Passed	The description clearly explains the problem, implementation, compatibility impact, design rationale, and verification limits.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/publish-file-number-guard

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

store_handler/eloq_data_store_service/rocksdb_cloud_data_store.cpp (1)

743-762: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Release db_ when GetCurrentEpoch returns an empty epoch.

When current_epoch.empty(), this branch resumes background work and returns false without closing db_, deleting it, or setting db_ = nullptr. Because OpenCloudDB opens db_ before this validation, the failed start path leaves a partially initialized database and retained resources; apply the same cleanup as the surrounding failure paths.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@store_handler/eloq_data_store_service/rocksdb_cloud_data_store.cpp` around
lines 743 - 762, Update the current_epoch.empty() failure branch in OpenCloudDB
to close and delete db_, then set db_ to nullptr before returning false, while
preserving the existing ContinueBackgroundWork() call as required.

🧹 Nitpick comments (1)

store_handler/eloq_data_store_service/rocksdb_cloud_data_store.cpp (1)

652-653: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Update stale purger comments.

The purger listener and BlockPurger() call were removed, but Lines 665-666, 729-730, and 766 still describe blocking the purger. Replace those comments with the actual background-work and auto-compaction lifecycle.

Proposed comment update

-    // Disable auto compactions before blocking purger
+    // Disable auto compactions during cloud database initialization

-    // Stop background work - memtable flush and compaction
-    // before blocking purger
+    // Pause background work while validating the database epoch

-    // Enable auto compactions after blocking purger
+    // Re-enable auto compactions after cloud database initialization

As per coding guidelines, update stale nearby comments and the corresponding docs/ design document when behavior changes.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@store_handler/eloq_data_store_service/rocksdb_cloud_data_store.cpp` around
lines 652 - 653, The comments near the RocksDB configuration and related code
still describe the removed purger listener and BlockPurger() behavior. Update
the stale comments around the affected sections to accurately describe the
current background-work and auto-compaction lifecycle, and update the
corresponding docs/ design document to match the implemented behavior.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@store_handler/eloq_data_store_service/rocksdb_cloud_data_store.cpp`:
- Around line 743-762: Update the current_epoch.empty() failure branch in
OpenCloudDB to close and delete db_, then set db_ to nullptr before returning
false, while preserving the existing ContinueBackgroundWork() call as required.

---

Nitpick comments:
In `@store_handler/eloq_data_store_service/rocksdb_cloud_data_store.cpp`:
- Around line 652-653: The comments near the RocksDB configuration and related
code still describe the removed purger listener and BlockPurger() behavior.
Update the stale comments around the affected sections to accurately describe
the current background-work and auto-compaction lifecycle, and update the
corresponding docs/ design document to match the implemented behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f3ee72f5-f39a-4131-a232-8e49108cdb87

📥 Commits

Reviewing files that changed from the base of the PR and between d342a5c and fab04aa.

📒 Files selected for processing (7)

• CMakeLists.txt
• store_handler/eloq_data_store_service/CMakeLists.txt
• store_handler/eloq_data_store_service/purger_event_listener.cpp
• store_handler/eloq_data_store_service/purger_event_listener.h
• store_handler/eloq_data_store_service/purger_sliding_window.cpp
• store_handler/eloq_data_store_service/purger_sliding_window.h
• store_handler/eloq_data_store_service/rocksdb_cloud_data_store.cpp

💤 Files with no reviewable changes (6)

• store_handler/eloq_data_store_service/purger_event_listener.cpp
• store_handler/eloq_data_store_service/CMakeLists.txt
• store_handler/eloq_data_store_service/purger_sliding_window.h
• store_handler/eloq_data_store_service/purger_event_listener.h
• CMakeLists.txt
• store_handler/eloq_data_store_service/purger_sliding_window.cpp