From 2fd87b6a8617f7f9e7d03b23cc2f5a6ca6ad5fb5 Mon Sep 17 00:00:00 2001
From: Ramkumar Chinchani <rchincha@cisco.com>
Date: Fri, 20 Mar 2020 10:56:19 -0700
Subject: [PATCH] pkg/api: use a rwlock when accessing storage

The original patch used a mutex, however, the workload patterns are
likely to be read-heavy, so use a rwlock instead.
---
 pkg/api/routes.go      | 26 ++++++++++++++++++--------
 pkg/storage/storage.go |  4 ++--
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/pkg/api/routes.go b/pkg/api/routes.go
index 02e2722c..24bc5588 100644
--- a/pkg/api/routes.go
+++ b/pkg/api/routes.go
@@ -42,17 +42,27 @@ const (
 
 type RouteHandler struct {
 	c        *Controller
-	blobLock sync.Mutex
+	blobLock sync.RWMutex
 }
 
 func NewRouteHandler(c *Controller) *RouteHandler {
-	rh := &RouteHandler{c: c, blobLock: sync.Mutex{}}
+	rh := &RouteHandler{c: c, blobLock: sync.RWMutex{}}
 	rh.SetupRoutes()
 
 	return rh
 }
 
-func (rh *RouteHandler) blobLockWrapper(f func (w http.ResponseWriter, r *http.Request)) func (w http.ResponseWriter, r *http.Request) {
+// blobRLockWrapper calls the real handler with read-lock held
+func (rh *RouteHandler) blobRLockWrapper(f func(w http.ResponseWriter, r *http.Request)) func(w http.ResponseWriter, r *http.Request) {
+	return func(w http.ResponseWriter, r *http.Request) {
+		rh.blobLock.RLock()
+		f(w, r)
+		rh.blobLock.RUnlock()
+	}
+}
+
+// blobLockWrapper calls the real handler with write-lock held
+func (rh *RouteHandler) blobLockWrapper(f func(w http.ResponseWriter, r *http.Request)) func(w http.ResponseWriter, r *http.Request) {
 	return func(w http.ResponseWriter, r *http.Request) {
 		rh.blobLock.Lock()
 		f(w, r)
@@ -67,23 +77,23 @@ func (rh *RouteHandler) SetupRoutes() {
 		g.HandleFunc(fmt.Sprintf("/{name:%s}/tags/list", NameRegexp.String()),
 			rh.ListTags).Methods("GET")
 		g.HandleFunc(fmt.Sprintf("/{name:%s}/manifests/{reference}", NameRegexp.String()),
-			rh.CheckManifest).Methods("HEAD")
+			rh.blobRLockWrapper(rh.CheckManifest)).Methods("HEAD")
 		g.HandleFunc(fmt.Sprintf("/{name:%s}/manifests/{reference}", NameRegexp.String()),
-			rh.GetManifest).Methods("GET")
+			rh.blobRLockWrapper(rh.GetManifest)).Methods("GET")
 		g.HandleFunc(fmt.Sprintf("/{name:%s}/manifests/{reference}", NameRegexp.String()),
 			rh.blobLockWrapper(rh.UpdateManifest)).Methods("PUT")
 		g.HandleFunc(fmt.Sprintf("/{name:%s}/manifests/{reference}", NameRegexp.String()),
 			rh.blobLockWrapper(rh.DeleteManifest)).Methods("DELETE")
 		g.HandleFunc(fmt.Sprintf("/{name:%s}/blobs/{digest}", NameRegexp.String()),
-			rh.CheckBlob).Methods("HEAD")
+			rh.blobRLockWrapper(rh.CheckBlob)).Methods("HEAD")
 		g.HandleFunc(fmt.Sprintf("/{name:%s}/blobs/{digest}", NameRegexp.String()),
-			rh.GetBlob).Methods("GET")
+			rh.blobRLockWrapper(rh.GetBlob)).Methods("GET")
 		g.HandleFunc(fmt.Sprintf("/{name:%s}/blobs/{digest}", NameRegexp.String()),
 			rh.blobLockWrapper(rh.DeleteBlob)).Methods("DELETE")
 		g.HandleFunc(fmt.Sprintf("/{name:%s}/blobs/uploads/", NameRegexp.String()),
 			rh.blobLockWrapper(rh.CreateBlobUpload)).Methods("POST")
 		g.HandleFunc(fmt.Sprintf("/{name:%s}/blobs/uploads/{session_id}", NameRegexp.String()),
-			rh.GetBlobUpload).Methods("GET")
+			rh.blobRLockWrapper(rh.GetBlobUpload)).Methods("GET")
 		g.HandleFunc(fmt.Sprintf("/{name:%s}/blobs/uploads/{session_id}", NameRegexp.String()),
 			rh.blobLockWrapper(rh.PatchBlobUpload)).Methods("PATCH")
 		g.HandleFunc(fmt.Sprintf("/{name:%s}/blobs/uploads/{session_id}", NameRegexp.String()),
diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go
index 44547317..7da82df1 100644
--- a/pkg/storage/storage.go
+++ b/pkg/storage/storage.go
@@ -35,7 +35,7 @@ type BlobUpload struct {
 // ImageStore provides the image storage operations.
 type ImageStore struct {
 	rootDir     string
-	lock        *sync.Mutex
+	lock        *sync.RWMutex
 	blobUploads map[string]BlobUpload
 	log         zerolog.Logger
 }
@@ -43,7 +43,7 @@ type ImageStore struct {
 // NewImageStore returns a new image store backed by a file storage.
 func NewImageStore(rootDir string, log zlog.Logger) *ImageStore {
 	is := &ImageStore{rootDir: rootDir,
-		lock:        &sync.Mutex{},
+		lock:        &sync.RWMutex{},
 		blobUploads: make(map[string]BlobUpload),
 		log:         log.With().Caller().Logger(),
 	}