The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/7822
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
Now that `StorageRemoteDriverNames()` is being used to generate frequently used DB queries, there is a cost implication to calling `SupportedDrivers()` when a particular driver isn't supported and the time taken to discover this is significant.
If a driver load fails then it (correctly) doesn't set its `loaded` boolean to true, meaning that the discovery process occurs on every call to `SupportedDrivers()` for every non-supported driver.
Instead we now cache (negatively - for unsupported drivers) the list of supported drivers, which in turn improves the performance of `StorageRemoteDriverNames()`.
CC @stgraber @freeekanayaka @monstermunchkin
From 57bd5b22ad8102ff21125000a75e262ff6e6ce28 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 27 Aug 2020 16:17:38 +0100
Subject: [PATCH 1/4] lxd/daemon: db.StorageRemoteDriverNames usage
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/daemon.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/daemon.go b/lxd/daemon.go
index 15d43ceaa1..cafe77ef02 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -828,7 +828,7 @@ func (d *Daemon) init() error {
}
// Have the db package determine remote storage drivers
- db.GetRemoteDrivers = storageDrivers.RemoteDriverNames(d.State())
+ db.StorageRemoteDriverNames =
storageDrivers.RemoteDriverNames(d.State())
/* Open the cluster database */
for {
From c64e135e44d0e92e595539591b81ac2ffb82b096 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 27 Aug 2020 16:18:29 +0100
Subject: [PATCH 2/4] lxd/db: StorageRemoteDriverNames usage
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/db/instances.go | 2 +-
lxd/db/instances_test.go | 2 +-
lxd/db/storage_pools.go | 4 ++--
lxd/db/storage_volumes.go | 12 ++++++------
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/lxd/db/instances.go b/lxd/db/instances.go
index 51cc7a0f36..d05267cb40 100644
--- a/lxd/db/instances.go
+++ b/lxd/db/instances.go
@@ -657,7 +657,7 @@ func (c *ClusterTx) GetInstancePool(projectName string,
instanceName string) (st
// as that must always be the same as the snapshot's storage pool.
instanceName, _, _ =
shared.InstanceGetParentAndSnapshotName(instanceName)
- remoteDrivers := GetRemoteDrivers()
+ remoteDrivers := StorageRemoteDriverNames()
// Get container storage volume. Since container names are globally
// unique, and their storage volumes carry the same name, their storage
diff --git a/lxd/db/instances_test.go b/lxd/db/instances_test.go
index 6dcc92fc1a..a2239ae94f 100644
--- a/lxd/db/instances_test.go
+++ b/lxd/db/instances_test.go
@@ -200,7 +200,7 @@ func TestInstanceListExpanded(t *testing.T) {
}
func TestCreateInstance(t *testing.T) {
- db.GetRemoteDrivers = func() []string {
+ db.StorageRemoteDriverNames = func() []string {
return []string{"ceph", "cephfs"}
}
diff --git a/lxd/db/storage_pools.go b/lxd/db/storage_pools.go
index 013547658b..78e351444f 100644
--- a/lxd/db/storage_pools.go
+++ b/lxd/db/storage_pools.go
@@ -80,7 +80,7 @@ func (c *ClusterTx) GetStoragePoolUsedBy(name string)
([]string, error) {
return []interface{}{&vols[i].volName, &vols[i].volType,
&vols[i].projectName, &vols[i].nodeID}
}
- remoteDrivers := GetRemoteDrivers()
+ remoteDrivers := StorageRemoteDriverNames()
s := fmt.Sprintf(`
SELECT storage_volumes.name, storage_volumes.type, projects.name,
storage_volumes.node_id
@@ -898,7 +898,7 @@ func (c *Cluster) isRemoteStorage(poolID int64) (bool,
error) {
return err
}
- isRemoteStorage = shared.StringInSlice(driver,
GetRemoteDrivers())
+ isRemoteStorage = shared.StringInSlice(driver,
StorageRemoteDriverNames())
return nil
})
diff --git a/lxd/db/storage_volumes.go b/lxd/db/storage_volumes.go
index 0517e289e2..f89d6ab1db 100644
--- a/lxd/db/storage_volumes.go
+++ b/lxd/db/storage_volumes.go
@@ -89,7 +89,7 @@ WHERE storage_volumes.type = ?
func (c *Cluster) GetStoragePoolVolumes(project string, poolID int64,
volumeTypes []int) ([]*api.StorageVolume, error) {
var nodeIDs []int
- remoteDrivers := GetRemoteDrivers()
+ remoteDrivers := StorageRemoteDriverNames()
err := c.Transaction(func(tx *ClusterTx) error {
var err error
@@ -187,7 +187,7 @@ func (c *Cluster) storagePoolVolumesGet(project string,
poolID, nodeID int64, vo
func (c *Cluster) storagePoolVolumesGetType(project string, volumeType int,
poolID, nodeID int64) ([]string, error) {
var poolName string
- remoteDrivers := GetRemoteDrivers()
+ remoteDrivers := StorageRemoteDriverNames()
query := fmt.Sprintf(`
SELECT storage_volumes_all.name
@@ -223,7 +223,7 @@ SELECT storage_volumes_all.name
// Returns snapshots slice ordered by when they were created, oldest first.
func (c *Cluster) GetLocalStoragePoolVolumeSnapshotsWithType(projectName
string, volumeName string, volumeType int, poolID int64) ([]StorageVolumeArgs,
error) {
result := []StorageVolumeArgs{}
- remoteDrivers := GetRemoteDrivers()
+ remoteDrivers := StorageRemoteDriverNames()
// ORDER BY id is important here as the users of this function can
expect that the results
// will be returned in the order that the snapshots were created. This
is specifically used
@@ -433,7 +433,7 @@ func storagePoolVolumeReplicateIfCeph(tx *sql.Tx, volumeID
int64, project, volum
}
volumeIDs := []int64{volumeID}
- remoteDrivers := GetRemoteDrivers()
+ remoteDrivers := StorageRemoteDriverNames()
// If this is a ceph volume, we want to duplicate the change across the
// the rows for all other nodes.
@@ -463,7 +463,7 @@ func (c *Cluster) CreateStoragePoolVolume(project,
volumeName, volumeDescription
return -1, fmt.Errorf("Volume name may not be a snapshot")
}
- remoteDrivers := GetRemoteDrivers()
+ remoteDrivers := StorageRemoteDriverNames()
err := c.Transaction(func(tx *ClusterTx) error {
driver, err := tx.GetStoragePoolDriver(poolID)
@@ -525,7 +525,7 @@ func (c *Cluster) storagePoolVolumeGetTypeID(project
string, volumeName string,
}
func (c *ClusterTx) storagePoolVolumeGetTypeID(project string, volumeName
string, volumeType int, poolID, nodeID int64) (int64, error) {
- remoteDrivers := GetRemoteDrivers()
+ remoteDrivers := StorageRemoteDriverNames()
s := fmt.Sprintf(`
SELECT storage_volumes_all.id
From 677f5a20f7f001a13e756e3d7bd241fd2fd9a73d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 27 Aug 2020 16:18:40 +0100
Subject: [PATCH 3/4] lxd/db/storage/pools: Renames GetRemoteDrivers to
StorageRemoteDriverNames for clarity
Easier to relate to linked function.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/db/storage_pools.go | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lxd/db/storage_pools.go b/lxd/db/storage_pools.go
index 78e351444f..e27a3d1e12 100644
--- a/lxd/db/storage_pools.go
+++ b/lxd/db/storage_pools.go
@@ -15,8 +15,8 @@ import (
"github.com/lxc/lxd/shared/api"
)
-// GetRemoteDrivers returns a list of remote storage driver names.
-var GetRemoteDrivers func() []string
+// StorageRemoteDriverNames returns a list of remote storage driver names.
+var StorageRemoteDriverNames func() []string
// GetStoragePoolsLocalConfig returns a map associating each storage pool name
to
// its node-specific config values (i.e. the ones where node_id is not NULL).
From 19693c8154170beb80d5b8cd20436f2e4496b93a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 27 Aug 2020 16:19:41 +0100
Subject: [PATCH 4/4] lxd/storage/drivers/load: Cache supported drivers
So that repeated calls to SupportedDrivers() don't cause driver introspection
every time which can cause slow downs if a storage driver isn't available and
takes time to discover this.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/load.go | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/lxd/storage/drivers/load.go b/lxd/storage/drivers/load.go
index 5e7889fbe0..432048724d 100644
--- a/lxd/storage/drivers/load.go
+++ b/lxd/storage/drivers/load.go
@@ -46,9 +46,18 @@ func Load(state *state.State, driverName string, name
string, config map[string]
return d, nil
}
+// supportedDrivers cache of supported drivers to avoid inspecting the storage
layer every time.
+var supportedDrivers []Info
+
// SupportedDrivers returns a list of supported storage drivers.
func SupportedDrivers(s *state.State) []Info {
- supportedDrivers := make([]Info, 0, len(drivers))
+ // Return cached list if available.
+ if supportedDrivers != nil {
+ return supportedDrivers
+ }
+
+ // Initialise fresh cache and populate.
+ supportedDrivers = make([]Info, 0, len(drivers))
for driverName := range drivers {
driver, err := Load(s, driverName, "", nil, nil, nil, nil)
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
