Merge pull request #1140 from stgraber/main

Improve performance of internal profile and instance listings

cmd/incusd/api_internal_recover.go

@@ -80,14 +80,19 @@ func internalRecoverScan(ctx context.Context, s *state.State, userPools []api.St
 			return err
 		}
 
+		profileDevices, err := dbCluster.GetDevices(ctx, tx.Tx(), "profile")
+		if err != nil {
+			return err
+		}
+
 		// Convert to map for lookups by project name later.
 		projectProfiles = make(map[string][]*api.Profile)
 		for _, profile := range profiles {
 			if projectProfiles[profile.Project] == nil {
 				projectProfiles[profile.Project] = []*api.Profile{}
 			}
 
-			apiProfile, err := profile.ToAPI(ctx, tx.Tx())
+			apiProfile, err := profile.ToAPI(ctx, tx.Tx(), profileDevices)
 			if err != nil {
 				return err
 			}

cmd/incusd/instance_patch.go

@@ -192,8 +192,13 @@ func instancePatch(d *Daemon, r *http.Request) response.Response {
 			return err
 		}
 
+		profileDevices, err := cluster.GetDevices(ctx, tx.Tx(), "profile")
+		if err != nil {
+			return err
+		}
+
 		for _, profile := range profiles {
-			apiProfile, err := profile.ToAPI(ctx, tx.Tx())
+			apiProfile, err := profile.ToAPI(ctx, tx.Tx(), profileDevices)
 			if err != nil {
 				return err
 			}

cmd/incusd/instance_post.go

@@ -622,8 +622,13 @@ func migrateInstance(ctx context.Context, s *state.State, inst instance.Instance
 					return err
 				}
 
+				profileDevices, err := dbCluster.GetDevices(ctx, tx.Tx(), "profile")
+				if err != nil {
+					return err
+				}
+
 				for _, profile := range rawProfiles {
-					apiProfile, err := profile.ToAPI(ctx, tx.Tx())
+					apiProfile, err := profile.ToAPI(ctx, tx.Tx(), profileDevices)
 					if err != nil {
 						return err
 					}

cmd/incusd/instance_put.go

@@ -138,8 +138,13 @@ func instancePut(d *Daemon, r *http.Request) response.Response {
 				return err
 			}
 
+			profileDevices, err := cluster.GetDevices(ctx, tx.Tx(), "profile")
+			if err != nil {
+				return err
+			}
+
 			for _, profile := range profiles {
-				apiProfile, err := profile.ToAPI(ctx, tx.Tx())
+				apiProfile, err := profile.ToAPI(ctx, tx.Tx(), profileDevices)
 				if err != nil {
 					return err
 				}

cmd/incusd/instances_get.go

@@ -15,15 +15,12 @@ import (
 	"github.com/lxc/incus/v6/internal/server/cluster"
 	"github.com/lxc/incus/v6/internal/server/db"
 	dbCluster "github.com/lxc/incus/v6/internal/server/db/cluster"
-	"github.com/lxc/incus/v6/internal/server/db/query"
 	"github.com/lxc/incus/v6/internal/server/instance"
 	"github.com/lxc/incus/v6/internal/server/instance/instancetype"
 	"github.com/lxc/incus/v6/internal/server/request"
 	"github.com/lxc/incus/v6/internal/server/response"
-	"github.com/lxc/incus/v6/internal/server/state"
 	"github.com/lxc/incus/v6/internal/version"
 	"github.com/lxc/incus/v6/shared/api"
-	"github.com/lxc/incus/v6/shared/logger"
 	localtls "github.com/lxc/incus/v6/shared/tls"
 	"github.com/lxc/incus/v6/shared/util"
 )
@@ -215,33 +212,12 @@ func urlInstanceTypeDetect(r *http.Request) (instancetype.Type, error) {
 func instancesGet(d *Daemon, r *http.Request) response.Response {
 	s := d.State()
 
-	for i := 0; i < 100; i++ {
-		result, err := doInstancesGet(s, r)
-		if err == nil {
-			return response.SyncResponse(true, result)
-		}
-
-		if !query.IsRetriableError(err) {
-			logger.Debugf("DBERR: containersGet: error %q", err)
-			return response.SmartError(err)
-		}
-		// 100 ms may seem drastic, but we really don't want to thrash
-		// perhaps we should use a random amount
-		time.Sleep(100 * time.Millisecond)
-	}
-
-	logger.Debugf("DBERR: containersGet, db is locked")
-	logger.Debugf(logger.GetStack())
-	return response.InternalError(fmt.Errorf("DB is locked"))
-}
-
-func doInstancesGet(s *state.State, r *http.Request) (any, error) {
 	resultFullList := []*api.InstanceFull{}
 	resultMu := sync.Mutex{}
 
 	instanceType, err := urlInstanceTypeDetect(r)
 	if err != nil {
-		return nil, err
+		return response.BadRequest(err)
 	}
 
 	// Parse the recursion field.
@@ -254,7 +230,7 @@ func doInstancesGet(s *state.State, r *http.Request) (any, error) {
 	filterStr := r.FormValue("filter")
 	clauses, err := filter.Parse(filterStr, filter.QueryOperatorSet())
 	if err != nil {
-		return nil, fmt.Errorf("Invalid filter: %w", err)
+		return response.BadRequest(fmt.Errorf("Invalid filter: %w", err))
 	}
 
 	mustLoadObjects := recursion > 0 || (recursion == 0 && clauses != nil && len(clauses.Clauses) > 0)
@@ -264,7 +240,7 @@ func doInstancesGet(s *state.State, r *http.Request) (any, error) {
 	allProjects := util.IsTrue(r.FormValue("all-projects"))
 
 	if allProjects && projectName != "" {
-		return nil, api.StatusErrorf(http.StatusBadRequest, "Cannot specify a project when requesting all projects")
+		return response.BadRequest(fmt.Errorf("Cannot specify a project when requesting all projects"))
 	} else if !allProjects && projectName == "" {
 		projectName = api.ProjectDefaultName
 	}
@@ -297,12 +273,12 @@ func doInstancesGet(s *state.State, r *http.Request) (any, error) {
 		return nil
 	})
 	if err != nil {
-		return nil, err
+		return response.SmartError(err)
 	}
 
 	userHasPermission, err := s.Authorizer.GetPermissionChecker(r.Context(), r, auth.EntitlementCanView, auth.ObjectTypeInstance)
 	if err != nil {
-		return nil, err
+		return response.InternalError(err)
 	}
 
 	// Removes instances the user doesn't have access to.
@@ -429,7 +405,7 @@ func doInstancesGet(s *state.State, r *http.Request) (any, error) {
 			for _, projectName := range filteredProjects {
 				insts, err := instanceLoadNodeProjectAll(r.Context(), s, projectName, instanceType)
 				if err != nil {
-					return nil, fmt.Errorf("Failed loading instances for project %q: %w", projectName, err)
+					return response.InternalError(fmt.Errorf("Failed loading instances for project %q: %w", projectName, err))
 				}
 
 				for _, inst := range insts {
@@ -499,7 +475,7 @@ func doInstancesGet(s *state.State, r *http.Request) (any, error) {
 	if clauses != nil && len(clauses.Clauses) > 0 {
 		resultFullList, err = instance.FilterFull(resultFullList, *clauses)
 		if err != nil {
-			return nil, err
+			return response.SmartError(err)
 		}
 	}
 
@@ -510,7 +486,7 @@ func doInstancesGet(s *state.State, r *http.Request) (any, error) {
 			resultList = append(resultList, url.String())
 		}
 
-		return resultList, nil
+		return response.SyncResponse(true, resultList)
 	}
 
 	if recursion == 1 {
@@ -519,10 +495,10 @@ func doInstancesGet(s *state.State, r *http.Request) (any, error) {
 			resultList = append(resultList, &resultFullList[i].Instance)
 		}
 
-		return resultList, nil
+		return response.SyncResponse(true, resultList)
 	}
 
-	return resultFullList, nil
+	return response.SyncResponse(true, resultFullList)
 }
 
 // Fetch information about the containers on the given remote node, using the

cmd/incusd/instances_post.go

@@ -991,6 +991,11 @@ func instancesPost(d *Daemon, r *http.Request) response.Response {
 				return err
 			}
 
+			dbProfileDevices, err := dbCluster.GetDevices(ctx, tx.Tx(), "profile")
+			if err != nil {
+				return err
+			}
+
 			profilesByName := make(map[string]dbCluster.Profile, len(dbProfiles))
 			for _, dbProfile := range dbProfiles {
 				profilesByName[dbProfile.Name] = dbProfile
@@ -1002,7 +1007,7 @@ func instancesPost(d *Daemon, r *http.Request) response.Response {
 					return fmt.Errorf("Requested profile %q doesn't exist", profileName)
 				}
 
-				apiProfile, err := profile.ToAPI(ctx, tx.Tx())
+				apiProfile, err := profile.ToAPI(ctx, tx.Tx(), dbProfileDevices)
 				if err != nil {
 					return err
 				}

cmd/incusd/profiles.go

@@ -189,32 +189,42 @@ func profilesGet(d *Daemon, r *http.Request) response.Response {
 			}
 		}
 
-		apiProfiles := make([]*api.Profile, 0, len(profiles))
-		for _, profile := range profiles {
-			if !userHasPermission(auth.ObjectProfile(p.Name, profile.Name)) {
-				continue
-			}
-
-			apiProfile, err := profile.ToAPI(ctx, tx.Tx())
+		if recursion {
+			profileDevices, err := dbCluster.GetDevices(ctx, tx.Tx(), "profile")
 			if err != nil {
 				return err
 			}
 
-			apiProfile.UsedBy, err = profileUsedBy(ctx, tx, profile)
-			if err != nil {
-				return err
-			}
+			apiProfiles := make([]*api.Profile, 0, len(profiles))
+			for _, profile := range profiles {
+				if !userHasPermission(auth.ObjectProfile(p.Name, profile.Name)) {
+					continue
+				}
 
-			apiProfile.UsedBy = project.FilterUsedBy(s.Authorizer, r, apiProfile.UsedBy)
-			apiProfiles = append(apiProfiles, apiProfile)
-		}
+				apiProfile, err := profile.ToAPI(ctx, tx.Tx(), profileDevices)
+				if err != nil {
+					return err
+				}
+
+				apiProfile.UsedBy, err = profileUsedBy(ctx, tx, profile)
+				if err != nil {
+					return err
+				}
+
+				apiProfile.UsedBy = project.FilterUsedBy(s.Authorizer, r, apiProfile.UsedBy)
+				apiProfiles = append(apiProfiles, apiProfile)
+			}
 
-		if recursion {
 			result = apiProfiles
 		} else {
-			urls := make([]string, len(apiProfiles))
-			for i, apiProfile := range apiProfiles {
-				urls[i] = apiProfile.URL(version.APIVersion, apiProfile.Project).String()
+			urls := make([]string, 0, len(profiles))
+			for _, profile := range profiles {
+				if !userHasPermission(auth.ObjectProfile(p.Name, profile.Name)) {
+					continue
+				}
+
+				apiProfile := api.Profile{Name: profile.Name}
+				urls = append(urls, apiProfile.URL(version.APIVersion, profile.Project).String())
 			}
 
 			result = urls
@@ -427,7 +437,12 @@ func profileGet(d *Daemon, r *http.Request) response.Response {
 			return fmt.Errorf("Fetch profile: %w", err)
 		}
 
-		resp, err = profile.ToAPI(ctx, tx.Tx())
+		profileDevices, err := dbCluster.GetDevices(ctx, tx.Tx(), "profile")
+		if err != nil {
+			return err
+		}
+
+		resp, err = profile.ToAPI(ctx, tx.Tx(), profileDevices)
 		if err != nil {
 			return err
 		}
@@ -518,7 +533,7 @@ func profilePut(d *Daemon, r *http.Request) response.Response {
 			return fmt.Errorf("Failed to retrieve profile %q: %w", name, err)
 		}
 
-		profile, err = current.ToAPI(ctx, tx.Tx())
+		profile, err = current.ToAPI(ctx, tx.Tx(), nil)
 		if err != nil {
 			return err
 		}
@@ -623,7 +638,7 @@ func profilePatch(d *Daemon, r *http.Request) response.Response {
 			return fmt.Errorf("Failed to retrieve profile=%q: %w", name, err)
 		}
 
-		profile, err = current.ToAPI(ctx, tx.Tx())
+		profile, err = current.ToAPI(ctx, tx.Tx(), nil)
 		if err != nil {
 			return err
 		}

internal/server/backup/backup_config_utils.go

@@ -51,8 +51,14 @@ func ConfigToInstanceDBArgs(state *state.State, c *config.Config, projectName st
 				return err
 			}
 
+			// Get all the profile devices.
+			profileDevices, err := cluster.GetDevices(ctx, tx.Tx(), "profile")
+			if err != nil {
+				return err
+			}
+
 			for _, profile := range profiles {
-				apiProfile, err := profile.ToAPI(ctx, tx.Tx())
+				apiProfile, err := profile.ToAPI(ctx, tx.Tx(), profileDevices)
 				if err != nil {
 					return err
 				}

internal/server/db/cluster/instances.go

@@ -77,16 +77,23 @@ type InstanceFilter struct {
 }
 
 // ToAPI converts the database Instance to API type.
-func (i *Instance) ToAPI(ctx context.Context, tx *sql.Tx) (*api.Instance, error) {
+func (i *Instance) ToAPI(ctx context.Context, tx *sql.Tx, instanceDevices map[int][]Device, profileDevices map[int][]Device) (*api.Instance, error) {
 	profiles, err := GetInstanceProfiles(ctx, tx, i.ID)
 	if err != nil {
 		return nil, err
 	}
 
+	if profileDevices == nil {
+		profileDevices, err = GetDevices(ctx, tx, "profile")
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	apiProfiles := make([]api.Profile, 0, len(profiles))
 	profileNames := make([]string, 0, len(profiles))
 	for _, p := range profiles {
-		apiProfile, err := p.ToAPI(ctx, tx)
+		apiProfile, err := p.ToAPI(ctx, tx, profileDevices)
 		if err != nil {
 			return nil, err
 		}
@@ -95,9 +102,18 @@ func (i *Instance) ToAPI(ctx context.Context, tx *sql.Tx) (*api.Instance, error)
 		profileNames = append(profileNames, p.Name)
 	}
 
-	devices, err := GetInstanceDevices(ctx, tx, i.ID)
-	if err != nil {
-		return nil, err
+	var devices map[string]Device
+	if instanceDevices != nil {
+		devices = map[string]Device{}
+
+		for _, dev := range instanceDevices[i.ID] {
+			devices[dev.Name] = dev
+		}
+	} else {
+		devices, err = GetInstanceDevices(ctx, tx, i.ID)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	apiDevices := DevicesToAPI(devices)

internal/server/db/cluster/profiles.go

@@ -52,15 +52,24 @@ type ProfileFilter struct {
 }
 
 // ToAPI returns a cluster Profile as an API struct.
-func (p *Profile) ToAPI(ctx context.Context, tx *sql.Tx) (*api.Profile, error) {
+func (p *Profile) ToAPI(ctx context.Context, tx *sql.Tx, profileDevices map[int][]Device) (*api.Profile, error) {
 	config, err := GetProfileConfig(ctx, tx, p.ID)
 	if err != nil {
 		return nil, err
 	}
 
-	devices, err := GetProfileDevices(ctx, tx, p.ID)
-	if err != nil {
-		return nil, err
+	var devices map[string]Device
+	if profileDevices != nil {
+		devices = map[string]Device{}
+
+		for _, dev := range profileDevices[p.ID] {
+			devices[dev.Name] = dev
+		}
+	} else {
+		devices, err = GetProfileDevices(ctx, tx, p.ID)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	profile := &api.Profile{