Conversation
Add a standalone single-node Redis C++ client under the repository root redis/, following the same .h + .cpp split as other libhv modules. - RedisMessage: RESP2 encoder/parser, RedisReply/RedisResult model - AsyncRedisClient: async long-connection client with FIFO pending queue, timeout, reconnect and batch command support - RedisClient: sync + async facade with typed helpers (get/set/del/exists/ expire/hget/hset/publish) and raw command interfaces - RedisPipeline / RedisTransaction: batch and MULTI/EXEC/DISCARD support - RedisSubscriber: dedicated pub/sub connection with resubscribe on reconnect Gate the module behind WITH_REDIS (default off), wired across Makefile, CMake and Bazel, and only available when WITH_EVPP is enabled. Add unit tests, example programs and Chinese docs (docs/cn/RedisClient.md).
Fix issues found during code review of the redis module: - RedisMessage: cap array reserve hint (P1). The '*' array parser used the untrusted element count directly in vector::reserve, so a header like '*9999999999999\r\n' could throw length_error/bad_alloc and terminate the process before any element arrived. Reserve a bounded hint and let push_back grow with the actually-received elements. - AsyncRedisClient: disable reconnect on handshake rejection (P1). An AUTH/ SELECT error reply means the password or db is wrong; reconnecting with the same settings produced an endless reconnect + re-auth storm because TcpClient resets its retry counter on every TCP connect. Now setReconnect(NULL) before closing so the failure is reported once. - RedisClient::tokenize: quote-aware splitting for commandf (P1). Previously a value containing whitespace was split into multiple tokens and a wrong command was sent silently. Now supports double/single quotes (redis-cli sdssplitargs style); an unterminated quote yields ERR_INVALID_PARAM. - RedisClient::commandAsync/commandBatchAsync: guard start() with isStarted() (P2), matching the sync path. Avoids a redundant startConnect and a spurious onError(-1) on every async command after the connection is established.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new optional Redis C++ module to libhv (under redis/) providing RESP2 encoding/parsing plus synchronous/asynchronous command APIs, batch helpers (pipeline/transaction), and a dedicated pub/sub subscriber, along with unit tests, examples, and build-system wiring.
Changes:
- Implemented RESP2 command/reply codec and streaming parser (
RedisMessage.*). - Added Redis clients:
AsyncRedisClient,RedisClient,RedisPipeline,RedisTransaction, andRedisSubscriber. - Integrated Redis into Makefile/CMake/Bazel builds, plus unit tests, examples, and documentation updates.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| unittest/redis_test_server.h | Declares a fake Redis TCP server utility for unit tests. |
| unittest/redis_test_server.cpp | Implements a minimal RESP2 server for tests, including pub/sub push support. |
| unittest/redis_subscriber_test.cpp | Adds unit tests for RedisSubscriber subscribe/unsubscribe and message delivery. |
| unittest/redis_protocol_test.cpp | Adds unit tests for RESP2 encode/decode semantics and parser recovery behavior. |
| unittest/redis_client_test.cpp | Adds unit tests for RedisClient sync/async commands and typed helpers. |
| unittest/redis_batch_test.cpp | Adds unit tests for pipeline/transaction behavior and error handling. |
| unittest/CMakeLists.txt | Adds CMake unittest targets for Redis tests when WITH_EVPP && WITH_REDIS. |
| scripts/unittest.sh | Ensures runtime can locate libhv and runs Redis unit tests when built. |
| redis/RedisTransaction.h | Declares transaction helper API. |
| redis/RedisTransaction.cpp | Implements MULTI/EXEC batching and local DISCARD behavior. |
| redis/RedisSubscriber.h | Declares pub/sub subscriber API and callbacks. |
| redis/RedisSubscriber.cpp | Implements subscriber connection/handshake, resubscribe, and message dispatch. |
| redis/RedisPipeline.h | Declares pipeline helper API. |
| redis/RedisPipeline.cpp | Implements batched command execution (sync/async). |
| redis/RedisMessage.h | Declares RESP2 command/reply types and streaming parser interface. |
| redis/RedisMessage.cpp | Implements RESP2 encoding and incremental parsing with recovery. |
| redis/RedisClient.h | Declares high-level sync client + typed helpers + pipeline/transaction factories. |
| redis/RedisClient.cpp | Implements sync wrappers over AsyncRedisClient and helper/tokenization logic. |
| redis/AsyncRedisClient.h | Declares async FIFO Redis client API and events. |
| redis/AsyncRedisClient.cpp | Implements async request queueing, handshake, timeouts, and reply matching. |
| README.md | Documents Redis feature and basic build/test commands and example links. |
| README-CN.md | Adds Redis feature/docs references in Chinese README. |
| Makefile.vars | Adds Redis headers list for installation/export. |
| Makefile | Wires Redis sources/headers/examples/unittests behind WITH_REDIS. |
| examples/redis_subscriber_test.cpp | Adds a Redis subscriber example program. |
| examples/redis_client_test.cpp | Adds a Redis client example program. |
| examples/CMakeLists.txt | Adds Redis examples when WITH_EVPP && WITH_REDIS. |
| examples/BUILD.bazel | Adds Bazel targets for Redis examples gated by with_redis. |
| docs/PLAN.md | Updates plan to reflect Redis client availability. |
| docs/cn/RedisClient.md | Adds Chinese documentation for the new Redis APIs and semantics. |
| docs/cn/README.md | Adds a link to the new Chinese RedisClient doc. |
| configure | Adds --with-redis help entry (works with existing --with-* parsing). |
| config.ini | Adds WITH_REDIS default (disabled). |
| CMakeLists.txt | Adds WITH_REDIS option and wires redis sources/headers when enabled. |
| cmake/vars.cmake | Adds REDIS_HEADERS CMake variable list. |
| BUILD.md | Documents building with --with-redis. |
| BUILD.bazel | Adds with_redis config setting and includes redis headers/srcs when enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+567
to
+585
| int AsyncRedisClient::commandBatch(const std::vector<RedisCommand>& commands, RedisRepliesCallback cb) { | ||
| if (commands.empty()) { | ||
| return ERR_INVALID_PARAM; | ||
| } | ||
| auto request = std::make_shared<Impl::PendingRequest>(); | ||
| request->expected_replies = commands.size(); | ||
| request->batch_callback = std::move(cb); | ||
| for (size_t i = 0; i < commands.size(); ++i) { | ||
| if (commands[i].empty()) { | ||
| return ERR_INVALID_PARAM; | ||
| } | ||
| request->payload += RedisEncodeCommand(commands[i]); | ||
| } | ||
| int ret = impl_->enqueueRequest(request); | ||
| if (ret != 0) { | ||
| impl_->invokeRequestCallback(request, ret); | ||
| } | ||
| return ret; | ||
| } |
Comment on lines
+362
to
+366
| void handleHandshakeReply(const RedisReply& reply) { | ||
| if (reply.isError()) { | ||
| handleClientError(ERR_RESPONSE); | ||
| return; | ||
| } |
Comment on lines
+317
to
+328
| else if (ch == '\'') { | ||
| // single-quoted: literal, '' inside means a single quote char | ||
| ++i; | ||
| while (i < n && line[i] != '\'') { | ||
| token.push_back(line[i]); | ||
| ++i; | ||
| } | ||
| if (i >= n) { | ||
| return RedisCommand(); // unterminated quote | ||
| } | ||
| ++i; // consume closing quote | ||
| } |
…, single-quote tokenizer
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.