Skip to content

add inline redis protocol support#3024

Merged
wwbmmm merged 7 commits into
apache:masterfrom
thweetkomputer:feat-support-inline-command
Jul 25, 2025
Merged

add inline redis protocol support#3024
wwbmmm merged 7 commits into
apache:masterfrom
thweetkomputer:feat-support-inline-command

Conversation

@thweetkomputer

@thweetkomputer thweetkomputer commented Jul 10, 2025

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number:

Problem Summary:

Add inline redis protocol support. Many redis benchmark tools use inline protocol, such as redis_benchmark, dfly_bench, although inline protocol is not guaranteed to be support for a redis-compatible database.

What is changed and the side effects?

Changed:

Side effects:

  • Performance effects:

There will be no overhead for commands following normal protocol (RESP2).

  • Breaking backward compatibility:

No compatiblity issues should be introduced unless someone replies on brpc to reject inline commands.


Check List:

@thweetkomputer

Copy link
Copy Markdown
Contributor Author

@chenBright Hi, could you please restart the CI, I see all the failures happened when installing dependencies.

@wwbmmm

wwbmmm commented Jul 14, 2025

Copy link
Copy Markdown
Contributor

@thweetkomputer Thanks for your contribution! Would you please add some unit tests for this feature?

@thweetkomputer

thweetkomputer commented Jul 15, 2025

Copy link
Copy Markdown
Contributor Author

@thweetkomputer Thanks for your contribution! Would you please add some unit tests for this feature?

Hi @wwbmmm , thanks for replying, I added the test.

@wwbmmm

wwbmmm commented Jul 15, 2025

Copy link
Copy Markdown
Contributor

LGTM

Comment thread src/brpc/redis_command.cpp Outdated
@thweetkomputer

Copy link
Copy Markdown
Contributor Author

Thanks @chenBright , I fixed it.

@thweetkomputer thweetkomputer marked this pull request as draft July 24, 2025 04:47
@thweetkomputer thweetkomputer marked this pull request as ready for review July 24, 2025 05:28

@chenBright chenBright left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thweetkomputer

Copy link
Copy Markdown
Contributor Author

Hi, how can this PR be merged?

@wwbmmm wwbmmm merged commit 353f2d0 into apache:master Jul 25, 2025
15 checks passed
wwbmmm pushed a commit that referenced this pull request Aug 2, 2025
* add inline redis protocol support

* complete code

* add check

* fix

* add inline unitest

* use find

* fix
chenBright pushed a commit that referenced this pull request Jun 14, 2026
Inline redis protocol support (#3024) accepts any alpha-leading line as
an inline command. The HTTP/2 client connection preface
("PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n") is alpha-leading, so it was parsed
as an inline command instead of returning PARSE_ERROR_TRY_OTHERS. This
prevented protocol auto-detection from falling through to HTTP/2, and
gRPC clients calling a server with redis_service enabled failed with
"connection closed before server preface received".

Detect the HTTP/2 preface (fully, or as a not-yet-complete prefix) in
the inline branch of RedisCommandParser::ConsumeImpl and defer to other
protocols. A command-name blacklist is not viable because some HTTP
methods are also valid redis commands (e.g. GET), whereas the preface
is a fixed magic string that no redis command begins with, making the
check unambiguous.

Add RedisTest.inline_does_not_eat_h2_preface covering the full preface,
a partial prefix, and a genuine inline command sharing the leading byte.

Co-authored-by: rajvarun77 <287367605+rajvarun77@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants