dragonflydb / dragonflydb/dragonfly

Consolidate RESP parsing and remove RedisParser

Open
#8,196 0 comments 0 reactions 1 assignee Claimed by @BorysTheDev View on GitHub
Dominant language
C++
Stars
31.5k
Forks
1.3k
Avg merge
1d 10h
Merged PRs (30d)
137

Description

## Summary

Dragonfly currently maintains three RESP parsers:

- `RespSrvParser` for inbound server commands
- hiredis-backed `RESPParser` for general replies
- legacy `RedisParser` for replication, migration, and several utilities

The original hiredis migration in #6265 completed the test migration, but the planned replication and migration work was never finished.

## Proposed direction

Keep two purpose-specific parsers:

- Use `RespSrvParser` for flat inbound commands, including the Redis replication stream.
- Use `RESPParser` for general server replies.
- Remove `RedisParser`.

## Required work

- Consider update the vendored hiredis parser to 1.4.1 or include its relevant security fixes.
- Add configurable array, bulk, nesting, and cumulative-memory limits to `RESPParser`.
- Add reliable pending/error state reporting.
- Preserve unread bytes when a RESP reply is followed by RDB or journal data in the same read.
- Migrate `ProtocolClient`, replication handshake, and slot migration replies to `RESPParser`.
- Migrate the Redis replication command stream to `RespSrvParser`.
- Migrate remaining utilities and benchmarks.
- Remove `redis_parser.{h,cc}` and its tests.

Related: #6265, #7768, #8187

## Future consideration

After this migration, evaluate whether a single parser implementation can serve all scenarios. It must support zero-copy command parsing, arbitrary RESP2/RESP3 replies, inline commands, unread-tail preservation, strict safety limits, and no significant performance regression. This is not required for this issue.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.