-
Notifications
You must be signed in to change notification settings - Fork 23.6k
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
[BUG] Calling RedisModule_SignalKeyAsReady from a timer callback does not trigger the reply callback. #7880
Comments
@manast it seems you pass let me know if the issue is still happening |
I updated the test code, it is still happening. |
i still see
instead of
|
not anymore :). I just forgot to update that in the issue, in my code it is correct. |
but as I mentioned, if you push before creating the timer then the call to signalKeysAsReady works, so the pointer must be correct, the problem is when you push from inside the timer AND you call the signalKeyAsReady from inside the timer. |
ok so indeed there's a bug: the function that releases blocked clients (handleClientsBlockedOnKeys) is called only from processCommand (i.e. the assumption is that only a command can cause a key to be "ready" - but that assumption seems to be an over-simplification) we can either call handleClientsBlockedOnKeys at the end of moduleTimerHandler or every once in a while in serverCron (seems a bit more risky) @oranagra WDYT? |
i think this was indeed overlooked, and i guess we should call handleClientsBlockedOnKeys from beforeSleep too. |
Fix redis#7879 redis#7880 1. Fix some comments 2. Make sure blocked-on-keys client privdata is accessible from withing the timeout callback 3. Handle blocked clients in beforeSleep - In case a key becomses "ready" outside of processCommand
Fix redis#7879 redis#7880 1. Fix some comments 2. Make sure blocked-on-keys client privdata is accessible from withing the timeout callback 3. Handle blocked clients in beforeSleep - In case a key becomses "ready" outside of processCommand
handled vie the above mentioned PR. |
- Clarify some documentation comments - Make sure blocked-on-keys client privdata is accessible from withing the timeout callback - Handle blocked clients in beforeSleep - In case a key becomes "ready" outside of processCommand See redis#7879 redis#7880 (cherry picked from commit addf47d)
- Clarify some documentation comments - Make sure blocked-on-keys client privdata is accessible from withing the timeout callback - Handle blocked clients in beforeSleep - In case a key becomes "ready" outside of processCommand See redis#7879 redis#7880 (cherry picked from commit addf47d)
- Clarify some documentation comments - Make sure blocked-on-keys client privdata is accessible from withing the timeout callback - Handle blocked clients in beforeSleep - In case a key becomes "ready" outside of processCommand See redis#7879 redis#7880
- Clarify some documentation comments - Make sure blocked-on-keys client privdata is accessible from withing the timeout callback - Handle blocked clients in beforeSleep - In case a key becomes "ready" outside of processCommand See redis#7879 redis#7880 (cherry picked from commit addf47d)
Describe the bug
When using
RedisModule_BlockClientOnKeys
it should be possible to unblock the client by callingRedisModule_SignalKeyAsReady
. However this is not working if the call is performed inside the callback of a timer.The following code reproduces the issue. Note that for RedisModule_SignalKeyAsReady to work at all, the key we are signalling must exist before we call it as reported here (#7878)
To reproduce
Just compile and load that module in redis, call it with for example "bug2 1234". It should create a list named 1234 with one element "foobar" in it. However the command is not unblocked after 1 second.
Expected behavior
The command should unblock one second after running it.
Additional information
This issue is reproducible with Redis 6.0.8.
Some observations. If the call to RPUSH is moved outside of the timer callback, then the command will unblock the first time it is called, but not the second time (as if it detects the LIST was already there and then ignore the signalling).
The text was updated successfully, but these errors were encountered: