Today, if calling RM_Call on a blocking command (like blpop or any module command for example that blocks the client), the command is executed but we get a REDISMODULE_REPLY_UNKNOWN as the RM_Call reply and there is no way to get the actual reply once the client is unblocked. I would like to suggest two approaches to solve it:
- Return a new reply type
REDISMODULE_REPLY_BLOCKEDthat allows setting a callback on it that will be called when the client will be unblocked. Maybe something like :RM_SetOnUnblcokedCallback(OnUnblocked callback, void* pd) - Introduce a new
RM_CallAsyncthat will get OnDone callback. The new API will call this callback immediately if the client was not blocked or will return and call the callback when the client will get unblocked.
I am not going into implementation details and I am sure it will not be easy and there is a lot of edge cases to consider (in both approaches). But I would like to hear your thoughts about it before diving into details.
@oranagra @yossigo @itamarhaber @guybe7
Comment From: oranagra
I'm all for it. I would add something that may not be obvious from the above description: Most modules should probably have no real need to call BLPOP they have better ways to achieve that. But modules may want to call XREAD, or more importantly, call a blocking command of another module.
Comment From: yossigo
@MeirShpilraien I agree this is indeed a pain point, and I also agree it's going to have many side affects that should be considered carefully. The issue of refactoring the blocking mechanism was raised a few times in the past and I think that might be a trigger for that.
Comment From: MeirShpilraien
@yossigo @oranagra I will start to go over the code and suggest a detailed implementation, which approach do you prefer between what I suggested (or maybe you have a different idea)?
Comment From: oranagra
@MeirShpilraien i suppose we rather have the normal RM_Call fail (early / somehow) when it attempts to execute a blocking command. since most callers of RM_Call will not expect that state or check for it.
Also, maybe the RM_CallAsync can some day be used for background execution of slow commands (if / when redis will support that).
I might be wrong. would love to hear the opinion of others.
p.s. I have a feeling that when implementing it you'll realize what's right (one implementation will feel harder to code, and harder to use)
Comment From: ushachar
@MeirShpilraien Did you do any additional work towards this?
Comment From: MeirShpilraien
@ushachar no, did not get to it yet.
Comment From: chenyang8094
mark and ref an issue #10307 . Maybe there might be some commonalities between them.