Skip to content

TLS Support for redis-benchmark#7959

Merged
oranagra merged 1 commit intoredis:unstablefrom
filipecosta90:redis.benchmark.openssl
Oct 28, 2020
Merged

TLS Support for redis-benchmark#7959
oranagra merged 1 commit intoredis:unstablefrom
filipecosta90:redis.benchmark.openssl

Conversation

@filipecosta90
Copy link
Contributor

@filipecosta90 filipecosta90 commented Oct 25, 2020 •
edited
Loading

This PR kicks off TLS support for redis-benchmark. When possible, it uses the same code as redis-cli namely benchmarkSecureConnection and writeConn.
Additionally it adds the tls variation to the redis-benchmark tests. Both secure/unsecure tests working as expected:

secure test variation

$ tclsh tests/test_helper.tcl --tls --single integration/redis-benchmark 
Cleanup: may take some time... OK
Starting test server at port 11111
[ready]: 673644
Testing integration/redis-benchmark
[ready]: 673645
[ready]: 673646
[ready]: 673650
[ready]: 673647
[ready]: 673649
[ready]: 673648
[ready]: 673654
[ready]: 673652
[ready]: 673653
[ready]: 673651
[ready]: 673656
[ready]: 673655
[ready]: 673657
[ready]: 673658
[ready]: 673659
[ok]: benchmark: set,get
[ok]: benchmark: full test suite
[ok]: benchmark: multi-thread set,get
[ok]: benchmark: pipelined full set,get
[ok]: benchmark: arbitrary command
[ok]: benchmark: keyspace length
[1/1 done]: integration/redis-benchmark (1 seconds)

                   The End

Execution time of different units:
  1 seconds - integration/redis-benchmark

\o/ All tests passed without errors!

Cleanup: may take some time... OK

unsecure test variation

$ tclsh tests/test_helper.tcl --single integration/redis-benchmark 
Cleanup: may take some time... OK
Starting test server at port 11111
[ready]: 673772
Testing integration/redis-benchmark
[ready]: 673771
[ready]: 673773
[ready]: 673774
[ready]: 673775
[ready]: 673776
[ready]: 673777
[ready]: 673778
[ready]: 673779
[ready]: 673780
[ready]: 673781
[ready]: 673782
[ready]: 673783
[ready]: 673784
[ready]: 673785
[ready]: 673786
[ok]: benchmark: set,get
[ok]: benchmark: full test suite
[ok]: benchmark: multi-thread set,get
[ok]: benchmark: pipelined full set,get
[ok]: benchmark: arbitrary command
[ok]: benchmark: keyspace length
[1/1 done]: integration/redis-benchmark (1 seconds)

                   The End

Execution time of different units:
  1 seconds - integration/redis-benchmark

\o/ All tests passed without errors!

Cleanup: may take some time... OK

madolson
madolson reviewed Oct 27, 2020
View reviewed changes
Copy link
Contributor

@madolson madolson left a comment

Choose a reason for hiding this comment

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

Just a high level question. I saw somewhere else that we wanted to refactor redis-benchmark so it was more deeply integrated with hiredis, does anyone else remember that thought?

oranagra
oranagra reviewed Oct 27, 2020
View reviewed changes
@filipecosta90 filipecosta90 force-pushed the redis.benchmark.openssl branch from 6944029 to 1b70587 Compare October 27, 2020 19:25
oranagra
oranagra reviewed Oct 27, 2020
View reviewed changes
@filipecosta90 filipecosta90 force-pushed the redis.benchmark.openssl branch from cd83d1f to ae349a4 Compare October 27, 2020 23:01
@oranagra oranagra merged commit 39436b2 into redis:unstable Oct 28, 2020
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Nov 4, 2020
@filipecosta90
TLS Support for redis-benchmark (redis#7959)
583dae6
@ssinghaldev
Copy link

ssinghaldev commented Nov 26, 2020 •
edited
Loading

Does this add support for cluster mode too or wait it previously there (for TLS connections)?

@oranagra oranagra added the release-notes indication that this issue needs to be mentioned in the release notes label Dec 9, 2020
@oranagra oranagra mentioned this pull request Jan 13, 2021
Merged
@c2nes c2nes mentioned this pull request Aug 13, 2021
Closed
@ivanstosic-janea ivanstosic-janea mentioned this pull request Feb 4, 2022
Merged
oranagra pushed a commit that referenced this pull request Feb 7, 2022
@ivanstosic-janea
Fix protocol error caused by redis-benchmark (#10236)
bb87560 
The protocol error was caused by the buggy `writeHandler` in `redis-benchmark.c`,
which didn't handle one of the cases, thereby repeating data, leading to protocol errors
when the values being sent are very long.

This PR fixes #10233, issue introduced by #7959
oranagra pushed a commit that referenced this pull request Apr 27, 2022
@ivanstosic-janea @oranagra
Fix protocol error caused by redis-benchmark (#10236)
0a26869 
The protocol error was caused by the buggy `writeHandler` in `redis-benchmark.c`,
which didn't handle one of the cases, thereby repeating data, leading to protocol errors
when the values being sent are very long.

This PR fixes #10233, issue introduced by #7959

(cherry picked from commit bb87560)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oranagra oranagra oranagra approved these changes

@yossigo yossigo Awaiting requested review from yossigo

@madolson madolson Awaiting requested review from madolson

Assignees

No one assigned

Labels

release-notes indication that this issue needs to be mentioned in the release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@filipecosta90 @ssinghaldev @yossigo @oranagra @madolson