Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize database calls on storagePoolVolumesGet #13349

Merged
merged 4 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions lxd/db/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,11 @@ func TestImportPreClusteringData(t *testing.T) {
err = c.Transaction(context.TODO(), func(ctx context.Context, tx *db.ClusterTx) error {
volumeType := cluster.StoragePoolVolumeTypeImage
filters := []db.StorageVolumeFilter{{
Type: &volumeType,
Type: &volumeType,
PoolID: &id,
tomponline marked this conversation as resolved.
Show resolved Hide resolved
}}

dbVolumes, err = tx.GetStoragePoolVolumes(context.Background(), id, true, filters...)
dbVolumes, err = tx.GetStorageVolumes(context.Background(), true, filters...)
if err != nil {
return fmt.Errorf("Failed loading storage volumes: %w", err)
}
Expand Down
35 changes: 23 additions & 12 deletions lxd/db/storage_volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,12 @@ WHERE storage_volumes.id = ?
return response, nil
}

// StorageVolumeFilter used for filtering storage volumes with GetStoragePoolVolumes().
// StorageVolumeFilter used for filtering storage volumes with GetStorageVolumes().
type StorageVolumeFilter struct {
Type *int
Project *string
Name *string
PoolID *int64
}

// StorageVolume represents a database storage volume record.
Expand All @@ -125,13 +126,13 @@ type StorageVolume struct {
ID int64
}

// GetStoragePoolVolumes returns all storage volumes attached to a given storage pool.
// GetStorageVolumes returns all storage volumes.
// If there are no volumes, it returns an empty list and no error.
// Accepts filters for narrowing down the results returned. If memberSpecific is true, then the search is
// restricted to volumes that belong to this member or belong to all members.
func (c *ClusterTx) GetStoragePoolVolumes(ctx context.Context, poolID int64, memberSpecific bool, filters ...StorageVolumeFilter) ([]*StorageVolume, error) {
func (c *ClusterTx) GetStorageVolumes(ctx context.Context, memberSpecific bool, filters ...StorageVolumeFilter) ([]*StorageVolume, error) {
var q = &strings.Builder{}
args := []any{poolID}
args := []any{}

q.WriteString(`
SELECT
Expand All @@ -148,16 +149,10 @@ func (c *ClusterTx) GetStoragePoolVolumes(ctx context.Context, poolID int64, mem
JOIN projects ON projects.id = storage_volumes_all.project_id
LEFT JOIN nodes ON nodes.id = storage_volumes_all.node_id
JOIN storage_pools ON storage_pools.id = storage_volumes_all.storage_pool_id
WHERE storage_volumes_all.storage_pool_id = ?
`)

if memberSpecific {
q.WriteString("AND (storage_volumes_all.node_id = ? OR storage_volumes_all.node_id IS NULL) ")
args = append(args, c.nodeID)
}

if len(filters) > 0 {
q.WriteString("AND (")
q.WriteString("WHERE (")

for i, filter := range filters {
// Validate filter.
Expand All @@ -176,6 +171,11 @@ func (c *ClusterTx) GetStoragePoolVolumes(ctx context.Context, poolID int64, mem
args = append(args, *filter.Type)
}

if filter.PoolID != nil {
qFilters = append(qFilters, "storage_volumes_all.storage_pool_id = ?")
args = append(args, *filter.PoolID)
}

if filter.Project != nil {
qFilters = append(qFilters, "projects.name = ?")
args = append(args, *filter.Project)
Expand All @@ -198,6 +198,16 @@ func (c *ClusterTx) GetStoragePoolVolumes(ctx context.Context, poolID int64, mem
}

q.WriteString(")")

if memberSpecific {
if len(filters) > 0 {
q.WriteString("AND (storage_volumes_all.node_id = ? OR storage_volumes_all.node_id IS NULL) ")
} else {
q.WriteString("WHERE (storage_volumes_all.node_id = ? OR storage_volumes_all.node_id IS NULL) ")
}

args = append(args, c.nodeID)
}
}

var err error
Expand Down Expand Up @@ -248,9 +258,10 @@ func (c *ClusterTx) GetStoragePoolVolume(ctx context.Context, poolID int64, proj
Project: &projectName,
Type: &volumeType,
Name: &volumeName,
PoolID: &poolID,
}}

volumes, err := c.GetStoragePoolVolumes(ctx, poolID, memberSpecific, filters...)
volumes, err := c.GetStorageVolumes(ctx, memberSpecific, filters...)
volumesLen := len(volumes)
if (err == nil && volumesLen <= 0) || errors.Is(err, sql.ErrNoRows) {
return nil, api.StatusErrorf(http.StatusNotFound, "Storage volume not found")
Expand Down
8 changes: 4 additions & 4 deletions lxd/patches.go
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,7 @@ func patchZfsSetContentTypeUserProperty(name string, d *Daemon) error {
}

// Get the pool's custom storage volumes.
customVolumes, err := tx.GetStoragePoolVolumes(ctx, poolID, false, db.StorageVolumeFilter{Type: &volTypeCustom})
customVolumes, err := tx.GetStorageVolumes(ctx, false, db.StorageVolumeFilter{Type: &volTypeCustom, PoolID: &poolID})
if err != nil {
return fmt.Errorf("Failed getting custom storage volumes of pool %q: %w", pool, err)
}
Expand Down Expand Up @@ -979,7 +979,7 @@ func patchStorageZfsUnsetInvalidBlockSettings(_ string, d *Daemon) error {
}

// Get the pool's custom storage volumes.
volumes, err := tx.GetStoragePoolVolumes(ctx, poolID, false, db.StorageVolumeFilter{Type: &volTypeCustom}, db.StorageVolumeFilter{Type: &volTypeVM})
volumes, err := tx.GetStorageVolumes(ctx, false, db.StorageVolumeFilter{Type: &volTypeCustom, PoolID: &poolID}, db.StorageVolumeFilter{Type: &volTypeVM, PoolID: &poolID})
if err != nil {
return fmt.Errorf("Failed getting custom storage volumes of pool %q: %w", pool, err)
}
Expand Down Expand Up @@ -1097,7 +1097,7 @@ func patchStorageZfsUnsetInvalidBlockSettingsV2(_ string, d *Daemon) error {
}

// Get the pool's custom storage volumes.
volumes, err := tx.GetStoragePoolVolumes(ctx, poolID, false, db.StorageVolumeFilter{Type: &volTypeCustom}, db.StorageVolumeFilter{Type: &volTypeVM})
volumes, err := tx.GetStorageVolumes(ctx, false, db.StorageVolumeFilter{Type: &volTypeCustom, PoolID: &poolID}, db.StorageVolumeFilter{Type: &volTypeVM, PoolID: &poolID})
if err != nil {
return fmt.Errorf("Failed getting custom storage volumes of pool %q: %w", pool, err)
}
Expand Down Expand Up @@ -1338,7 +1338,7 @@ func patchStorageRenameCustomISOBlockVolumesV2(name string, d *Daemon) error {
}

// Get the pool's custom storage volumes.
customVolumes, err := tx.GetStoragePoolVolumes(ctx, poolID, false, db.StorageVolumeFilter{Type: &volTypeCustom})
customVolumes, err := tx.GetStorageVolumes(ctx, false, db.StorageVolumeFilter{Type: &volTypeCustom, PoolID: &poolID})
if err != nil {
return fmt.Errorf("Failed getting custom storage volumes of pool %q: %w", pool, err)
}
Expand Down
4 changes: 2 additions & 2 deletions lxd/storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ func UsedBy(ctx context.Context, s *state.State, pool Pool, firstOnly bool, memb

err = s.DB.Cluster.Transaction(ctx, func(ctx context.Context, tx *db.ClusterTx) error {
// Get all the volumes using the storage pool.
volumes, err := tx.GetStoragePoolVolumes(ctx, pool.ID(), memberSpecific)
poolID := pool.ID() // Create local variable to get the pointer.
volumes, err := tx.GetStorageVolumes(ctx, memberSpecific, db.StorageVolumeFilter{PoolID: &poolID})
if err != nil {
return fmt.Errorf("Failed loading storage volumes: %w", err)
}
Expand Down Expand Up @@ -184,7 +185,6 @@ func UsedBy(ctx context.Context, s *state.State, pool Pool, firstOnly bool, memb
}

// Get all buckets using the storage pool.
poolID := pool.ID()
filters := []db.StorageBucketFilter{{
PoolID: &poolID,
}}
Expand Down
3 changes: 2 additions & 1 deletion lxd/storage_pools.go
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,8 @@ func storagePoolDelete(d *Daemon, r *http.Request) response.Response {
err = s.DB.Cluster.Transaction(r.Context(), func(ctx context.Context, tx *db.ClusterTx) error {
// Get all the volumes using the storage pool on this server.
// Only image volumes should remain now.
volumes, err := tx.GetStoragePoolVolumes(ctx, pool.ID(), true)
poolID := pool.ID() // Create local variable to get the pointer.
volumes, err := tx.GetStorageVolumes(ctx, true, db.StorageVolumeFilter{PoolID: &poolID})
if err != nil {
return fmt.Errorf("Failed loading storage volumes: %w", err)
}
Expand Down
24 changes: 7 additions & 17 deletions lxd/storage_volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,29 +688,19 @@ func storagePoolVolumesGet(d *Daemon, r *http.Request) response.Response {
}

if allPools {
poolNames, err := tx.GetStoragePoolNames(ctx)
dbVolumes, err = tx.GetStorageVolumes(ctx, memberSpecific, filters...)
if err != nil {
return fmt.Errorf("Failed to get storage volumes: %w", err)
}

for _, pool := range poolNames {
poolID, err := tx.GetStoragePoolID(ctx, pool)
if err != nil {
return fmt.Errorf("Failed to get storage volumes: %w", err)
}

poolVolumes, err := tx.GetStoragePoolVolumes(ctx, poolID, memberSpecific, filters...)
if err != nil {
return fmt.Errorf("Failed loading storage volumes: %w", err)
}

dbVolumes = append(dbVolumes, poolVolumes...)
return fmt.Errorf("Failed loading storage volumes: %w", err)
}

return err
}

dbVolumes, err = tx.GetStoragePoolVolumes(ctx, poolID, memberSpecific, filters...)
for i := range filters {
filters[i].PoolID = &poolID
}

dbVolumes, err = tx.GetStorageVolumes(ctx, memberSpecific, filters...)
if err != nil {
return fmt.Errorf("Failed loading storage volumes: %w", err)
}
Expand Down
Loading