-
Notifications
You must be signed in to change notification settings - Fork 165
Add optional fencing token minted atomically on Obtain #94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,23 +81,32 @@ func (c *Client) ObtainMulti(ctx context.Context, keys []string, ttl time.Durati | |
| value := token + opt.getMetadata() | ||
| ttlVal := strconv.FormatInt(int64(ttl/time.Millisecond), 10) | ||
|
|
||
| fence := opt.getFence() | ||
| fenceKey := "" | ||
| if fence { | ||
| fenceKey = keys[0] + ":fence" | ||
| } | ||
|
|
||
| var fenceToken int64 | ||
|
|
||
| if err := withRetry(ctx, ttl, opt.getRetryStrategy(), func(ctx context.Context) (bool, error) { | ||
| ok, err := c.obtain(ctx, keys, value, len(token), ttlVal) | ||
| ok, ft, err := c.obtain(ctx, keys, value, len(token), ttlVal, fenceKey) | ||
| if err != nil { | ||
| // any non-nil error from obtain is terminal (transient redis | ||
| // errors are unlikely to clear within a lock TTL and retrying a | ||
| // broken server is futile). | ||
| return true, err | ||
| } | ||
| if ok { | ||
| fenceToken = ft | ||
| return true, nil | ||
| } | ||
| // lock is held by someone else; retryable. | ||
| return false, nil | ||
| }); err != nil { | ||
| return nil, err | ||
| } | ||
| return &Lock{Client: c, keys: keys, value: value, tokenLen: len(token)}, nil | ||
| return &Lock{Client: c, keys: keys, value: value, tokenLen: len(token), fenceToken: fenceToken, fenced: fence}, nil | ||
| } | ||
|
|
||
| // withRetry runs attempt repeatedly until it signals it is done, the retry | ||
|
|
@@ -160,15 +169,21 @@ func withRetry(ctx context.Context, ttl time.Duration, retry RetryStrategy, atte | |
| } | ||
| } | ||
|
|
||
| func (c *Client) obtain(ctx context.Context, keys []string, value string, tokenLen int, ttlVal string) (bool, error) { | ||
| _, err := luaObtain.Run(ctx, c.client, keys, value, tokenLen, ttlVal).Result() | ||
| func (c *Client) obtain(ctx context.Context, keys []string, value string, tokenLen int, ttlVal, fenceKey string) (ok bool, fenceToken int64, err error) { | ||
| res, err := luaObtain.Run(ctx, c.client, keys, value, tokenLen, ttlVal, fenceKey).Result() | ||
| if err != nil { | ||
| if errors.Is(err, redis.Nil) { | ||
| return false, nil | ||
| return false, 0, nil | ||
| } | ||
| return false, err | ||
| return false, 0, err | ||
| } | ||
|
|
||
| // With fencing the reply is the integer token; otherwise it is the "OK" status. | ||
| if token, isInt := res.(int64); isInt { | ||
| return true, token, nil | ||
| } | ||
| return true, nil | ||
|
|
||
| return true, 0, nil | ||
| } | ||
|
|
||
| func (c *Client) randomToken() (string, error) { | ||
|
|
@@ -190,9 +205,11 @@ func (c *Client) randomToken() (string, error) { | |
| // Lock represents an obtained, distributed lock. | ||
| type Lock struct { | ||
| *Client | ||
| keys []string | ||
| value string | ||
| tokenLen int | ||
| keys []string | ||
| value string | ||
| tokenLen int | ||
| fenceToken int64 | ||
| fenced bool | ||
| } | ||
|
|
||
| // Obtain is a short-cut for New(...).Obtain(...). | ||
|
|
@@ -221,6 +238,12 @@ func (l *Lock) Token() string { | |
| return l.value[:l.tokenLen] | ||
| } | ||
|
|
||
| // FenceToken returns the lock's fencing token, or false if it was obtained | ||
| // without Options.Fence. | ||
| func (l *Lock) FenceToken() (int64, bool) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would simplify this and always
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call — done. |
||
| return l.fenceToken, l.fenced | ||
| } | ||
|
|
||
| // Metadata returns the metadata of the lock. | ||
| func (l *Lock) Metadata() string { | ||
| return l.value[l.tokenLen:] | ||
|
|
@@ -297,6 +320,11 @@ type Options struct { | |
| // Token is a unique value that is used to identify the lock. By default, a random tokens are generated. Use this | ||
| // option to provide a custom token instead. | ||
| Token string | ||
|
|
||
| // Fence enables a fencing token, minted on each new acquisition and | ||
| // returned by Lock.FenceToken. Stored at "<key>:fence". | ||
| // Default: disabled. | ||
| Fence bool | ||
| } | ||
|
|
||
| func (o *Options) getMetadata() string { | ||
|
|
@@ -313,6 +341,10 @@ func (o *Options) getToken() string { | |
| return "" | ||
| } | ||
|
|
||
| func (o *Options) getFence() bool { | ||
| return o != nil && o.Fence | ||
| } | ||
|
|
||
| func (o *Options) getRetryStrategy() RetryStrategy { | ||
| if o != nil && o.RetryStrategy != nil { | ||
| return o.RetryStrategy | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one little concern left: I don't like the fact that we are automatically generating key names for the fence key, I always prefer to delegate this to the user as there are sometimes non-trivial implications, e.g. redis cluster or other redis implementations where key names matter. I would therefore suggest:
what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed — auto-generating the key name hides exactly the kind of Cluster/key-placement footgun you're describing, so letting the caller own it is the right call. Done:
Options.FenceKeyis now caller-supplied.I also took it one step further for Cluster safety — the fence key now goes in
KEYS(notARGV), as the last entry when fencing is on, with anARGVflag telling the script to treat it specially. That way Redis validates the slot up front and a misplaced fence key fails withCROSSSLOTbefore the script runs, rather than mid-execution.