valkey-benchmark: reject empty command sequence instead of hanging

[dhruv2x]

A repeat count with no command (e.g. valkey-benchmark 10000) builds an empty command sequence but still starts the benchmark. With nothing to send, the event loop spins forever at ~100% CPU printing -nan.

Guard against this by checking seq_len before running: if no command was parsed, print a usage error and exit 1 instead of looping.

For example, before this change:

$ valkey-benchmark -h 127.0.0.1 -p 6379 10000
10000: rps=0.0 (overall: -nan) avg_msec=-nan 0 requests
10000: rps=0.0 (overall: 0.0) avg_msec=-nan 0 requests
^C         # hangs forever at ~100% CPU, never exits

After this change:

$ valkey-benchmark -h 127.0.0.1 -p 6379 10000
No command specified: a repeat count must be followed by a command.
Use -n <requests> to set the total number of requests.
$ echo $?
1

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5912331f-83de-41c8-9fdd-5838d80e2977

📥 Commits

Reviewing files that changed from the base of the PR and between a05a96f and ee93139.

📒 Files selected for processing (2)

• src/valkey-benchmark.c
• tests/integration/valkey-benchmark.tcl

🚧 Files skipped from review as they are similar to previous changes (2)

• tests/integration/valkey-benchmark.tcl
• src/valkey-benchmark.c

---

📝 Walkthrough

Walkthrough

The benchmark now rejects executions where a repeat count is not followed by a command, reports an error, and includes an integration test covering the failure.

Changes

Benchmark command validation

Layer / File(s)	Summary
Reject empty benchmark commands src/valkey-benchmark.c, tests/integration/valkey-benchmark.tcl	The benchmark exits with status 1 when no command is parsed, and the integration test verifies the exit status and repeat-count error message.

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

Possibly related PRs

• valkey-io/valkey#4142: Both changes address repeat counts without a following benchmark command in src/valkey-benchmark.c.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: rejecting an empty command sequence to avoid hanging.
Description check	✅ Passed	The description accurately explains the same bug, fix, and expected behavior, and is clearly related to the changeset.
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.

---

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]

Actionable comments posted: 1

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

Inline comments:
In `@tests/integration/valkey-benchmark.tcl`:
- Around line 119-123: Update the test “benchmark: repeat count without command
errors out cleanly” to capture Tcl’s catch options via the third argument,
inspect the returned -errorcode, and assert the child process status is
explicitly 1 rather than treating catch’s return value as the exit code.
Preserve the existing error-output assertion.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f3404fd4-bdc9-49b7-a567-a17c19e15f44

📥 Commits

Reviewing files that changed from the base of the PR and between 6b006c9 and a05a96f.

📒 Files selected for processing (2)

• src/valkey-benchmark.c
• tests/integration/valkey-benchmark.tcl

[valkey-review-bot]

I found one edge case where the new empty-sequence check is still bypassed. A repeat count followed by a separator is parsed as a literal ; command instead of the intended usage error.

[dhruv2x]

hello @sarthakaggarwal97 can you have a look?