StackExchange / StackExchange.Redis

General purpose redis client
https://stackexchange.github.io/StackExchange.Redis/
Other
5.86k stars 1.5k forks source link

LockTakeAsync return true for same key under 2 sessions #2588

Closed htzhang2 closed 8 months ago

htzhang2 commented 8 months ago

App: Net core 6.0 apis Library: StackExchange.Redis 2.2.62 Env: Service Fabric cluster Resources: Azure redis cache

Code:

string lockKey = "abc-123"; bool lockAcquired = await this.failOverRedisCache.StringSetAsync( lockKey, lockKey, TimeSpan.FromSeconds(5), When.NotExists) .ConfigureAwait(false);

Expected: One node return true, another node return false

Actual: Both nodes return true

mgravell commented 8 months ago

The title says LockTakeAsync, but the body shows StringSetAsync (with When.NotExists); this is possibly a moot distinction, but can you clarify which you're actually seeing a problem with?

Are the two app/client instances configured identically? In particular, does either/both have a key prefix configured? And if so: are they the same? Ditto database numbers? And perhaps more generally: are you sure they're connecting to the same redis instance? And were they at the same time - is it possible this was simply a timeout scenario on the expiration (5 seconds)?

If you take the lock separately with each node, is it visible with GET abc-123 in redis-cli? (which is readily available via azure management, I believe).

Note: for your convenience in investigating this, it may be more convenient to use something unique about the client instance as the value - for example the machine name if they are on different machines - so that you can see who is holding the lock at any particular time.

mgravell commented 8 months ago

Note also: 2.2.62 is from July 2021, and is marked as deprecated. Our first advice is always going to be "update the library version to current". I'm not going to recall if there was a specific issue in that version (there probably wasn't), but: we can't do anything to change that version, so...

mgravell commented 8 months ago

this needs missing detail. Closing as non-actionable