Conversation
|
This change is |
90c69aa to
7499f51
Compare
Add `syncutil.CondMutex`.
One of a big problems around `sync.Cond` is that waiting for condition
to become true does not respect context cancellation.
This makes `sync.Cond` unusable in all but a handful situations.
`CondMutex` solves this problem by implementing conditional mutex:
```
data := struct {
mu struct {
CondMutex
n int
}
}{}
data.LockWhen(func() bool { data.mu.n > 0 })
```
In addition to locking when condition becomes true, CondMutex
respects context completion via `LockWhenCtx` method:
```
if err := data.LockWhenCtx(ctx, func() bool { data.mu.n > 100 }); err != nil {
return errors.New("never got 100 items")
}
// Lock is held here
defer data.Unlock()
```
Epic: None
Release note: None
knz
left a comment
There was a problem hiding this comment.
I am not keen to introduce a new abstraction in syncutil until we have a clear use case for it.
Do you have some examples from the CockroachDB source code where we would be able to use this advantageously?
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @miretskiy)
pkg/util/syncutil/condition.go line 151 at r1 (raw file):
}() for {
This looks like a CPU-intensive busy-loop. What mechanism is implemented to prevent CPU spikes here if the writer on the other side holds the lock longer?
miretskiy
left a comment
There was a problem hiding this comment.
i'm not pushing to add this ... just something I played with.
I've recently seen some potential use cases. For example, an object that performs async initialization and that has
callers wait for initialization to finish -- usually accomplished with a channel that gets closed.
Reviewable status:
pkg/util/syncutil/condition.go line 151 at r1 (raw file):
Previously, knz (Raphael 'kena' Poss) wrote…
This looks like a CPU-intensive busy-loop. What mechanism is implemented to prevent CPU spikes here if the writer on the other side holds the lock longer?
I don't think so; it goes to sleep if the condition is not true.
|
Yevgeniy Miretskiy seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Add
syncutil.CondMutex.One of a big problems around
sync.Condis that waiting for conditionto become true does not respect context cancellation.
This makes
sync.Condunusable in all but a handful situations.CondMutexsolves this problem by implementing conditional mutex:In addition to locking when condition becomes true, CondMutex
respects context completion via
LockWhenCtxmethod:Epic: None
Release note: None