go-redis icon indicating copy to clipboard operation
go-redis copied to clipboard

SetNX returning false for new keys

Open alex-rufo opened this issue 9 months ago • 4 comments

The existing client already incorporates support for retrials. Should an error occur during execution and it's not a readTimeout, the command undergoes retrying within the _process function. However, there are scenarios where retrying may not be appropriate. Take, for instance, the case where a connection closes during a SetNX operation in Redis. In such instances, we cannot know with absolute certainty if the element was successfully inserted or not. Retrying in this case could lead to misleading outcomes. For instance, if we retry, the key might already exist, causing Redis to return false. This could potentially mislead consumers into believing that the keys weren't present before executing the command.

Expected Behavior

To enhance reliability, consider making the SetNX call idempotent. This ensures that even if it's retried, the outcome remains consistent. If achieving idempotence isn't feasible, it's crucial to signal an error when we cannot ascertain the behavior of SetNX with absolute certainty. This approach fosters clarity and reliability in handling potential retries, safeguarding against ambiguous or misleading outcomes.

Current Behavior

In some cases, SetNX returns false even when no key was previously defined.

Steps to Reproduce

Unfortunately, there is no easy way for me to reproduce the issue :(

alex-rufo avatar Apr 29 '24 13:04 alex-rufo

yes, i encounter this issue

wnz27 avatar May 13 '24 04:05 wnz27

截屏2024-05-13 12 38 03 截屏2024-05-13 12 38 16 截屏2024-05-13 12 39 37

wnz27 avatar May 13 '24 04:05 wnz27

I have just meet this issue, too.

zhouguoqionghai avatar Aug 13 '24 11:08 zhouguoqionghai

After reading the source code, I found where the problem is. baseClient.process method send the client command to redis server and read the response. Redis server will normal exec the command and send the reponse, if due to some exception happended and it's a network problem. baseClient.process will retry, that's where the problem is. For other command, retry may have no side effect, but it will make the return code of setnx no longer reliable any more. retry := shouldRetry(err, atomic.LoadUint32(&retryTimeout) == 1)

zhouguoqionghai avatar Aug 15 '24 03:08 zhouguoqionghai