Several issues on SADD/RPUSH over existing string key

[nightroman]

Describe the bug

Invoking SADD over existing string key unexpectedly works without errors and
seemingly results in two duplicated keys. Then invoking RPUSH operation over
the same key results in GarnetServer Assert and cryptic SERedis client errors.

GarnetServer Assert:

garnet/libs/server/Objects/Set/SetObject.cs

Line 111 in 2dc2c73

Debug.Assert(header->type == GarnetObjectType.Set);

Steps to reproduce the bug

Use the below code or the attached console app project:
GarnetIssue5.zip

using StackExchange.Redis;
using System;
using System.Linq;

using var redis = ConnectionMultiplexer.Connect("127.0.0.1:3278,syncTimeout=600000");
var server = redis.GetServers()[0];
var db = redis.GetDatabase();
const string key = "test:1";

// do StringSet
db.StringSet(key, "v1");

// do SetAdd using the same key
// expected: An error is returned when the value stored at key is not a set. https://redis.io/docs/latest/commands/sadd/
// result: No error and the result is True ~ success
var res1 = db.SetAdd(key, "v2");
Console.WriteLine(res1); // True

// two keys "test:1"
var keys = server.Keys(db.Database, key).ToList();
Console.WriteLine(keys.Count); // 2
foreach (var k in keys) // two "test:1"
    Console.WriteLine(k);

// do ListRightPush using the same key
// expected: When key holds a value that is not a list, an error is returned. https://redis.io/docs/latest/commands/rpush/
// result: Assert in GarnetServer, cryptic errors in SERedis client
db.ListRightPush(key, "v3");

Output: unexpected "True", two duplicated keys "test:1", cryptic SERedis client error

True
2
test:1
test:1
StackExchange.Redis.RedisConnectionException: SocketFailure (ReadSocketError/ConnectionReset, last-recv: 36) on 127.0.0.1:3278/Interactive, Idle/Faulted, last: RPUSH, origin: ReadFromPipe, outstanding: 1, last-read: 2s ago, last-write: 2s ago, unanswered-write: 2s ago, keep-alive: 60s, state: ConnectedEstablished, mgr: 8 of 10 available, in: 0, in-pipe: 0, out-pipe: 0, last-heartbeat: 0s ago, last-mbeat: 0s ago, global: 0s ago, v: 2.7.33.41805
 ---> Pipelines.Sockets.Unofficial.ConnectionResetException: An existing connection was forcibly closed by the remote host.
 ---> System.Net.Sockets.SocketException (10054): An existing connection was forcibly closed by the remote host.
...

Expected behavior

• Object operations over different type keys should result in expected errors.
• Duplicated keys and seemingly inconsistent states should not be produced.

Additional context

• Garnet 1.0.7
• StackExchange.Redis 2.7.33
• Windows 11

[badrishc]

Good finds, thanks!

[badrishc]

Garnet uses two stores internally: a main store to hold raw strings and an object store to hold objects. Thus it's not unexpected to allow the same key in both stores. Changing this will require us to check both stores every time, which is too expensive. We are working on unifying the two indexes but that's not ready yet.

However, we still need to see why the subsequent ListRightPush behaved in an unexpected manner.

[badrishc]

Update: Your second observation (performing a list operation on a set data type) is a duplicate of #218.

[nightroman]

@badrishc Thank you, all makes sense.

We are working on unifying the two indexes but that's not ready yet.

Looking forward to this!