kvserver: cache the storeClockTimestamp every tick #142037
Conversation
![blathers[crl]](https://avatars.githubusercontent.com/in/59700?s=40&u=373d9101a12edc3847d2fb6ae78e3adcfb4ca41e&v=4){kind=avatar}
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
iskettaneh
force-pushed
the
contention
branch
from
c748438 to
f91524f
Compare
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @arulajmani)
-- commits line 4 at r1:
nit: Maybe r.store.Clock().NowAsClockTimestamp() to be more precise here (instead of time now()).
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status:
pkg/kv/kvserver/replica_raft.go line 1474 at r1 (raw file):
storeClockTimestamp := r.store.Clock().NowAsClockTimestamp() // Update lastTickTimestamp so that we don't have to redo the work multiple
nit: consider expanding this comment to say something about mutex contention we avoid. Maybe even specifically giving the example of SupportExpired will help.
iskettaneh
force-pushed
the
contention
branch
from
f91524f to
8dcb331
Compare
Instead of calculating the r.store.Clock().NowAsClockTimestamp() multiple times during the tick, this commit caches timestamp during the tick(), and then it gets reused instead of redoing the work again. Preliminary testing showed that this reduces the clock mutex contention by 50%. Fixes: cockroachdb#142049 Release note: None
iskettaneh
force-pushed
the
contention
branch
from
8dcb331 to
2471bb8
Compare
|
TFTR! bors r+ |
Instead of calculating the time now() multiple times during the tick. This commit caches timestamp during the tick(), and then it gets reused instead of redoing the work again.
Preliminary testing showed that this reduces the clock mutex contention by 50%.
Fixes: #142049
Release note: None