[lxc-devel] [lxd/master] lxd/daemon: Properly cache the storage information

[stgraber on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 502 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20171218/e6167720/attachment.bin>
-------------- next part --------------
From 0d6db8981f2a4bba2b57f6aec419276b09fe454c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 18 Dec 2017 01:55:32 -0500
Subject: [PATCH] lxd/daemon: Properly cache the storage information
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This moves all the data we need into the cache so that we don't cause
any filesystem related calls when we received "GET /1.0".

Closes #4025

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/api_1.0.go             | 13 +++----------
 lxd/storage.go             | 42 ++++++++++++++++++++++++++++--------------
 lxd/storage_pools_utils.go | 23 +++--------------------
 3 files changed, 34 insertions(+), 44 deletions(-)

diff --git a/lxd/api_1.0.go b/lxd/api_1.0.go
index ab48696c8..4c18d2459 100644
--- a/lxd/api_1.0.go
+++ b/lxd/api_1.0.go
@@ -123,13 +123,7 @@ func api10Get(d *Daemon, r *http.Request) Response {
 		ServerVersion:          version.Version}
 
 	drivers := readStoragePoolDriversCache()
-	for _, driver := range drivers {
-		// Initialize a core storage interface for the given driver.
-		sCore, err := storageCoreInit(driver)
-		if err != nil {
-			continue
-		}
-
+	for driver, version := range drivers {
 		if env.Storage != "" {
 			env.Storage = env.Storage + " | " + driver
 		} else {
@@ -137,11 +131,10 @@ func api10Get(d *Daemon, r *http.Request) Response {
 		}
 
 		// Get the version of the storage drivers in use.
-		sVersion := sCore.GetStorageTypeVersion()
 		if env.StorageVersion != "" {
-			env.StorageVersion = env.StorageVersion + " | " + sVersion
+			env.StorageVersion = env.StorageVersion + " | " + version
 		} else {
-			env.StorageVersion = sVersion
+			env.StorageVersion = version
 		}
 	}
 
diff --git a/lxd/storage.go b/lxd/storage.go
index aaf581c0a..4a651d70e 100644
--- a/lxd/storage.go
+++ b/lxd/storage.go
@@ -63,17 +63,16 @@ func getCustomUmountLockID(poolName string, volumeName string) string {
 
 // Simply cache used to storage the activated drivers on this LXD instance. This
 // allows us to avoid querying the database everytime and API call is made.
-var storagePoolDriversCacheInitialized bool
 var storagePoolDriversCacheVal atomic.Value
 var storagePoolDriversCacheLock sync.Mutex
 
-func readStoragePoolDriversCache() []string {
+func readStoragePoolDriversCache() map[string]string {
 	drivers := storagePoolDriversCacheVal.Load()
 	if drivers == nil {
-		return []string{}
+		return map[string]string{}
 	}
 
-	return drivers.([]string)
+	return drivers.(map[string]string)
 }
 
 // storageType defines the type of a storage
@@ -854,6 +853,12 @@ func SetupStorageDriver(s *state.State, forceCheck bool) error {
 		}
 	}
 
+	// Update the storage drivers cache in api_1.0.go.
+	storagePoolDriversCacheUpdate(s.DB)
+	return nil
+}
+
+func storagePoolDriversCacheUpdate(dbNode *db.Node) {
 	// Get a list of all storage drivers currently in use
 	// on this LXD instance. Only do this when we do not already have done
 	// this once to avoid unnecessarily querying the db. All subsequent
@@ -863,18 +868,27 @@ func SetupStorageDriver(s *state.State, forceCheck bool) error {
 	// copy-on-write semantics without locking in the read case seems
 	// appropriate. (Should be cheaper then querying the db all the time,
 	// especially if we keep adding more storage drivers.)
-	if !storagePoolDriversCacheInitialized {
-		tmp, err := s.DB.StoragePoolsGetDrivers()
-		if err != nil && err != db.NoSuchObjectError {
-			return nil
-		}
 
-		storagePoolDriversCacheLock.Lock()
-		storagePoolDriversCacheVal.Store(tmp)
-		storagePoolDriversCacheLock.Unlock()
+	drivers, err := dbNode.StoragePoolsGetDrivers()
+	if err != nil && err != db.NoSuchObjectError {
+		return
+	}
+
+	data := map[string]string{}
+	for _, driver := range drivers {
+		// Initialize a core storage interface for the given driver.
+		sCore, err := storageCoreInit(driver)
+		if err != nil {
+			continue
+		}
 
-		storagePoolDriversCacheInitialized = true
+		// Grab the version
+		data[driver] = sCore.GetStorageTypeVersion()
 	}
 
-	return nil
+	storagePoolDriversCacheLock.Lock()
+	storagePoolDriversCacheVal.Store(data)
+	storagePoolDriversCacheLock.Unlock()
+
+	return
 }
diff --git a/lxd/storage_pools_utils.go b/lxd/storage_pools_utils.go
index 1059d3765..6df16c870 100644
--- a/lxd/storage_pools_utils.go
+++ b/lxd/storage_pools_utils.go
@@ -259,13 +259,7 @@ func dbStoragePoolCreateAndUpdateCache(db *db.Node, poolName string, poolDescrip
 	}
 
 	// Update the storage drivers cache in api_1.0.go.
-	storagePoolDriversCacheLock.Lock()
-	drivers := readStoragePoolDriversCache()
-	if !shared.StringInSlice(poolDriver, drivers) {
-		drivers = append(drivers, poolDriver)
-	}
-	storagePoolDriversCacheVal.Store(drivers)
-	storagePoolDriversCacheLock.Unlock()
+	storagePoolDriversCacheUpdate(db)
 
 	return id, nil
 }
@@ -273,24 +267,13 @@ func dbStoragePoolCreateAndUpdateCache(db *db.Node, poolName string, poolDescrip
 // Helper around the low-level DB API, which also updates the driver names
 // cache.
 func dbStoragePoolDeleteAndUpdateCache(db *db.Node, poolName string) error {
-	pool, err := db.StoragePoolDelete(poolName)
+	_, err := db.StoragePoolDelete(poolName)
 	if err != nil {
 		return err
 	}
 
 	// Update the storage drivers cache in api_1.0.go.
-	storagePoolDriversCacheLock.Lock()
-	drivers := readStoragePoolDriversCache()
-	for i := 0; i < len(drivers); i++ {
-		if drivers[i] == pool.Driver {
-			drivers[i] = drivers[len(drivers)-1]
-			drivers[len(drivers)-1] = ""
-			drivers = drivers[:len(drivers)-1]
-			break
-		}
-	}
-	storagePoolDriversCacheVal.Store(drivers)
-	storagePoolDriversCacheLock.Unlock()
+	storagePoolDriversCacheUpdate(db)
 
 	return err
 }