-
Notifications
You must be signed in to change notification settings - Fork 990
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
fix: unblock transactions only if requirements are correct #2345
Conversation
src/server/blocking_controller.cc
Outdated
do { | ||
WatchItem& wi = queue.front(); | ||
Transaction* head = wi.get(); | ||
for (auto wi_it = queue.begin(), wi_e = queue.end(); wi_it != wi_e; ++wi_it) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as I said before, I would like to preserve a simple and efficient case of lists, sorted sets notifying a single waiter and stop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't change the behaviour)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't get your message at first. let's check how this implementation works. I can implement your suggestion regarding different queues for list, zset and stream like an optimization,
489bfd9
to
54b891b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approve the approach, just some style comments 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
cf15451
to
72976f7
Compare
fixes #2294 bug: we unblock XREADGROUP cmd even if we don't have new values fix: added check with custom requirements for blocking comands
72976f7
to
d368aed
Compare
fixes #2294
bug: we unblocked XREADGROUP cmd even if we don't have new values
fix: added check with custom requirements for blocking commands