Skip to content

feat: add eloq_store_root_meta_cache_size to eloq_store_config#377

Closed
eatbreads wants to merge 4 commits intoeloqdata:mainfrom
eatbreads:main
Closed

feat: add eloq_store_root_meta_cache_size to eloq_store_config#377
eatbreads wants to merge 4 commits intoeloqdata:mainfrom
eatbreads:main

Conversation

@eatbreads
Copy link
Collaborator

@eatbreads eatbreads commented Jan 23, 2026

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

  • New Features

    • Added a configurable global RootMeta cache size for EloqStore (default 1GB). If left at default, the system can auto-size the cache based on node memory to optimize performance.
    • Cache allocation is now accounted for in overall node memory calculations.
  • Chores

    • Updated internal submodule pointer with no user-facing behavioral changes.

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

@coderabbitai
Copy link

coderabbitai bot commented Jan 23, 2026

Walkthrough

Adds a global EloqStore root-meta cache-size flag and config fallback/auto-derivation, parses and assigns it to eloqstore configs, subtracts it from node memory accounting before other sizing, and updates the eloqstore submodule pointer.

Changes

Cohort / File(s) Summary
Configuration: root-meta cache
store_handler/eloq_data_store_service/eloq_store_config.cpp
Added DEFINE_string(eloq_store_root_meta_cache_size, "1GB", ...); compute root_meta_cache_size from flag, config file, or auto-derived (node_memory_mb / 20 with "MB" suffix); parse via parse_size, assign to eloqstore_configs_.root_meta_cache_size, log auto-selection when used, and subtract from node_memory_mb prior to other sizing (e.g., manifest_limit).
Submodule pointer
store_handler/eloq_data_store_service/eloqstore
Updated submodule commit pointer (bumped to a newer commit). No in-diff code behavior changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • thweetkomputer
  • MrGuin

Poem

🐇 I nibble flags and count the RAM,
I tuck root-meta into a tidy dam.
From config, flag, or memory's share,
I stash the cache with gentle care.
A submodule hop — the code feels light.

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is a checklist template with no substantive content describing the actual changes or their rationale. Replace the template with a meaningful description explaining why the cache size configuration was added, what problem it solves, and details about the eloqstore dependency bump.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding eloq_store_root_meta_cache_size configuration flag to eloq_store_config, which is the primary code addition shown in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@store_handler/eloq_data_store_service/eloqstore`:
- Line 1: Update the size-validation and parsing to explicitly reject zero and
absurdly large values and add tests: modify is_valid_size() and parse_size() in
eloq_store_config.cpp to ensure parsed numeric value > 0 and <= a defined
MAX_ROOT_META_CACHE_BYTES (choose a sensible cap or compare to system total
memory via a helper like getTotalSystemMemory()), and ensure unit parsing only
accepts supported units (e.g., B, KB, MB, GB, TB) so unknown units like "PB" are
rejected; then add unit/edge-case tests in eloqstore_config_test (and an
integration test that sets eloq_store_root_meta_cache_size and verifies
EloqStore uses the parsed value) to cover valid cases ("1GB","512MB","2TB") and
invalid cases ("0GB","-1GB","1PB","abc","").

@thweetkomputer thweetkomputer changed the title feat: add eloq_store_root_meta_cache_size and bump eloqstore to 8617e36 feat: add eloq_store_root_meta_cache_size Jan 30, 2026
@thweetkomputer thweetkomputer changed the title feat: add eloq_store_root_meta_cache_size feat: add eloq_store_root_meta_cache_size to eloq_store_config Jan 30, 2026
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