Be clearer about the layer store locking rules#2080
Be clearer about the layer store locking rules#2080openshift-merge-bot[bot] merged 1 commit intomainfrom
Conversation
... as modified by #2036 . Also document a LOCKING BUG I don't now care to fix. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
|
LGTM |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, mtrmac The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| // and since the rundir cannot be shared for different stores, it is safe to assume the | ||
| // current process has exclusive access to it. | ||
| // | ||
| // LOCKING BUG? the .DiffSize operation does not currently hold an exclusive lock on the primary store. |
There was a problem hiding this comment.
@mtrmac Is this yet to be verified ? I mean about the comment LOCKING BUG? if yes then maybe adding TODO: here would help someone else to revisit this in spare time.
There was a problem hiding this comment.
I do think it is a locking bug.
It’s not 100% clear to me that it needs to be fixed in entirety — c/image’s “image size” operation uses layer metadata instead of store.ImageSize. There are some callers that use this .DiffSize path, but maybe they could be migrated to the metadata-only implementation. (We would still need to either reimplement, or deprecate, this operation as is…)
But .store.ContainerSize does seem to be impacted, with no obvious alternative implementation.
... as modified by #2036 .
Also document a LOCKING BUG I don't now care to fix.