reader-writer file locks #278
Conversation
vrothberg
force-pushed
the
rw-lock
branch
3 times, most recently
from
ad9b760
to
7702c69
Compare
vrothberg
force-pushed
the
rw-lock
branch
3 times, most recently
from
2375bf5
to
8dc3973
Compare
|
Nice work. |
|
@vrothberg One thing you might want to try, would be to vendor this into CRI-O in a test build and then fire off the tests against it. Might give us a better feeling that we will not have issues. |
|
For the record, I opened the following CI PRs to check for potential regressions: |
|
Podman and Buildah are happy, CRI-O seems to be constrained by CI flakes. |
vrothberg
force-pushed
the
rw-lock
branch
5 times, most recently
from
e706e38
to
e7d00a0
Compare
|
Alright. I fixed some issues when using @rhatdan @nalind @giuseppe Can you have a look at the three commits of this PR? |
|
Podman and Buildah are green. CRI-O is green but the e2e tests which have a regression from upstream (as all other PRs for CRI-O). |
|
LGTM |
|
Good catch! We also need the locking loop fix added to |
Deferring method calls on loop variables must be avoided by all means as
the calls will be invoked on the last item of the loop.
The intermediate fix used in this commit is to allocate a new variable
on the heap for each loop iteration. An example transformation is:
FROM:
for _, x := range x_slice {
x.Lock()
defer x.Unlock()
}
TO:
for _, x_itr := range x_slice {
x := x_itr
x.Lock()
defer x.Unlock()
}
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Implement reader-writer locks to allow allow multiple readers to hold the lock in parallel. * The locks are still based on fcntl(2). * Changing the lock from a reader to a writer and vice versa will block on the syscall. * A writer lock can be held only by one process. To protect against concurrent accesses by gourtines within the same process space, use a writer mutex. * Extend the Locker interface with the `RLock()` method to acquire a reader lock. If the lock is set to be read-only, all calls to `Lock()` will be redirected to `RLock()`. A reader lock is only released via fcntl(2) when all gourtines within the same process space have unlocked it. This is done via an internal counter which is protected (among other things) by an internal state mutex. * Panic on violations of the lock protocol, namely when calling `Unlock()` on an unlocked lock. This helps detecting violations in the code but also protects the storage from corruption. Doing this has revealed some bugs fixed in ealier commits. Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Thanks for the review, @nalind! I updated the commits accordingly and repushed. |
Enable executing parallel `GetBlob()` executions in containers/image by using reader-lock acquisitions in `ImageBigData()` and `Diff()`. Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
|
LGTM |
ready for review :^)
Signed-off-by: Valentin Rothberg rothberg@redhat.com