Skip to content

Commit

Permalink
Only use ModifiedSince, not TouchedSince, on store.graphLock
Browse files Browse the repository at this point in the history
At least containers#926 suggests that
using timestamps "seems to fail", without elaborating further.

At the very least, ModifiedSince is the more general check, because it
can work across shared filesystems or on filesystems with bad timestamp
granularity, it's about as costly (a single syscall, pread vs. fstat),
and we can now completely eliminate tracking store.lastLoaded.

The more common code shape will also help factor the common code out further.

Signed-off-by: Miloslav Trmač <[email protected]>
  • Loading branch information
mtrmac committed Dec 1, 2022
1 parent 2582470 commit f6914a7
Showing 1 changed file with 12 additions and 7 deletions.
19 changes: 12 additions & 7 deletions store.go
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,6 @@ type store struct {
transientStore bool

// The following fields can only be accessed with graphLock held.
lastLoaded time.Time
graphLockLastWrite lockfile.LastWrite
graphDriver drivers.Driver
layerStore rwLayerStore
Expand Down Expand Up @@ -919,10 +918,14 @@ func (s *store) getGraphDriver() (drivers.Driver, error) {
func (s *store) GraphDriver() (drivers.Driver, error) {
s.graphLock.Lock()
defer s.graphLock.Unlock()
if s.graphLock.TouchedSince(s.lastLoaded) {
lastWrite, modified, err := s.graphLock.ModifiedSince(s.graphLockLastWrite)
if err != nil {
return nil, err
}
if modified {
s.graphDriver = nil
s.layerStore = nil
s.lastLoaded = time.Now()
s.graphLockLastWrite = lastWrite
}
return s.getGraphDriver()
}
Expand All @@ -932,10 +935,14 @@ func (s *store) GraphDriver() (drivers.Driver, error) {
func (s *store) getLayerStore() (rwLayerStore, error) {
s.graphLock.Lock()
defer s.graphLock.Unlock()
if s.graphLock.TouchedSince(s.lastLoaded) {
lastWrite, modified, err := s.graphLock.ModifiedSince(s.graphLockLastWrite)
if err != nil {
return nil, err
}
if modified {
s.graphDriver = nil
s.layerStore = nil
s.lastLoaded = time.Now()
s.graphLockLastWrite = lastWrite
}
if s.layerStore != nil {
return s.layerStore, nil
Expand Down Expand Up @@ -2578,7 +2585,6 @@ func (s *store) mount(id string, options drivers.MountOpts) (string, error) {
if err != nil {
return "", err
}
s.lastLoaded = time.Now()
}

if options.UidMaps != nil || options.GidMaps != nil {
Expand Down Expand Up @@ -2726,7 +2732,6 @@ func (s *store) Diff(from, to string, options *DiffOptions) (io.ReadCloser, erro
if err != nil {
return nil, err
}
s.lastLoaded = time.Now()
}

for _, s := range layerStores {
Expand Down

0 comments on commit f6914a7

Please sign in to comment.