[thweetkomputer]

redis_cmd_current_rounds_ was sized by the number of registered commands but indexed by the RedisCommandType enum value. Optional commands are pinned with gaps at 176-186 (COMPACT/FAULT_INJECT/ELOQVEC_*/KEYSPACE), so their enum values exceed the registered-command count and indexing them read/wrote past the buffer (caught by ASan when running fault_inject). Size the per-core vector by the enum value range instead.

Summary by CodeRabbit

• Bug Fixes
  • Improved metrics initialization for Redis command tracking, making per-core counters more reliable and consistent across command types (including optional/gapped values).

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bf9b53f1-ade3-475a-8730-fd93e34259b4

📥 Commits

Reviewing files that changed from the base of the PR and between ee8a889 and 4be0820.

📒 Files selected for processing (1)

• data_substrate

---

Walkthrough

Updates Redis metrics initialization to use a fixed 256-slot vector for redis_cmd_current_rounds_ indexing, and advances the data_substrate subproject to a new pinned commit.

Changes

Metrics vector sizing fix

Layer / File(s)	Summary
Fixed-size metrics vector src/redis_service.cpp	redis_cmd_current_rounds_ is resized to 256 entries when metrics are enabled, with comments indicating RedisCommandType-based indexing.
Subproject pin update data_substrate	The subproject reference is updated from one pinned commit hash to another.

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

Possibly related PRs

• eloqdata/eloqkv#316: Touches the same RedisServiceImpl::Start metrics initialization path in src/redis_service.cpp.
• eloqdata/eloqkv#518: Relates to RedisCommandType stability, which matches the enum-indexing assumption used here.

Suggested labels: trigger-ci

Suggested reviewers: MrGuin, lzxddz

Poem

A bunny set a tidy row,
For command slots to start and grow.
Two-fifty-six now holds the way,
While submodules hop to stay in play. 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main fix: sizing Redis command round counters by the enum range.
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 unit tests (beta)

• Create PR with unit tests

---

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]

🧹 Nitpick comments (1)

src/redis_service.cpp (1)

741-750: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Good fix, but consider guarding against future enum growth.

The fixed size of 256 correctly covers the pinned enum gaps described in the PR objectives. To prevent this bug from silently reoccurring if RedisCommandType grows past 256, consider a compile-time guard tied to the enum's max value.

♻️ Suggested defensive guard

     if (metrics::enable_metrics)
     {
         redis_cmd_current_rounds_.resize(core_num_);
         for (auto &vec : redis_cmd_current_rounds_)
         {
+            // NOLINTNEXTLINE if RedisCommandType ever exceeds 256, bump this.
+            static_assert(static_cast<size_t>(RedisCommandType::MAX_VALUE) < 256,
+                          "redis_cmd_current_rounds_ sizing must cover enum range");
             // Indexed by RedisCommandType, whose values are pinned with gaps
             // (optional commands occupy 176-186), so size by the enum range
             // rather than the number of registered commands.
             vec.resize(256, 1);
         }

(Adjust RedisCommandType::MAX_VALUE to whatever the actual last/sentinel enumerator is named.)

🤖 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 `@src/redis_service.cpp` around lines 741 - 750, The fixed 256 resize in the
metrics initialization is fine for the current enum gaps, but
`redis_cmd_current_rounds_` should be protected against future
`RedisCommandType` growth. Add a compile-time guard in the same initialization
area that checks the enum’s maximum/sentinel value (for example the last
`RedisCommandType` enumerator used by the command indexing logic) and fails the
build if it exceeds the 256 slot capacity, so `redis_service.cpp` cannot
silently overflow this assumption later.

🤖 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.

Nitpick comments:
In `@src/redis_service.cpp`:
- Around line 741-750: The fixed 256 resize in the metrics initialization is
fine for the current enum gaps, but `redis_cmd_current_rounds_` should be
protected against future `RedisCommandType` growth. Add a compile-time guard in
the same initialization area that checks the enum’s maximum/sentinel value (for
example the last `RedisCommandType` enumerator used by the command indexing
logic) and fails the build if it exceeds the 256 slot capacity, so
`redis_service.cpp` cannot silently overflow this assumption later.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0c54252a-3407-4819-8a81-0afb4a140a37

📥 Commits

Reviewing files that changed from the base of the PR and between b65dd16 and ee8a889.

📒 Files selected for processing (1)

• src/redis_service.cpp