Skip to content

Commit e83fa66

Browse files
author
gongna-au
committed
docs: update LATENCY PR description to reflect review fixes
1 parent 29b5fa0 commit e83fa66

1 file changed

Lines changed: 61 additions & 0 deletions

File tree

LATENCY_PR_DESCRIPTION.md

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# PR Description: feat(server): implement LATENCY command set (Redis 7.0+ compatibility)
2+
3+
## Summary
4+
This PR implements the `LATENCY` command for Kvrocks, focusing on the `LATENCY HISTOGRAM` subcommand introduced in Redis 7.0. It leverages the existing histogram infrastructure (`histogram-bucket-boundaries` config + `CommandHistogram` stats) to expose per-command latency distributions via the standard Redis protocol.
5+
6+
## Changes
7+
### New File: `src/commands/cmd_latency.cc`
8+
9+
Implements `CommandLatency` with three subcommands:
10+
11+
- **`LATENCY HISTOGRAM [command ...]`** — Returns a cumulative distribution of command latencies.
12+
- Output uses the Redis 7.0 format: outer map keyed by command name, each value is a map with `calls` (integer) and `histogram_usec` (map of boundary → cumulative count).
13+
- Frequency counts from `CommandHistogram::buckets` are converted to cumulative sums at output time.
14+
- Empty leading buckets are skipped (matching Redis behavior of omitting zero-count ranges).
15+
- The last bucket (overflow) is rendered as `"inf"`.
16+
- Returns an empty map if `histogram-bucket-boundaries` is not configured.
17+
18+
- **`LATENCY RESET [event ...]`** — Returns `(integer) 0`.
19+
- In Redis, this resets the spike-sampling event system (`LATENCY LATEST`/`HISTORY`/`GRAPH`), which Kvrocks does not have. Per Redis docs, histogram data is reset via `CONFIG RESETSTAT`, not `LATENCY RESET`. Returning 0 correctly indicates no event classes were reset.
20+
21+
- **`LATENCY HELP`** — Returns usage information for all supported subcommands.
22+
23+
### Build System
24+
No changes needed. The file is automatically picked up by `file(GLOB_RECURSE KVROCKS_SRCS src/*.cc)` in root `CMakeLists.txt`.
25+
26+
### Command Registration
27+
Registered under `CommandCategory::Server` via `REDIS_REGISTER_COMMANDS(Server, ...)`, same category as `cmd_server.cc`. This is safe because the macro generates unique static variables per translation unit using `__LINE__`.
28+
29+
## Compatibility
30+
31+
| Subcommand | Redis | Kvrocks | Note |
32+
| --- | --- | --- | --- |
33+
| `LATENCY HISTOGRAM [cmd ...]` | 7.0+ | **Yes** | Cumulative distribution, `histogram_usec` field, empty buckets skipped |
34+
| `LATENCY HELP` | 7.0+ | **Yes** | |
35+
| `LATENCY RESET [event ...]` | 2.8.13+ | **Yes** | Always returns 0 (no spike-sampling infrastructure) |
36+
| `LATENCY LATEST` | 2.8.13+ | No | Requires spike-sampling event system |
37+
| `LATENCY HISTORY event` | 2.8.13+ | No | Requires spike-sampling event system |
38+
| `LATENCY GRAPH event` | 2.8.13+ | No | Requires spike-sampling event system |
39+
| `LATENCY DOCTOR` | 2.8.13+ | No | Requires spike-sampling event system |
40+
41+
## Configuration Prerequisite
42+
Histograms are only tracked when `histogram-bucket-boundaries` is set in `kvrocks.conf`. Example:
43+
```
44+
histogram-bucket-boundaries 1,5,10,25,50,100,250,500,1000,2500,5000,10000
45+
```
46+
If empty (default), `LATENCY HISTOGRAM` returns an empty map — this is expected behavior, not an error.
47+
48+
## RESP Protocol Details
49+
- Uses RESP3 Map (`%`) via `conn->HeaderOfMap()`, which auto-degrades to interleaved RESP2 arrays.
50+
- Per-command entry structure:
51+
```
52+
command_name => {
53+
"calls" => (integer) N,
54+
"histogram_usec" => {
55+
boundary_1 => (integer) cumulative_count_1,
56+
boundary_2 => (integer) cumulative_count_2,
57+
...
58+
"inf" => (integer) total_calls
59+
}
60+
}
61+
```

0 commit comments

Comments
 (0)