For KeyIsLocked error reported for timeout, if a lock is recently updated, don't try to resolve it. #758
Conversation
Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
ekexium
force-pushed
the
skip-resolve-lock
branch
from
1c7c201 to
0ea9651
Compare
Signed-off-by: ekexium <[email protected]>
|
PTAL @MyonKeminta @cfzjywxk . Performance test reveals that the PR introduces slight performance regression. But I haven't figured it out. |
| // This should only happen for wait timeout. | ||
| if lockInfo := keyErr.GetLocked(); lockInfo != nil && | ||
| lockInfo.DurationToLastUpdateMs > 0 && | ||
| lockInfo.DurationToLastUpdateMs < skipResolveThresholdMs { |
There was a problem hiding this comment.
The default wait time in TiKV is 1s and the threshold is 1.2s, it seems that it will never resolve lock if the lock state have ever updated, and the threshold doesn't take any effect.
I think a proper threshold can depend on the actual wait timeout in TiKV, such as something like 0.3*waitTimeout.
There was a problem hiding this comment.
The 1 second default value is a setting in TiKV and may change without letting client-go know. Did you mean simply setting the threshold to 300ms?
I don't have a clear idea on what an optimal strategy would look like. For the hot-update workload, I think both of the strategies work fine.
There was a problem hiding this comment.
Maybe 300ms is fine as the 1 second would not be changed in most cases.
Is there any situation that this setting that would impact performance a lot? I don't come up with one yet.
There was a problem hiding this comment.
I've verified 300ms threshold in the hot-update workload. It works.
Signed-off-by: ekexium <[email protected]>
|
@MyonKeminta PTAL again |
ref