Merge pull request #440 from stgraber/main

Database performance improvements

cmd/incusd/instance_console.go

@@ -448,7 +448,7 @@ func instanceConsolePost(d *Daemon, r *http.Request) response.Response {
 	}
 
 	// Forward the request if the container is remote.
-	client, err := cluster.ConnectIfInstanceIsRemote(s.DB.Cluster, projectName, name, s.Endpoints.NetworkCert(), s.ServerCert(), r, instanceType)
+	client, err := cluster.ConnectIfInstanceIsRemote(s, projectName, name, r, instanceType)
 	if err != nil {
 		return response.SmartError(err)
 	}

cmd/incusd/instance_exec.go

@@ -571,7 +571,7 @@ func instanceExecPost(d *Daemon, r *http.Request) response.Response {
 	}
 
 	// Forward the request if the container is remote.
-	client, err := cluster.ConnectIfInstanceIsRemote(s.DB.Cluster, projectName, name, s.Endpoints.NetworkCert(), s.ServerCert(), r, instanceType)
+	client, err := cluster.ConnectIfInstanceIsRemote(s, projectName, name, r, instanceType)
 	if err != nil {
 		return response.SmartError(err)
 	}

cmd/incusd/instance_sftp.go

@@ -72,7 +72,7 @@ func instanceSFTPHandler(d *Daemon, r *http.Request) response.Response {
 	}
 
 	// Forward the request if the instance is remote.
-	client, err := cluster.ConnectIfInstanceIsRemote(s.DB.Cluster, projectName, instName, s.Endpoints.NetworkCert(), s.ServerCert(), r, instanceType)
+	client, err := cluster.ConnectIfInstanceIsRemote(s, projectName, instName, r, instanceType)
 	if err != nil {
 		return response.SmartError(err)
 	}

cmd/incusd/response.go

