In other words, why not allow the PUBLISH command to do this:
PUBLISH CHANNEL MESSAGE_1 MESSAGE_2 ...
Is this done because of performance considerations?
I looked at the code: seems trivial to loop over a bunch of messages, publishing them.
Comment From: hpatro
Hello @dtikhonov-iex ,
This should be do-able in a backward compatible manner. I think the performance would be rather better by publishing more messages in a single call if MULTI/EXEC isn't used. We could also extend the same syntax to SPBULISH.
The other alternative to consider is
PUBLISH CHANNEL MESSAGE [CHANNEL MESSAGE ...]
@madolson / @oranagra Any thoughts ?
Comment From: madolson
I'm generally against making commands take optional arguments that prevents us from adding other behavior in the future. I think the behavior @hpatro outlined is pretty annoying for clients to handle and breaks symmetry with sharded pubsub. I'm in favor of leaving the command as is and letting folks use multi exec.
Comment From: dtikhonov
PUBLISH has not changed since Redis v2: so I think the chances of it being extended with options are not that high.
The API I propose -- PUBLISH CHANNEL MSG1 MSG2... would retain the same return value with the same meaning.
I'd be willing to give this a shot and submit a PR...
Comment From: hpatro
@madolson How about MPUBLISH a new command, something similar to MSET ?
Comment From: madolson
@dtikhonov @hpatro I'm missing the why it's all that useful? We could do some performance testing, and it would probably be a couple of percentage points faster. We strive to keep Redis fairly simple.
Comment From: dtikhonov
@madolson I am migrating a few codebases to Redis cluster. One of the programs accumulates updates and writes them to Redis periodically issuing several HSET, LPUSH, and PUBLISH commands. The original code opens a pipeline and issues the commands. Because the keys are different, I cannot use pipelines. I converted the code to accumulate updates by key and ussie multiple-argument HSET and LPUSH commands.
For PUBLISH, I have to get a connection to an individual node and issue several commands in a pipeline. If it were possible to use a single command to publish several message, my code would be simpler -- so this is why I started my search for such command.
In addition, some testing showed that multi-argument HSET and LPUSH commands in this scenario use much less server CPU time.
Comment From: hpatro
@madolson One benefit I could think of is in cluster mode, if the application could support this API, the high cost incurred from the metadata overhead (#12196) in a cluster bus message could be compensated. We could send multiple message(s) on a single cluster bus message and increase the throughput.
Comment From: hpatro
For PUBLISH, I have to get a connection to an individual node and issue several commands in a pipeline. If it were possible to use a single command to publish several message, my code would be simpler -- so this is why I started my search for such command.
@dtikhonov Why do you need to publish on each node. You could publish on any node and the message would be broadcasted to all of the nodes in Redis cluster. Am I missing something ?
Comment From: dtikhonov
That's what I am doing, @hpatro, you are not missing anything. My code does this:
client = cluster.getRandomNode();
pipeline = client.getPipeline();
for (msg of messages)
pipeline.publish('channel', msg);
pipeline.flush();
A multi-arg PUBLISH would allow me to write the above as
cluster.publish('channel', messages);
Comment From: madolson
@madolson One benefit I could think of is in cluster mode, if the application could support this API, the high cost incurred from the metadata overhead (https://github.com/redis/redis/issues/12196) in a cluster bus message could be compensated. We could send multiple message(s) on a single cluster bus message and increase the throughput.
This is a good point imo. We would need a new type of message though to indicate multiple messages, but it would reduce the overhead quite a bit. We discussed about a different strategy to reduce the overhead through SPUBLISH, but perhaps this is a worthwhile optimization.
Comment From: hpatro
@madolson Shall we look to implement this for both PUBLISH/SPUBLISH as both utilize the cluster bus and the overhead is the same ?
Comment From: dtikhonov
I'd like to give this a shot. I'll probably need a few pointers along the way. What do you say?
Comment From: hpatro
@dtikhonov Sure but let's gather all the details and then proceed for the implementation.
@redis/core-team What are your thoughts on introducing the above new command(s) i.e. MPUBLISH/MSPUBLISH ?
Comment From: madolson
SPUBLISH
I still think we made a mistake and that this should be replicated and not propagated on the clusterbus. I think in Redis 8 we should consider a flag or a breaking change to force this to go through the primaries.
Comment From: madolson
I'm going to say I'm OK with MPUBLISH as a command, that takes the form you described.
Comment From: oranagra
I'm ok with MPUBLISH (multiple messages to one channel). @madolson can you explain your previous comment? IIRC we propagate to slaves in non-cluster and via the cluster bus in cluster mode. What do you suggest to change?
Comment From: hpatro
Thanks @madolson / @oranagra for taking a look. 👍
@oranagra Here is my understanding. Sending message over cluster bus is expensive (overhead) and also the message delivery guarantee is lesser (message(s) can get lost if the link gets disconnected). Sending it via the replication link solves both of these problem i.e. lesser overhead, the replication link data gets buffered and could be resent if the replication link gets disconnected for a short period.
Sharded pubsub channel(s) are restricted to a single shard and could benefit from the above properties of replication link in cluster mode as well. With this approach, we would need to decide if we would still allow SPUBLISH on replica(s) or not. For backward compatibility we could choose to use replication link for message sent on primary and cluster bus link for replicas.
I had added some more thoughts over here https://github.com/redis/redis/issues/12196#issuecomment-1564704087. Maybe it would be good to consolidate and file a new issue for sharded pubsub improvements.
Comment From: madolson
I have nothing further to add beyond what hpatro mentioned.
I had added some more thoughts over here https://github.com/redis/redis/issues/12196#issuecomment-1564704087. Maybe it would be good to consolidate and file a new issue for sharded pubsub improvements.
Yes, let's move the conversation to #12196 if possible.