new lockDuplicateRequest #186
Conversation
| "time" | ||
| ) | ||
|
|
||
| type key struct { |
There was a problem hiding this comment.
This is the global namespace, probably best to use a name like lockDuplicateKey or similar.
| } | ||
|
|
||
| var newValue value | ||
| parse, _ := time.ParseDuration("+5s") |
There was a problem hiding this comment.
It'd be much more efficient to use a constant that is set to 5 * time.Second here rather than parsing a string in every call.
| type value struct { | ||
| //If expiredTimeStamp is negative number , it means it will return SERVFAIL. | ||
| expiredTimeStamp int64 | ||
| mu *newMutex |
There was a problem hiding this comment.
Why re-implement the mutex here? Can't you use the regular mutex instead and pass a *value below?
There was a problem hiding this comment.
If timeout, this plugin should unlock-all same request and then return SERVFAIL to downstream.
Tradition Mutex can not support it.
| r.m.Delete(k) | ||
| } else if expiredTimeStamp >= 0 && expiredTimeStamp < currentTimeStamp { | ||
| r.m.Delete(k) | ||
| v.expiredTimeStamp = ^expiredTimeStamp |
There was a problem hiding this comment.
Since v is not locked here, I don't think this is thread-safe.
Regardless, instead of doing these calculations, using a channel that is closed to signal "timeout" would be better. Could even use a context with timeout in this case. Or simply add a (fixed) timeout into the value, then check it with https://pkg.go.dev/time#Time.After against the current time.
There was a problem hiding this comment.
Yes, It is not thread-safe.
I prefer timeTicker.
#185