Skip to content

Fix compile error with open log service#315

Merged
thweetkomputer merged 2 commits into
mainfrom
fix_bootstrap
Dec 23, 2025
Merged

Fix compile error with open log service#315
thweetkomputer merged 2 commits into
mainfrom
fix_bootstrap

Conversation

@thweetkomputer

@thweetkomputer thweetkomputer commented Dec 23, 2025

Copy link
Copy Markdown
Collaborator

Here are some reminders before you submit the pull request

  • Add tests for the change
  • Document changes
  • Reference the link of issue using fixes eloqdb/tx_service#issue_id
  • Reference the link of RFC if exists
  • Pass ./mtr --suite=mono_main,mono_multi,mono_basic

Summary by CodeRabbit

  • Chores
    • Enhanced build-time configuration handling to better control storage cleanup operations during application shutdown based on compilation settings.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Dec 23, 2025

Copy link
Copy Markdown

Walkthrough

Build-time guards around RocksDB operations in DataSubstrate::Shutdown() are tightened to conditionally exclude RocksDB storage path access and removal when OPEN_LOG_SERVICE is defined, while preserving normal logservice shutdown flow. No runtime control flow changes occur beyond these preprocessor-guarded sections.

Changes

Cohort / File(s) Summary
RocksDB Shutdown Cleanup Guards
core/src/data_substrate.cpp
Tightened build-time conditions around RocksDB-related operations during shutdown: RocksDB path retrieval and removal now only execute when LOG_STATE_TYPE_RKDB_ALL is defined AND OPEN_LOG_SERVICE is not defined; code is excluded in bootstrap mode.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

Suggested reviewers

  • lzxddz
  • MrGuin
  • liunyl

Poem

🐰 A gate stands guard at shutdown's door,
Where RocksDB paths were cleaned before—
Now guarded tight in bootstrap's way,
Only when the flag says "play,"
The cleanup dance can have its say! 🔐

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description contains only an unchecked template checklist with no actual implementation details, context about the fix, or explanation of the compile error being resolved. Replace the template checklist with a meaningful description explaining the compile error, why it occurs with OPEN_LOG_SERVICE, and how the build-time guards resolve it.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title directly relates to the main change: fixing a compile error with open log service by adding build-time guards around RocksDB operations.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix_bootstrap

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c6efb6 and 50216c4.

📒 Files selected for processing (1)
  • core/src/data_substrate.cpp
🔇 Additional comments (1)
core/src/data_substrate.cpp (1)

250-267: ✓ Compile error fix is correct.

The preprocessor guards properly isolate RocksDB-specific code to prevent undefined method calls when OPEN_LOG_SERVICE is defined. The conditional logic correctly:

  1. Declares rocksdb_storage_path only when both LOG_STATE_TYPE_RKDB_ALL is defined and OPEN_LOG_SERVICE is not defined (lines 250–252)
  2. Skips RocksDB cleanup when OPEN_LOG_SERVICE is active (lines 257–267)
  3. Preserves general log storage cleanup regardless of build mode (lines 269–280)

The fix resolves the compile error while maintaining correct shutdown behavior across all build configurations.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

@thweetkomputer thweetkomputer merged commit 7d44114 into main Dec 23, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants