Skip to content

chore: change eloq_store_max_write_batch_pages default to 32#385

Merged
thweetkomputer merged 3 commits intomainfrom
fix-mapping-snapshot-lifetime-zc
Jan 30, 2026
Merged

chore: change eloq_store_max_write_batch_pages default to 32#385
thweetkomputer merged 3 commits intomainfrom
fix-mapping-snapshot-lifetime-zc

Conversation

@thweetkomputer
Copy link
Collaborator

@thweetkomputer thweetkomputer commented Jan 29, 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

  • Chores
    • Updated internal submodule reference — maintenance work with no functional changes.
    • Adjusted default configuration for write batching, reducing the default maximum write-batch pages; may affect batching behavior and performance characteristics.

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

@coderabbitai
Copy link

coderabbitai bot commented Jan 29, 2026

Walkthrough

Updated the eloqstore submodule pointer and reduced the default value of eloq_store_max_write_batch_pages from 256 to 32; no public API signatures were changed.

Changes

Cohort / File(s) Summary
Submodule Pointer Update
store_handler/eloq_data_store_service/eloqstore
Advanced submodule reference from commit ca395ee54d6f... to a93af59a5fdb...; no code changes in this repository.
Config Default Change
store_handler/eloq_data_store_service/eloq_store_config.cpp
Changed default for DEFINE_uint32(eloq_store_max_write_batch_pages) from 256 to 32; no signature or control-flow changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • lzxddz
  • liunyl

Poem

🐰 I hopped along the file-lined hill,
A pointer nudged, a number shrank until,
The store stays steady, defaults now light —
I nibble code by moonlit byte. 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete; it only contains the unchecked template checklist with no actual description of changes, motivation, or context for why the default value was changed. Add a description explaining the purpose of changing the default value from 256 to 32, any performance or resource implications, and complete the checklist items by adding issue references, tests, and documentation as applicable.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'chore: change eloq_store_max_write_batch_pages default to 32' clearly and specifically describes the main change in the changeset—updating a configuration flag's default value.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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
  • Commit unit tests in branch fix-mapping-snapshot-lifetime-zc

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 PR description to include the upstream commit summary:
mention commit 0251a3d5891919c4d19f78b4f052411fc900a63e, state that it fixes
"MappingSnapshot UAF after shard stop", and note that it adds comprehensive test
coverage across multiple components (include a short list of affected areas if
known). Keep the summary brief and add the commit hash so reviewers can
reference the upstream change directly.

@thweetkomputer thweetkomputer force-pushed the fix-mapping-snapshot-lifetime-zc branch from 0251a3d to 59607e5 Compare January 29, 2026 11:11
@thweetkomputer thweetkomputer changed the title update eloqstore: fix: prevent MappingSnapshot UAF after shard stop chore: change eloq_store_max_write_batch_pages default to 32 Jan 30, 2026
@thweetkomputer thweetkomputer merged commit f08f56d into main Jan 30, 2026
4 checks passed
@thweetkomputer thweetkomputer deleted the fix-mapping-snapshot-lifetime-zc branch January 30, 2026 07:51
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