@@ -45,7 +45,7 @@ func forwardedResponseIfTargetIsRemote(s *state.State, r *http.Request) response
 // the container with the given name. If the container is local, nothing gets
 // done and nil is returned.
 func forwardedResponseIfInstanceIsRemote(s *state.State, r *http.Request, project, name string, instanceType instancetype.Type) (response.Response, error) {
-	client, err := cluster.ConnectIfInstanceIsRemote(s.DB.Cluster, project, name, s.Endpoints.NetworkCert(), s.ServerCert(), r, instanceType)
+	client, err := cluster.ConnectIfInstanceIsRemote(s, project, name, r, instanceType)
 	if err != nil {
 		return nil, err
 	}

internal/server/cluster/connect.go

@@ -80,9 +80,14 @@ func Connect(address string, networkCert *localtls.CertInfo, serverCert *localtl
 // ConnectIfInstanceIsRemote figures out the address of the cluster member which is running the instance with the
 // given name in the specified project. If it's not the local member will connect to it and return the connected
 // client (configured with the specified project), otherwise it will just return nil.
-func ConnectIfInstanceIsRemote(cluster *db.Cluster, projectName string, instName string, networkCert *localtls.CertInfo, serverCert *localtls.CertInfo, r *http.Request, instanceType instancetype.Type) (incus.InstanceServer, error) {
+func ConnectIfInstanceIsRemote(s *state.State, projectName string, instName string, r *http.Request, instanceType instancetype.Type) (incus.InstanceServer, error) {
+	// No need to connect if not clustered.
+	if !s.ServerClustered {
+		return nil, nil
+	}
+
 	var address string // Cluster member address.
-	err := cluster.Transaction(context.TODO(), func(ctx context.Context, tx *db.ClusterTx) error {
+	err := s.DB.Cluster.Transaction(context.TODO(), func(ctx context.Context, tx *db.ClusterTx) error {
 		var err error
 		address, err = tx.GetNodeAddressOfInstance(ctx, projectName, instName, instanceType)
 		return err
@@ -95,7 +100,7 @@ func ConnectIfInstanceIsRemote(cluster *db.Cluster, projectName string, instName
 		return nil, nil // The instance is running on this local member, no need to connect.
 	}
 
-	client, err := Connect(address, networkCert, serverCert, r, false)
+	client, err := Connect(address, s.Endpoints.NetworkCert(), s.ServerCert(), r, false)
 	if err != nil {
 		return nil, err
 	}

internal/server/dns/handler.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/miekg/dns"
@@ -14,9 +15,14 @@ import (
 
 type dnsHandler struct {
 	server *Server
+	mu     sync.Mutex
 }
 
 func (d dnsHandler) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
+	// Don't allow concurent queries.
+	d.mu.Lock()
+	defer d.mu.Unlock()
+
 	// Check if we're ready to serve queries.
 	if d.server.zoneRetriever == nil {
 		m := new(dns.Msg)

internal/server/instance/instance_utils.go

@@ -485,13 +485,13 @@ func BackupLoadByName(s *state.State, project, name string) (*backup.InstanceBac
 		return err
 	})
 	if err != nil {
-		return nil, fmt.Errorf("Load backup from database: %w", err)
+		return nil, err
 	}
 
 	// Load the instance it belongs to
 	instance, err := LoadByID(s, args.InstanceID)
 	if err != nil {
-		return nil, fmt.Errorf("Load instance from database: %w", err)
+		return nil, err
 	}
 
 	return backup.NewInstanceBackup(s, instance, args.ID, name, args.CreationDate, args.ExpiryDate, args.InstanceOnly, args.OptimizedStorage), nil

internal/server/network/network_utils.go

@@ -79,30 +79,47 @@ func MACDevName(mac net.HardwareAddr) string {
 // UsedByInstanceDevices looks for instance NIC devices using the network and runs the supplied usageFunc for each.
 // Accepts optional filter arguments to specify a subset of instances.
 func UsedByInstanceDevices(s *state.State, networkProjectName string, networkName string, networkType string, usageFunc func(inst db.InstanceArgs, nicName string, nicConfig map[string]string) error, filters ...cluster.InstanceFilter) error {
-	return s.DB.Cluster.Transaction(context.TODO(), func(ctx context.Context, tx *db.ClusterTx) error {
+	// Get the instances.
+	projects := map[string]api.Project{}
+	instances := []db.InstanceArgs{}
+
+	err := s.DB.Cluster.Transaction(context.TODO(), func(ctx context.Context, tx *db.ClusterTx) error {
 		return tx.InstanceList(ctx, func(inst db.InstanceArgs, p api.Project) error {
-			// Get the instance's effective network project name.
-			instNetworkProject := project.NetworkProjectFromRecord(&p)
+			projects[inst.Project] = p
+			instances = append(instances, inst)
 
-			// Skip instances who's effective network project doesn't match this Network's project.
-			if instNetworkProject != networkProjectName {
-				return nil
-			}
+			return nil
+		}, filters...)
+	})
+	if err != nil {
+		return err
+	}
 
-			// Look for NIC devices using this network.
-			devices := db.ExpandInstanceDevices(inst.Devices.Clone(), inst.Profiles)
-			for devName, devConfig := range devices {
-				if isInUseByDevice(networkName, networkType, devConfig) {
-					err := usageFunc(inst, devName, devConfig)
-					if err != nil {
-						return err
-					}
+	// Go through the instances and run usageFunc.
+	for _, inst := range instances {
+		p := projects[inst.Project]
+
+		// Get the instance's effective network project name.
+		instNetworkProject := project.NetworkProjectFromRecord(&p)
+
+		// Skip instances who's effective network project doesn't match this Network's project.
+		if instNetworkProject != networkProjectName {
+			return nil
+		}
+
+		// Look for NIC devices using this network.
+		devices := db.ExpandInstanceDevices(inst.Devices.Clone(), inst.Profiles)
+		for devName, devConfig := range devices {
+			if isInUseByDevice(networkName, networkType, devConfig) {
+				err := usageFunc(inst, devName, devConfig)
+				if err != nil {
+					return err
 				}
 			}
+		}
+	}
 
-			return nil
-		}, filters...)
-	})
+	return nil
 }
 
 // UsedBy returns list of API resources using network. Accepts firstOnly argument to indicate that only the first

test/suites/backup.sh

@@ -610,14 +610,14 @@ test_backup_rename() {
   ensure_import_testimage
   ensure_has_localhost_remote "${INCUS_ADDR}"
 
-  if ! incus query -X POST /1.0/instances/c1/backups/backupmissing -d '{\"name\": \"backupnewname\"}' --wait 2>&1 | grep -q "Error: Instance not found" ; then
+  if ! incus query -X POST /1.0/instances/c1/backups/backupmissing -d '{\"name\": \"backupnewname\"}' --wait 2>&1 | grep -q "Error: Instance backup not found" ; then
     echo "invalid rename response for missing container"
     false
   fi
 
   incus init testimage c1
 
-  if ! incus query -X POST /1.0/instances/c1/backups/backupmissing -d '{\"name\": \"backupnewname\"}' --wait 2>&1 | grep -q "Error: Load backup from database: Instance backup not found" ; then
+  if ! incus query -X POST /1.0/instances/c1/backups/backupmissing -d '{\"name\": \"backupnewname\"}' --wait 2>&1 | grep -q "Error: Instance backup not found" ; then
     echo "invalid rename response for missing backup"
     false
   fi