feat:slow log field

[chejinge]

Summary by CodeRabbit

Release Notes

• New Features
  • Enhanced slowlog to track and display element counts for complex data types (hashes, sets, lists, sorted sets), providing better visibility into command performance metrics.

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

[coderabbitai]

Walkthrough

The slowlog system is extended to track element counts for complex data types. A new element_count field is added to the SlowlogEntry structure, the SlowlogPushEntry method signature is updated to accept this parameter, and slowlog capturing logic now queries database storage to obtain element counts for hashes, sets, lists, and sorted sets when commands exceed slowlog thresholds.

Changes

Cohort / File(s)	Summary
Data Structure & Interface Definitions include/pika_define.h, include/pika_server.h	Added element_count (int64\_t, default -1) member to SlowlogEntry struct; added default constructor to SlowlogEntry; updated SlowlogPushEntry method signature to include optional element_count parameter
Slowlog Capture & Element Counting src/pika_client_conn.cc	Enhanced slowlog processing to capture element counts by inspecting command names, querying DB storage (HLen, SCard, LLen, ZCard) for complex types, and passing element\_count to SlowlogPushEntry
Slowlog Serialization & Storage src/pika_admin.cc, src/pika_server.cc	Updated slowlog entry serialization to conditionally include element\_count field in output; modified SlowlogPushEntry implementation to store element\_count in slowlog entry

Sequence Diagram

sequenceDiagram
    participant Client
    participant PikaClientConn
    participant DB as Storage
    participant SlowlogCmd
    
    Client->>PikaClientConn: Execute command
    PikaClientConn->>PikaClientConn: Command exceeds slowlog threshold
    PikaClientConn->>PikaClientConn: Inspect command type
    alt Complex Type (hash, set, list, zset)
        PikaClientConn->>DB: Query element count<br/>(HLen/SCard/LLen/ZCard)
        DB-->>PikaClientConn: Return count
        PikaClientConn->>PikaClientConn: element_count = count
    else Other Type
        PikaClientConn->>PikaClientConn: element_count = -1
    end
    PikaClientConn->>PikaClientConn: SlowlogPushEntry(argv, time, duration, element_count)
    PikaClientConn->>SlowlogCmd: Store entry with metadata
    SlowlogCmd->>SlowlogCmd: Serialize with element_count if ≥ 0

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Element counting logic in pika_client_conn.cc: Verify that DB queries (HLen, SCard, LLen, ZCard) are correctly matched to command types and that element count retrieval doesn't introduce performance overhead
• Type-specific command filtering: Ensure the command name inspection logic correctly identifies all relevant complex data types and doesn't miss edge cases
• Default value propagation: Confirm that the -1 default for element_count is consistently used and doesn't inadvertently hide missing data

Poem

🐰 Hops through slowlogs with counting glee,
Tracking hashes, sets, and lists with spree,
Element counts now dance in the log,
No more mystery in the database fog! 🌲

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	The title 'feat:slow log field' is vague and generic, using non-descriptive phrasing that doesn't clearly convey the specific change being made.	Use a more descriptive title that specifies what was added to the slowlog, such as 'feat: add element_count field to slowlog entries' or 'feat: track element counts in slowlog for complex data types'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

include/pika_define.h (1)

287-294: SlowlogEntry.element_count semantics look good; consider in-class defaults instead of ctor

The new element_count field with -1 sentinel integrates cleanly with the slowlog code paths that treat < 0 as “not applicable.” The explicit default constructor is fine, but it also turns SlowlogEntry into a non-aggregate, which can break any brace-initialization uses that may have existed.

If you want to avoid that risk and keep the type an aggregate, you could instead rely on in-class default member initializers and drop the custom ctor, e.g.:

 struct SlowlogEntry {
-  int64_t id;
-  int64_t start_time;
-  int64_t duration;
+  int64_t id = 0;
+  int64_t start_time = 0;
+  int64_t duration = 0;
   net::RedisCmdArgsType argv;
-  int64_t element_count;  // 复杂数据类型的元素个数，-1表示不适用
-  SlowlogEntry() : id(0), start_time(0), duration(0), element_count(-1) {}
+  int64_t element_count = -1;  // element count for complex types, -1 means N/A
 };

This preserves default values while keeping initialization flexible and consistent.

src/pika_client_conn.cc (1)

233-327: Slowlog element_count computation is correct; consider centralizing command-to-type mapping

The new ProcessSlowlog logic correctly:

• Guards on the slowlog threshold before doing any extra work.
• Normalizes cmd_name to lowercase and only issues an HLEN/SCARD/LLEN/ZCARD when the command is one of the expected hash/set/list/zset operations.
• Uses element_count = -1 as a “no data” sentinel, which aligns with the admin and server code, and passes it through to SlowlogPushEntry.
• Extends the slowlog error log with an element_count fragment only when the value is set, preserving the old log format otherwise.

Functionally this looks solid. If you want to reduce duplication and make it easier to extend, you might factor the long if/else if command-name chain into a small helper that maps cmd_name → container type (or directly to a lambda that performs the appropriate *Card call), so future commands don’t have to modify this function’s control flow.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7153e72 and e7344a7.

📒 Files selected for processing (5)

• include/pika_define.h (1 hunks)
• include/pika_server.h (1 hunks)
• src/pika_admin.cc (1 hunks)
• src/pika_client_conn.cc (2 hunks)
• src/pika_server.cc (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

include/pika_server.h (1)

src/pika_server.cc (2)

• SlowlogPushEntry (954-987)
• SlowlogPushEntry (954-954)

src/pika_client_conn.cc (4)

src/pika_command.cc (2)

• argv (1057-1057)
• argv (1057-1057)

src/pstd/src/pstd_string.cc (2)

• StringToLower (638-641)
• StringToLower (638-638)

src/storage/src/lists_data_key_format.h (1)

• key (103-103)

src/storage/src/zsets_data_key_format.h (1)

• key (112-112)

src/pika_admin.cc (1)

src/net/src/redis_cli.cc (1)

• arg (239-239)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Analyze (go)
• GitHub Check: build_on_ubuntu
• GitHub Check: build_on_centos
• GitHub Check: build_on_macos

🔇 Additional comments (3)

src/pika_server.cc (1)

954-987: SlowlogPushEntry correctly propagates element_count

The extended signature and entry.element_count = element_count; assignment integrate cleanly with the existing slowlog path; default -1 from the header will maintain prior behavior when callers don’t supply a value. No concurrency or correctness issues here.

src/pika_admin.cc (1)

3339-3355: Slowlog serialization cleanly extends to optional element_count

The logic to emit a 5-element array only when slowlog.element_count >= 0 and append the extra integer field after the argv list preserves backward compatibility (4 fields when not set) while exposing the new metric when available. This matches the -1 sentinel semantics used in SlowlogEntry.

include/pika_server.h (1)

240-246: SlowlogPushEntry API extension is backward compatible

The updated declaration adding int64_t element_count = -1 keeps existing callers working via the default argument while allowing new call sites (like ProcessSlowlog) to pass a real count. The type remains compatible with PikaCmdArgsType, and the definition in pika_server.cc matches the signature.