Bug: Subscriber client crash when listen to '__redis__:invalidate' messages queue.

[GT185076]

Found in :
https://github.com/StackExchange/StackExchange.Redis/issues/1461
Redis ver 6.2.6 (on docker's container)
Stackexchange ver : 2.2.79 / 2.5.61 ( on dotnet core 3.1 )

Lets say we have a subscriber for invalidate keys queue : (to implement near cache)

var subscriber = invalidator.GetSubscriber();
subscriber.Subscribe(" _ _ redis _ _ :invalidate").OnMessage(message =>
{
Console.WriteLine($"redis:invalidate: {message}");
});

We never get messages came from values changes or expired (we do get messages came from explicit publish call.)

When looking deeper we can see that the client id of the subscriber change after setting new values of tracking keys.. meaning it's was crash..

I got this : System.InvalidCastException: 'Cannot convert to RedisValue: MultiBulk' .
in this method :
Internal RedisValue AsRedisValue()

[mgravell]

Can I assume this pairs with #1461 (comment)?

Presumably, then, client-side caching is breaking all the rules expected of pub/sub, which has historically always been a single value (per publish), which is what the code expects. If client-side caching internally broadcasts a multi-bulk, I'd say this isn't a "bug", it is a "feature request" - and a non-trivial one. We can't just hook that into the existing API, unless we just say "meh, well just take the first value" (or whatever gives the key). To support this properly requires a bespoke API focused on client-side cache support. Which would be a great thing to add, and we've been planning to add it: but it isn't necessarily trivial, as shown here.

[GT185076]

Thanks for the quick replay ..
You need to look on in from my side, I didn't do nothing wrong, I just trying to listen to a messages queue (by the book) and I got no response at all.
It's working fine in redis-cli and there is no alternative in your comment.
We agree that Redis change there message type to multi-bulk and stackExchange did not close this gap yet.
For me this is too fundamental feature to leave it without a treatment.

[mgravell]

It isn't a question of leaving it without treatment; it is a question of where the time comes from. Everything takes time and effort.

Note: I will readily accept, as a bug, the connection crash; we should fix that. However, the immediate "fix" for that is likely to be "silently ignore anything that isn't a simple-string or bulk-string", so: if this API is broadcasting a multi-bulk, it won't actually allow you to listen to the events.

[GT185076]

By the way, listen to keyspace channel work fine.. what is the different?
Appropriate fix can be to generate several events according to numbers of bulks arrived , this way you don't break the interface.
tnks

[mgravell]

what is the different?

presumably these system notifications are - uniquely - broadcasting a multi-bulk rather than a simple-string/bulk-string; indeed, if we look at this doc, we see the response:

*3
$7
message
$20
__redis__:invalidate
*1
$3
foo

the important bit is the last 3 lines, *1, $3, foo - which is an array of length one that has a string, rather than simply being a string. Now; if the array is always length 1 with a string value: I don't mind changing the code to accept that, which means you would just see "foo". The question in my head is: is the array always length 1?

Appropriate fix can be to generate several events according to numbers of bulks arrived

It isn't as simple as that; if we did that, we'd be very brittle in terms of your code understanding where values start and end; if it is always length 1: this is a non-problem.

Suggestion: for now, we check:

• if it is a simple value (the expected/normal case): surface that
• if it is an array, and it is length 1 with a simple value: surface that
• otherwise: ignore it

[GT185076]

Tnks , its work !

[mgravell]

@GT185076 just to be sure I understand your comment: is that "that works for me; sounds like a plan!", or is it "I've built that locally / downloaded the myget, and can confirm that it now works!" ?

[baytekink]

By the way, listen to keyspace channel work fine.. what is the different? Appropriate fix can be to generate several events according to numbers of bulks arrived , this way you don't break the interface. tnks

What do you mean by "listen to keyspace channel"? Can you post a working sample code. Thx

[mgravell]

What do you mean by "listen to keyspace channel"?

Presumably this means: the other inbuilt system broadcasts for keyspace notifications work successfully, because they push bulk-strings, not multi-bulks.

[GT185076]

@GT185076 just to be sure I understand your comment: is that "that works for me; sounds like a plan!", or is it "I've built that locally / downloaded the myget, and can confirm that it now works!" ?

I downloaded from myget and test it , I also clone the repository and look on the code .

[GT185076]

What do you mean by "listen to keyspace channel"?

Presumably this means: the other inbuilt system broadcasts for keyspace notifications work successfully, because they push bulk-strings, not multi-bulks.

Correct.

[GT185076]

By the way, listen to keyspace channel work fine.. what is the different? Appropriate fix can be to generate several events according to numbers of bulks arrived , this way you don't break the interface. tnks

What do you mean by "listen to keyspace channel"? Can you post a working sample code. Thx

Here (need to remove spaces in the channel name) :

`ConnectionMultiplexer conn = ConnectionMultiplexer.Connect("localhost");
ISubscriber subscriber = conn.GetSubscriber();
subscriber.Subscribe(" _ _ keyspace@0 _ _ :*", (channel, value) =>
{
string key = channel.ToString().Split(":")[1];

switch (value)
{
case "set":
Console.WriteLine("SET " + key);
break;
case "expired":
Console.WriteLine("EXPIRED " + key);
break;
}
});`