-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Dev/no lock #2308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dev/no lock #2308
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2308 +/- ##
=======================================
Coverage 92.21% 92.21%
=======================================
Files 111 111
Lines 28781 28781
=======================================
Hits 26539 26539
Misses 2242 2242 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
deb55e4
to
ec58410
Compare
As usual, sporadic CI failures with docker. |
bb1ae86
to
8d86e87
Compare
This PR, if accepted, will impact #2295 and make it simpler. |
8d86e87
to
fe816aa
Compare
fe816aa
to
f4e4139
Compare
f4e4139
to
ee84ad1
Compare
🍾 |
Pull Request check-list
Please make sure to review and check all of these items:
$ tox
pass with this change (including linting)?NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Description of change
An
asyncio.Lock
present around theredis.asyncio.Connection._parser.read_response()
is removed. This is not required, since only a single task should be sending/receiving on a singleConnection
at the same time.There is no lock around the
send()
code andWithout a lock around the entire
execute_command()
(send, and receive) there has been no guarantee that a response is returned which matches the sent command, if more than one task are sending.A single unittest for concurrent execution was skipped to not run in single-connection mode. This unittest would have also sporadically failed before this PR if the concurrent commands were different. As it happened, all 10 were the exact same ping command and so mismatching requests/responses were never detected.