search

redis / go-redis

Redis Go client
https://redis.uptrace.dev
BSD 2-Clause "Simplified" License
19.95k stars 2.35k forks source link

SetNX returning false for new keys #2985

Open alex-rufo opened 5 months ago

alex-rufo commented 5 months ago

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 :(

wnz27 commented 4 months ago

yes, i encounter this issue

wnz27 commented 4 months ago
截屏2024-05-13 12 38 03 截屏2024-05-13 12 38 16 截屏2024-05-13 12 39 37
zhouguoqionghai commented 1 month ago

I have just meet this issue, too.

zhouguoqionghai commented 1 month ago

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)