fix oom reported in eloqkv

[liunyl]

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

• Refactor
  • Adjusted memory management logic affecting how waiting requests are processed during periods of memory pressure.

[coderabbitai]

Walkthrough

A preprocessor conditional in the memory-cleaning branch of ShardCleanCc::Execute was changed from ONE_KEY_OBJECT to ON_KEY_OBJECT, altering whether DequeueWaitListAfterMemoryFree(true) is executed. This affects how waiting ccrequests are managed when the heap remains above threshold.

Changes

Cohort / File(s)	Summary
Preprocessor Conditional Update include/cc/cc_request.h	Changed memory-management branch guard from ONE_KEY_OBJECT to ON_KEY_OBJECT, which changes whether the wait-list dequeue operation is executed during memory pressure handling in ShardCleanCc::Execute.

Sequence Diagram(s)

sequenceDiagram
    participant EC as ShardCleanCc::Execute
    participant MH as Heap Monitor
    participant WL as Wait List

    EC->>MH: Check heap above threshold
    alt ONE_KEY_OBJECT (old behavior)
        MH-->>EC: Threshold exceeded
        EC->>WL: DequeueWaitListAfterMemoryFree(true)
        WL-->>EC: Process waiting requests
    else ON_KEY_OBJECT (new behavior)
        MH-->>EC: Threshold exceeded
        EC->>EC: Skip dequeue operation
        EC-->>WL: Wait-list not processed
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Area requiring attention: Verify that the macro name change (ONE_KEY_OBJECT → ON_KEY_OBJECT) is intentional and not a typo, and confirm that skipping the dequeue operation in the new path is the desired behavior
• Critical concern: Understand the impact of this change on request queue management and memory pressure handling to ensure it doesn't introduce stalls or request timeouts

Suggested reviewers

• lzxddz
• xiexiaoy

Poem

A rabbit hops through memory's lane, 🐰
Adjusting guards to ease the strain,
One key swapped for one anew,
Queues now dance a different due,
Cleaner code, more logic true! 🎉

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description only contains the template checklist with all items unchecked; it lacks actual details about the change, issue reference, testing status, and implementation specifics.	Complete the description by checking off completed items, explicitly referencing the issue number, documenting what was changed and why, and confirming test results.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix oom reported in eloqkv' is specific and directly related to the changeset, which fixes an out-of-memory issue by modifying a preprocessor conditional in ShardCleanCc::Execute.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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 oom

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d17a8d2 and 63b15fe.

📒 Files selected for processing (1)

• include/cc/cc_request.h (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: thweetkomputer
Repo: eloqdata/tx_service PR: 150
File: include/cc/local_cc_shards.h:626-631
Timestamp: 2025-10-09T03:56:58.811Z
Learning: For the LocalCcShards class in include/cc/local_cc_shards.h: Writer locks (unique_lock) should continue using the original meta_data_mux_ (std::shared_mutex) rather than fast_meta_data_mux_ (FastMetaDataMutex) at this stage. Only reader locks may use the FastMetaDataMutex wrapper.

🔇 Additional comments (1)

include/cc/cc_request.h (1)

8430-8437: The macro-name change appears intentional and correct, not a typo. However, the undefined guard still needs clarification.

The git diff confirms the change was #ifndef ONE_KEY_OBJECT → #ifndef ON_KEY_OBJECT. However, verification shows that ONE_KEY_OBJECT never existed in the codebase—ON_KEY_OBJECT is the actual feature name referenced in comments throughout object_cc_map.h. This suggests the change is fixing a prior typo, not introducing one.

Since neither macro is currently defined anywhere, the #ifndef guard is always true and the "wait for memory" path (#else block) is dead code. The guard will become meaningful only if ON_KEY_OBJECT is later conditionally defined at build time.

Two valid options:

1. Remove the guard entirely if the abort-on-OOM behavior is now unconditional and intentional.
2. Ensure ON_KEY_OBJECT is explicitly documented and conditionally defined in the build system if conditional behavior is required.

Clarify the intent and either commit to the unconditional abort or formally introduce the build flag.

Likely an incorrect or invalid review 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

• X
• Mastodon
• Reddit
• LinkedIn

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