Add new MPUBLISH command #1657
Add new MPUBLISH command #1657
Conversation
There was a problem hiding this comment.
I'll add some refs first.
redis have this pending issue: [QUESTION] Why not allow PUBLISH to publish multiple messages?
and this pending PR: How MPUBLISH could work #12267
|
@enjoy-binbin Thank you for the links. |
|
I guess I should consider introducing a new command for this instead of changing the |
Agree with it |
|
Do you already want to use this feature? One case is we can pipeline it in this monment (Waiting for Redis to actually implement it if not in a hurry). or implementing |
|
Thanks, @enjoy-binbin @torwig Yes, variant |
torwig
changed the title
|
Implemented the feature as a separate command |
| redis::PubSub pubsub_db(svr->storage); | ||
|
|
||
| auto s = pubsub_db.Publish(args_[1], args_[i]); | ||
| if (!s.ok()) { |
There was a problem hiding this comment.
Seems that Publish will be likely to success?
| total_receivers += receivers; | ||
| } | ||
|
|
||
| *output = redis::Integer(total_receivers); |
There was a problem hiding this comment.
Would an array of int be better here?
There was a problem hiding this comment.
@mapleFU Possibly, yes. However, according to this PR (redis/redis#12267) it's likely that MPUBLISH will have a single integer as a response.
There was a problem hiding this comment.
Oh this LGTM. Seems this is not stable. Maybe we can mark "mpublish" as "experimental" or something, and make it able to change the syntax of it.
There was a problem hiding this comment.
In general, I think it's not easy to utilize the return value of the PUBLISH command, except to check whether it is 0 or more. The actual goal of Pub/Sub pattern is to decouple publishers from subscribers and if I want to check the number of receivers I should know the exact number of them. And for example, if I know that there should be 5 subscribers but I received only 4 in response to the PUBLISH command, what should I do?
There was a problem hiding this comment.
Sounds ok. It's ok to leave it like this
There was a problem hiding this comment.
This patch general LGTM. I've some tiny problems.
Seems pubsub_db.Publish different messages separately. It's ok if all write works well. However, could user find how many message is successed when Publish failed?
Another is that this calls a look for each arg. Could we make mpublish faster, and batch the notifying later? Would this save some time?
Return Integer reply: the number of clients that received the message. Note that in a cluster, only clients that are connected to the same node as the publishing client are included in the count.
The new `MPUBLISH` command allows publishing one or more messages to a channel. Syntax: `MPUBLISH channel-name message1 message2 ... messageN`
The new
MPUBLISHcommand allows publishing one or more messages to a channel.Syntax:
MPUBLISH channel-name message1 message2 ... messageN