Add redis-proxy: dual-write Redis proxy with pub/sub forwarding#351
Add redis-proxy: dual-write Redis proxy with pub/sub forwarding#351
Conversation
Implement a Redis-protocol proxy that supports dual-write to primary (Redis) and secondary (ElasticKV) backends. Includes full pub/sub message forwarding via redcon Detach + go-redis PubSub. Verified all 581 Misskey unit tests pass through the proxy.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a robust Redis-protocol proxy designed to facilitate the migration of Redis data to an ElasticKV store. It provides a flexible architecture for routing commands, allowing for various stages of migration from simple passthrough to full dual-write with shadow reads. The proxy ensures operational stability through bounded asynchronous operations, comprehensive metrics, and proactive error reporting, making it a critical component for managing data consistency during transitions. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive Redis proxy with dual-write capabilities, pub/sub forwarding, metrics, and error reporting. The overall architecture is well-structured, with clear separation of concerns into different packages for configuration, backends, command handling, and proxy logic. The use of a semaphore for bounding asynchronous operations and the detailed command classification are excellent. However, I've identified a few critical issues related to binary data handling that could lead to data corruption, as well as some opportunities for improving graceful shutdown and simplifying transaction handling logic.
There was a problem hiding this comment.
Pull request overview
Adds a new redis-proxy component to the codebase: a Redis-protocol proxy that routes commands to a primary backend and optionally dual-writes / shadow-reads against a secondary backend, with Prometheus metrics and optional Sentry reporting.
Changes:
- Introduce
proxypackage implementing command classification/routing, dual-write + shadow-read comparison, and Redis protocol response translation. - Add Pub/Sub forwarding support using
redcondetached connections bridged togo-redisPubSub. - Add unit + integration tests plus a
cmd/redis-proxyexecutable with metrics + Sentry wiring.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/redis-proxy/main.go | CLI entrypoint that wires config, backends, metrics endpoint, and proxy server lifecycle. |
| proxy/backend.go | Backend abstraction and go-redis implementation (including optional PubSub factory). |
| proxy/command.go | Command classification table used to decide routing (read/write/admin/blocking/txn/pubsub/script). |
| proxy/compare.go | Shadow-read comparison logic and divergence classification + reporting. |
| proxy/config.go | Proxy configuration and mode parsing/defaults. |
| proxy/dualwrite.go | Dual-write/shadow-read execution logic, async bounding, metrics, and Sentry reporting hooks. |
| proxy/integration_test.go | End-to-end integration tests against real Redis instances (skipped if unavailable). |
| proxy/metrics.go | Prometheus metrics definitions and registration. |
| proxy/proxy.go | Redcon server implementation: command dispatch, txn handling, pubsub session startup, response encoding. |
| proxy/proxy_test.go | Unit tests covering command classification, divergence logic, dual-write behavior, and Sentry cooldown logic. |
| proxy/pubsub.go | Detached pub/sub session implementation bridging client <-> upstream PubSub. |
| proxy/sentry.go | Sentry reporter wrapper with fingerprint-based cooldown and bounded history tracking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- Extract dispatchPubSubCommand to reduce readClientCommands complexity (CC=11→8) - Use metricsSrv.Shutdown for graceful metrics server stop - Guard against nil logger in NewSentryReporter - Cap ShouldReport map after eviction to prevent unbounded growth - Normalize command names with strings.ToUpper in DualWriter methods - Fix goAsync doc comment to match actual behavior - Simplify execTxn by merging duplicate result-handling branches
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Redis-protocol proxy (cmd/redis-proxy) that can route reads/writes across a primary Redis backend and a secondary ElasticKV backend, including optional shadow-read divergence detection, Prometheus metrics, Sentry anomaly reporting, and Pub/Sub forwarding.
Changes:
- Added a redcon-based proxy server with command classification, transaction handling, and Pub/Sub session bridging.
- Implemented dual-write + optional shadow-read comparison logic with Prometheus metrics and Sentry reporting.
- Added unit and integration tests plus a runnable
redis-proxyCLI entrypoint.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/redis-proxy/main.go | CLI entrypoint wiring config, metrics server, Sentry, and proxy server lifecycle. |
| proxy/backend.go | Backend abstraction and go-redis based RedisBackend implementation (incl. pipelining + PubSub creation). |
| proxy/command.go | Redis command classification table for routing (read/write/admin/txn/pubsub/etc.). |
| proxy/compare.go | Shadow-read comparison + divergence classification and reporting. |
| proxy/config.go | Proxy configuration and mode parsing/defaults. |
| proxy/dualwrite.go | Dual-write/shadow-read execution, async bounding, and Sentry/metrics instrumentation. |
| proxy/integration_test.go | Integration tests that run against real Redis instances when available. |
| proxy/metrics.go | Prometheus metrics definitions and registration. |
| proxy/proxy.go | Main redcon proxy server: dispatch/routing, txn support, RESP writing, Pub/Sub session startup. |
| proxy/proxy_test.go | Unit tests covering classification, dual-writer behavior, shadow reader, sentry reporter, and backend defaults. |
| proxy/pubsub.go | Detached-connection Pub/Sub session that forwards messages and handles pubsub-mode commands. |
| proxy/sentry.go | Sentry reporter with cooldown-based de-duplication and flush support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
- Pass []byte directly in bytesArgsToInterfaces instead of converting to string - Handle []byte type assertion in argsToBytes for correct Sentry reporting - Fix responseEqual panic when comparing mismatched interface types - Return false in ShouldReport when at capacity to prevent Sentry flooding
When all subscriptions are removed, the detached connection transitions to normal command mode instead of being stuck in pub/sub-only mode. Clients can then execute regular Redis commands (GET, SET, transactions, etc.) or re-enter pub/sub mode with a new SUBSCRIBE/PSUBSCRIBE. Key changes: - Add respWriter interface so writeResponse works with both Conn and DetachedConn - Refactor pubsubSession with commandLoop that handles both pub/sub and normal modes - Support transactions (MULTI/EXEC/DISCARD) in normal mode on detached connections - Track forwardMessages goroutine lifecycle for clean pub/sub mode transitions - Extract command name constants (MULTI, EXEC, DISCARD, PING, QUIT)
There was a problem hiding this comment.
Pull request overview
Adds a new redis-proxy component to the codebase that speaks the Redis protocol and routes commands to primary/secondary backends to support dual-write migration to ElasticKV, with observability (Prometheus + Sentry) and pub/sub forwarding.
Changes:
- Implement Redis-protocol proxy server (redcon) with command classification, transactions, and backend routing (dual-write + optional shadow reads).
- Add pub/sub bridging using
redcon.Detach+go-redisPubSub and support transitioning back to normal command mode. - Add Prometheus metrics, Sentry reporting/dedup, and accompanying unit + integration tests; introduce
cmd/redis-proxyentrypoint.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/redis-proxy/main.go | New CLI binary wiring config, metrics server, and proxy server lifecycle. |
| proxy/backend.go | Backend abstraction + go-redis implementation, including PubSub capability. |
| proxy/command.go | Command classification table for routing decisions. |
| proxy/compare.go | Shadow-read comparison + divergence classification/metrics/Sentry reporting. |
| proxy/config.go | Proxy configuration + supported operating modes. |
| proxy/dualwrite.go | Core dual-write/shadow-read execution, bounded async work, and metrics/Sentry hooks. |
| proxy/integration_test.go | Optional integration tests against real Redis instances. |
| proxy/metrics.go | Prometheus metric definitions/registration for proxy operations. |
| proxy/proxy.go | redcon server handlers, transaction execution, and RESP response writing helpers. |
| proxy/proxy_test.go | Unit tests covering classification, dual-writer behavior, metrics, and Sentry dedup. |
| proxy/pubsub.go | Pub/sub session implementation with message forwarding and mode switching. |
| proxy/sentry.go | Sentry initialization + cooldown-based dedup for anomaly reporting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- Fix startForwarding race: capture upstream under lock before goroutine start - Reject SUBSCRIBE/PSUBSCRIBE during MULTI transaction to prevent state corruption - Normalize non-redis.Error to "ERR ..." prefix in writeRedisError for valid RESP - Truncate divergence values in Sentry reports to prevent data leakage
- Add PrimaryDB, PrimaryPassword, SecondaryDB, SecondaryPassword to ProxyConfig - Add DB and Password fields to BackendOptions, passed to go-redis client - Add CLI flags: -primary-db, -primary-password, -secondary-db, -secondary-password - Silently accept SELECT and AUTH commands (return OK without forwarding) since DB/auth are configured at the connection-pool level
There was a problem hiding this comment.
Pull request overview
Adds a Redis-protocol proxy service (redis-proxy) that can route reads/writes across two Redis-protocol backends (Redis + ElasticKV), with optional shadow reads for divergence detection and support for pub/sub forwarding via detached redcon connections.
Changes:
- Introduces the proxy server core (command classification/routing, dual-write + shadow-read logic, RESP response writer, transaction handling).
- Adds pub/sub support using
redcon.Detach()+go-redisPubSubforwarding, including a “return to normal mode” when unsubscribed. - Adds observability: Prometheus metrics + optional Sentry reporting, plus unit/integration tests for non-pubsub behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
cmd/redis-proxy/main.go |
CLI entrypoint wiring config, backends, metrics endpoint, and graceful shutdown. |
proxy/backend.go |
Backend abstraction and go-redis implementation (including a dedicated PubSub connection). |
proxy/command.go |
Redis command classification table for routing decisions. |
proxy/config.go |
Proxy config + modes (redis-only, dual-write, shadow, elastickv-primary/only). |
proxy/dualwrite.go |
Dual-writer/shadow-reader execution paths + bounded async work. |
proxy/compare.go |
Shadow-read comparison, divergence classification, and divergence reporting hook. |
proxy/metrics.go |
Prometheus metrics definitions/registration. |
proxy/sentry.go |
Optional Sentry reporter with cooldown-based de-duplication support. |
proxy/proxy.go |
redcon server, RESP writing helpers, command dispatch, and transaction support. |
proxy/pubsub.go |
Detached pub/sub session implementation and upstream message forwarding. |
proxy/proxy_test.go |
Unit tests for core proxy components (classification, dual-write, sentry gating, etc.). |
proxy/integration_test.go |
Integration tests against real Redis instances for basic command flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
… panic Co-authored-by: bootjp <1306365+bootjp@users.noreply.github.com>
Co-authored-by: bootjp <1306365+bootjp@users.noreply.github.com>
Fix matchSecondary window scan, boundary consistency, and SELECT test panic
|
@copilot open a new pull request to apply changes based on the comments in this thread |
…leEntry test Co-authored-by: bootjp <1306365+bootjp@users.noreply.github.com>
fix: deterministic writeUnsubAll ordering + fix MatchSecondaryMiddleEntry test coverage
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // uses a shared connection pool and cannot maintain per-client DB state. | ||
| if name == "SELECT" { | ||
| // Redis arity: SELECT <db>. Require exactly one DB argument. | ||
| if len(args) != 2 { |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
Magic number: 2, in detected (mnd)
| return false | ||
| } | ||
| // Enforce Redis arity: SELECT requires exactly one DB index argument. | ||
| if len(args) != 2 { |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
Magic number: 2, in detected (mnd)
Implement a Redis-protocol proxy that supports dual-write to primary (Redis) and secondary (ElasticKV) backends. Includes full pub/sub message forwarding via redcon Detach + go-redis PubSub.
Verified all 581 Misskey unit tests pass through the proxy.