fix: refactor monitoring code outside locks (#1596)

We use locks to protect OCI layouts. However, our critical sections have
too long and cover code instead of data.

Fixes issue #1595

Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
This commit is contained in:
Ramkumar Chinchani
2023-07-07 13:51:00 -07:00
committed by GitHub
parent cd6f679359
commit b22989cfe0
2 changed files with 64 additions and 28 deletions
+32 -14
View File
@@ -404,15 +404,23 @@ func (is *ImageStoreLocal) GetImageTags(repo string) ([]string, error) {
// GetImageManifest returns the image manifest of an image in the specific repository.
func (is *ImageStoreLocal) GetImageManifest(repo, reference string) ([]byte, godigest.Digest, string, error) {
var lockLatency time.Time
dir := path.Join(is.rootDir, repo)
if !is.DirExists(dir) {
return nil, "", "", zerr.ErrRepoNotFound
}
var lockLatency time.Time
var err error
is.RLock(&lockLatency)
defer is.RUnlock(&lockLatency)
defer func() {
is.RUnlock(&lockLatency)
if err == nil {
monitoring.IncDownloadCounter(is.metrics, repo)
}
}()
index, err := common.GetIndex(is, repo, is.log)
if err != nil {
@@ -440,8 +448,6 @@ func (is *ImageStoreLocal) GetImageManifest(repo, reference string) ([]byte, god
return nil, "", "", err
}
monitoring.IncDownloadCounter(is.metrics, repo)
return buf, manifestDesc.Digest, manifestDesc.MediaType, nil
}
@@ -457,8 +463,17 @@ func (is *ImageStoreLocal) PutImageManifest(repo, reference, mediaType string, /
var lockLatency time.Time
var err error
is.Lock(&lockLatency)
defer is.Unlock(&lockLatency)
defer func() {
is.Unlock(&lockLatency)
if err == nil {
monitoring.SetStorageUsage(is.metrics, is.rootDir, repo)
monitoring.IncUploadCounter(is.metrics, repo)
}
}()
digest, err := common.ValidateManifest(is, repo, reference, mediaType, body, is.log)
if err != nil {
@@ -584,23 +599,28 @@ func (is *ImageStoreLocal) PutImageManifest(repo, reference, mediaType string, /
}
}
monitoring.SetStorageUsage(is.metrics, is.rootDir, repo)
monitoring.IncUploadCounter(is.metrics, repo)
return desc.Digest, subjectDigest, nil
}
// DeleteImageManifest deletes the image manifest from the repository.
func (is *ImageStoreLocal) DeleteImageManifest(repo, reference string, detectCollision bool) error {
var lockLatency time.Time
dir := path.Join(is.rootDir, repo)
if !is.DirExists(dir) {
return zerr.ErrRepoNotFound
}
var lockLatency time.Time
var err error
is.Lock(&lockLatency)
defer is.Unlock(&lockLatency)
defer func() {
is.Unlock(&lockLatency)
if err == nil {
monitoring.SetStorageUsage(is.metrics, is.rootDir, repo)
}
}()
index, err := common.GetIndex(is, repo, is.log)
if err != nil {
@@ -654,8 +674,6 @@ func (is *ImageStoreLocal) DeleteImageManifest(repo, reference string, detectCol
_ = os.Remove(p)
}
monitoring.SetStorageUsage(is.metrics, is.rootDir, repo)
return nil
}
+32 -14
View File
@@ -315,15 +315,23 @@ func (is *ObjectStorage) GetImageTags(repo string) ([]string, error) {
// GetImageManifest returns the image manifest of an image in the specific repository.
func (is *ObjectStorage) GetImageManifest(repo, reference string) ([]byte, godigest.Digest, string, error) {
var lockLatency time.Time
dir := path.Join(is.rootDir, repo)
if fi, err := is.store.Stat(context.Background(), dir); err != nil || !fi.IsDir() {
return nil, "", "", zerr.ErrRepoNotFound
}
var lockLatency time.Time
var err error
is.RLock(&lockLatency)
defer is.RUnlock(&lockLatency)
defer func() {
is.RUnlock(&lockLatency)
if err == nil {
monitoring.IncDownloadCounter(is.metrics, repo)
}
}()
index, err := common.GetIndex(is, repo, is.log)
if err != nil {
@@ -351,8 +359,6 @@ func (is *ObjectStorage) GetImageManifest(repo, reference string) ([]byte, godig
return nil, "", "", err
}
monitoring.IncDownloadCounter(is.metrics, repo)
return buf, manifestDesc.Digest, manifestDesc.MediaType, nil
}
@@ -368,8 +374,17 @@ func (is *ObjectStorage) PutImageManifest(repo, reference, mediaType string, //n
var lockLatency time.Time
var err error
is.Lock(&lockLatency)
defer is.Unlock(&lockLatency)
defer func() {
is.Unlock(&lockLatency)
if err == nil {
monitoring.SetStorageUsage(is.metrics, is.rootDir, repo)
monitoring.IncUploadCounter(is.metrics, repo)
}
}()
dig, err := common.ValidateManifest(is, repo, reference, mediaType, body, is.log)
if err != nil {
@@ -486,23 +501,28 @@ func (is *ObjectStorage) PutImageManifest(repo, reference, mediaType string, //n
return "", "", err
}
monitoring.SetStorageUsage(is.metrics, is.rootDir, repo)
monitoring.IncUploadCounter(is.metrics, repo)
return desc.Digest, subjectDigest, nil
}
// DeleteImageManifest deletes the image manifest from the repository.
func (is *ObjectStorage) DeleteImageManifest(repo, reference string, detectCollisions bool) error {
var lockLatency time.Time
dir := path.Join(is.rootDir, repo)
if fi, err := is.store.Stat(context.Background(), dir); err != nil || !fi.IsDir() {
return zerr.ErrRepoNotFound
}
var lockLatency time.Time
var err error
is.Lock(&lockLatency)
defer is.Unlock(&lockLatency)
defer func() {
is.Unlock(&lockLatency)
if err == nil {
monitoring.SetStorageUsage(is.metrics, is.rootDir, repo)
}
}()
index, err := common.GetIndex(is, repo, is.log)
if err != nil {
@@ -555,8 +575,6 @@ func (is *ObjectStorage) DeleteImageManifest(repo, reference string, detectColli
}
}
monitoring.SetStorageUsage(is.metrics, is.rootDir, repo)
return nil
}