Fix Race Condition in Memcached Store, Close Loadable to Stop Setter Routine #100
Conversation
Fix setWithTags test, add Close method to stop setter goroutine
| if err = s.addKeyToTagValue(tagKey, key); err == nil { | ||
| return nil | ||
| } | ||
| // loop to retry any failure (including race conditions) |
There was a problem hiding this comment.
I think race conditions should not be handled like this. Could you please use a mutex instead?
There was a problem hiding this comment.
@eko sorry i shouldve specified what the race condition is. its a distributed race condition, because multiple clients can update the same key at any time
previously the setTags code was using the Set operation to update tags. the problem with this is that if client1 issues a Set after client2 has read the contents of the same key via Get. client2 will miss the update issued by client1. this means client2 will overwrite what client1 wrote and the update is lost.
to avoid this, synchronization is done via memcached's optimistic concurrency control mechanisms. using CompareAndSwap with the item returned from a previous Get ensures that the value has not changed since. if the item has changed, an error is returned. thus why addKeyToTagValue is retried
There was a problem hiding this comment.
Ok, thank you for clarifying this! So let's use this 3-times retry mechanism :)
| if err = s.addKeyToTagValue(tagKey, key); err == nil { | ||
| return nil | ||
| } | ||
| // loop to retry any failure (including race conditions) |
There was a problem hiding this comment.
Ok, thank you for clarifying this! So let's use this 3-times retry mechanism :)
Summary