Skip to content

When the password is wrong, redis-benchmark should exit#8855

Merged
oranagra merged 1 commit intoredis:unstablefrom
yangbodong22011:fix-redis-benchmark
Apr 25, 2021
Merged

When the password is wrong, redis-benchmark should exit#8855
oranagra merged 1 commit intoredis:unstablefrom
yangbodong22011:fix-redis-benchmark

Conversation

@yangbodong22011
Copy link
Contributor

When the password is wrong, the benchmark should not be started, otherwise we will get the wrong qps information.

@oranagra oranagra linked an issue Apr 25, 2021 that may be closed by this pull request
@oranagra oranagra merged commit 8423b77 into redis:unstable Apr 25, 2021
@oranagra
Copy link
Member

for the record, it does print:

ERROR: NOAUTH Authentication required.
ERROR: failed to fetch CONFIG from 127.0.0.1:6379
WARN: could not fetch server CONFIG

@filipecosta90
Copy link
Contributor

for the record, it does print:

ERROR: NOAUTH Authentication required.
ERROR: failed to fetch CONFIG from 127.0.0.1:6379
WARN: could not fetch server CONFIG

@oranagra I believe this will make redis-benchmark to stop working on RE, given we can't fetch the config. If I'm correct we might need to address this on a different manner...

@oranagra
Copy link
Member

@filipecosta90 thanks for pointing this out.
I suppose we must distinguish between fatal and non fatal errors.
Can you make a PR to improve?

@filipecosta90
Copy link
Contributor

@filipecosta90 thanks for pointing this out.
I suppose we must distinguish between fatal and non fatal errors.
Can you make a PR to improve?

Will do @oranagra 👍 . will have it open today

@yangbodong22011
Copy link
Contributor Author

I believe this will make redis-benchmark to stop working on RE, given we can't fetch the config. If I'm correct we might need to address this on a different manner...

Sorry for not paying attention to the existing issues #5854 , these discussions should have been completed in the previous issue.

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.

Need to add auth check in benchmark?

3 participants