feat(daemon): avoid multiple initialization #1126
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1126 +/- ##
=======================================
Coverage 75.06% 75.07%
=======================================
Files 108 108
Lines 7800 7803 +3
=======================================
+ Hits 5855 5858 +3
Misses 1380 1380
Partials 565 565
Continue to review full report at Codecov.
|
pkg/v1/daemon/image.go
Outdated
| @@ -99,6 +100,9 @@ func Image(ref name.Reference, options ...Option) (v1.Image, error) { | |||
| } | |||
|
|
|||
| func (i *image) initialize() error { | |||
| i.mu.Lock() | |||
There was a problem hiding this comment.
Instead of a mutex, could we use a sync.Once? This would call tarball.Image only the first time initialize is called, taking into account multiple concurrent calls.
There was a problem hiding this comment.
Sure. As described in the description, we need to keep the err. Is it ok with you?
Note that we need to keep err in the image struct like imageOpener.
There was a problem hiding this comment.
Sorry, I missed that part of the original comment. That seems better to me than a mutex that will usually guard a no-op. Storing the error seems fine too.
There was a problem hiding this comment.
Nice. Thanks for doing this!
edit: I'll wait for @jonjohnsonjr to approve+merge in case he has other comments.
Description
For example, if
LayerByDiffIDofdaemon.Imageis called twice in parallel,initializemight be called twice at the same time. Then,tarball.Imagemight be called twice. It is not efficient, so this PR locksinitialize()before checkingi.tarballImagefor nil so that it will not be initialized twice.Please let me know if you prefer
sync.Once. Note that we need to keeperrin theimagestruct likeimageOpener.go-containerregistry/pkg/v1/daemon/image.go
Line 46 in c3aee49
Related PR
#1121