Option to skip NetworkStream for local subscribers

[lukas-ais]

Is there an option to optimize the subscription notification (Publish) for recipients in the same application process?

According to my analysis of the recent version (1.2.6) the notification (Publish) is always send using a NetworkStream, also for subscribers in the same process.

Do you think, an option would be possible to skip using NetworkStream within same process? Instead the receiver should get the message directly.

[mgravell]

The problem here is that arbitrary messages don't include any kind of sender or message identity. That means that if we did this, there would be no way of knowing which messages to ignore (because we sent them ourselves and have already raised them). There are also possible ordering issues to consider

On Wed, 22 Aug 2018, 09:21 Lukas G (AIS), notifications@github.com wrote:

Is there an option to optimize the subscription notification (Publish) for recipients in the same application process?

According to my analysis of the recent version (1.2.6) the notification (Publish) is always send using a NetworkStream, also for subscribers in the same process.

Do you think, an option would be possible to skip using NetworkStream within same process? Instead the receiver should get the message directly.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/StackExchange/StackExchange.Redis/issues/924, or mute the thread https://github.com/notifications/unsubscribe-auth/AABDsGOi-9LapHkRM_adbD1D0V2WDXsRks5uTRSjgaJpZM4WHNLC .

[lukas-ais]

Sure, I do not know the internal structures. But I'm wondering a bit about the problems you mentioned.

Just to be sure I want to explain my idea a little bit more concrete close to the software structure I've identified during the debugging.

Here is the callstack of sending the message to a stream:

StackExchange.Redis.dll!StackExchange.Redis.PhysicalConnection.WriteRaw(System.IO.Stream stream, long value, bool withLengthPrefix) Line 500
StackExchange.Redis.dll!StackExchange.Redis.PhysicalConnection.WriteHeader(StackExchange.Redis.RedisCommand command, int arguments) Line 474
StackExchange.Redis.dll!StackExchange.Redis.Message.CommandChannelValueMessage.WriteImpl(StackExchange.Redis.PhysicalConnection physical) Line 836
StackExchange.Redis.dll!StackExchange.Redis.PhysicalBridge.WriteMessageToServer(StackExchange.Redis.PhysicalConnection connection, StackExchange.Redis.Message message) Line 800
StackExchange.Redis.dll!StackExchange.Redis.PhysicalBridge.WriteMessageDirect(StackExchange.Redis.PhysicalConnection tmp, StackExchange.Redis.Message next) Line 555
StackExchange.Redis.dll!StackExchange.Redis.PhysicalBridge.WriteQueue(int maxWork) Line 600
StackExchange.Redis.dll!StackExchange.Redis.SocketManager.WriteAllQueues() Line 419
StackExchange.Redis.dll!StackExchange.Redis.SocketManager..cctor.AnonymousMethod__53_0(object context) Line 107

The callstack for the receive part looks like this:

StackExchange.Redis.dll!StackExchange.Redis.MessageCompletable.TryComplete(bool isAsync) Line 33
StackExchange.Redis.dll!StackExchange.Redis.CompletionManager.ProcessAsyncCompletionQueueImpl() Line 163
StackExchange.Redis.dll!StackExchange.Redis.CompletionManager.ProcessAsyncCompletionQueue(object state) Line 111
StackExchange.Redis.dll!StackExchange.Redis.CompletionManager.CompleteSyncOrAsync(StackExchange.Redis.ICompletable operation) Line 51
StackExchange.Redis.dll!StackExchange.Redis.ConnectionMultiplexer.OnMessage(StackExchange.Redis.RedisChannel subscription, StackExchange.Redis.RedisChannel channel, StackExchange.Redis.RedisValue payload) Line 81
StackExchange.Redis.dll!StackExchange.Redis.PhysicalConnection.MatchResult(StackExchange.Redis.RawResult result) Line 913
StackExchange.Redis.dll!StackExchange.Redis.PhysicalConnection.ProcessBuffer(byte[] underlying, ref int offset, ref int count) Line 979
StackExchange.Redis.dll!StackExchange.Redis.PhysicalConnection.ProcessReadBytes(int bytesRead) Line 1001
StackExchange.Redis.dll!StackExchange.Redis.PhysicalConnection.StackExchange.Redis.ISocketCallback.Read() Line 1028
StackExchange.Redis.dll!StackExchange.Redis.SocketManager.ProcessItems(System.Collections.Generic.Queue<StackExchange.Redis.ISocketCallback> queue, StackExchange.Redis.SocketManager.CallbackOperation operation) Line 57
StackExchange.Redis.dll!StackExchange.Redis.SocketManager.ProcessItems(bool setState) Line 111
StackExchange.Redis.dll!StackExchange.Redis.SocketManager.ReadImpl() Line 367
StackExchange.Redis.dll!StackExchange.Redis.SocketManager.Read() Line 124
StackExchange.Redis.dll!StackExchange.Redis.SocketManager..cctor.AnonymousMethod__53_3(object state) Line 26

(FYI: In CompletionManager.CompleteSyncOrAsync I replaced the ThreadPool.QueueUserWorkItem call by direct call of ProcessAsyncCompletionQueue for better understanding the communication.)

In both stacks the class PhysicalConnection is active. So I assume, there is a relation between these two instances of PhysicalConnection, isn't it? Further I assume, that the filtering you mentioned will not be performed on the level of a physical connection. This is why I (still) think, that it should be possible to hand over the message from one physical connection to the other one without serializing the message.

[mgravell]

Yes, handing over the message without serializing it is trivial. That isn't the problem. The problem is not trying to handle it twice - once immediately, and once when it comes back from the server broadcast. We still need to send it to the server so that other clients get the message. There is currently no mechanism that would allow us to avoid receiving it and knowing - reliably - that we are the connection that sent it. That is a topic I will raise on the server GitHub (it seems to be a common request).

[NickCraver]

Agree we shouldn't do this because of ordering alone, but duplication is inherently unsolvable without a major breaking behavior change on messages.

[mgravell]

Also, it isn't just a breaking change - any "fix" we added (by adding payload rules) would break compatibility with other redis libraries. I really don't want to do that.

On Thu, 23 Aug 2018, 22:40 Nick Craver, notifications@github.com wrote:

Agree we shouldn't do this because of ordering alone, but duplication is inherently unsolvable without a major breaking behavior change on messages.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/StackExchange/StackExchange.Redis/issues/924#issuecomment-415580038, or mute the thread https://github.com/notifications/unsubscribe-auth/AABDsLlB-WXp32vhozEFJnmPTHUbWBccks5uTyFKgaJpZM4WHNLC .

[NickCraver]

@lukas-ais I think there are a lot of reasons not to do this, and not nearly enough justification to do it. It would just cause unavoidable problems.

If you still need such functionality, I'd recommend doing it before calling the StackExchange.Redis library. You could call your own handler directly instead of a publish, or do both, or whatever combination you wanted. But since there's no way for us to gracefully handle any of the issues raised above in any generic way, this isn't a piece of functionality that belongs in the library layer.

[mgravell]

xref: https://github.com/antirez/redis/issues/5289

[mgravell]

I'm going to close this out as something that we're not going to do without server support; however, if we get server support, I'll absolutely revisit this.