Fix SQL Performance: missing indices (#561)

* db: add setupJoinTables

* db: add migration to add missing indices

* db: setup all join tables

* db: use unsigned integer

stores/hostdb.go

@@ -91,13 +91,25 @@ type (
 		Hosts []dbHost  `gorm:"many2many:host_allowlist_entry_hosts;constraint:OnDelete:CASCADE"`
 	}
 
+	// dbHostAllowlistEntryHost is a join table between dbAllowlistEntry and dbHost.
+	dbHostAllowlistEntryHost struct {
+		DBAllowlistEntryID uint `gorm:"primaryKey"`
+		DBHostID           uint `gorm:"primaryKey;index"`
+	}
+
 	// dbBlocklistEntry defines a table that stores the host blocklist.
 	dbBlocklistEntry struct {
 		Model
 		Entry string   `gorm:"unique;index;NOT NULL"`
 		Hosts []dbHost `gorm:"many2many:host_blocklist_entry_hosts;constraint:OnDelete:CASCADE"`
 	}
 
+	// dbHostBlocklistEntryHost is a join table between dbBlocklistEntry and dbHost.
+	dbHostBlocklistEntryHost struct {
+		DBBlocklistEntryID uint `gorm:"primaryKey"`
+		DBHostID           uint `gorm:"primaryKey;index"`
+	}
+
 	// dbConsensusInfo defines table which stores the latest consensus info
 	// known to the hostdb. It should only ever contain a single entry with
 	// the consensusInfoID primary key.
@@ -286,9 +298,15 @@ func (dbInteraction) TableName() string { return "host_interactions" }
 // TableName implements the gorm.Tabler interface.
 func (dbAllowlistEntry) TableName() string { return "host_allowlist_entries" }
 
+// TableName implements the gorm.Tabler interface.
+func (dbHostAllowlistEntryHost) TableName() string { return "host_allowlist_entry_hosts" }
+
 // TableName implements the gorm.Tabler interface.
 func (dbBlocklistEntry) TableName() string { return "host_blocklist_entries" }
 
+// TableName implements the gorm.Tabler interface.
+func (dbHostBlocklistEntryHost) TableName() string { return "host_blocklist_entry_hosts" }
+
 // convert converts a host into a hostdb.Host.
 func (h dbHost) convert() hostdb.Host {
 	var lastScan time.Time

stores/metadata.go

@@ -69,6 +69,11 @@ type (
 		Contracts []dbContract `gorm:"many2many:contract_set_contracts;constraint:OnDelete:CASCADE"`
 	}
 
+	dbContractSetContract struct {
+		DBContractSetID uint `gorm:"primaryKey;"`
+		DBContractID    uint `gorm:"primaryKey;index"`
+	}
+
 	dbObject struct {
 		Model
 
@@ -126,8 +131,8 @@ type (
 
 	// dbContractSector is a join table between dbContract and dbSector.
 	dbContractSector struct {
-		DBContractID uint `gorm:"primaryKey"`
-		DBSectorID   uint `gorm:"primaryKey"`
+		DBSectorID   uint `gorm:"primaryKey;"`
+		DBContractID uint `gorm:"primaryKey;index"`
 	}
 
 	// rawObject is used for hydration and is made up of one or many raw sectors.
@@ -164,6 +169,9 @@ func (dbArchivedContract) TableName() string { return "archived_contracts" }
 // TableName implements the gorm.Tabler interface.
 func (dbContract) TableName() string { return "contracts" }
 
+// TableName implements the gorm.Tabler interface.
+func (dbContractSetContract) TableName() string { return "contract_set_contracts" }
+
 // TableName implements the gorm.Tabler interface.
 func (dbContractSector) TableName() string { return "contract_sectors" }
 

stores/migrations.go

@@ -44,15 +44,6 @@ var (
 	}
 )
 
-type dbHostBlocklistEntryHost struct {
-	DBBlocklistEntryID uint8 `gorm:"primarykey;column:db_blocklist_entry_id"`
-	DBHostID           uint8 `gorm:"primarykey;index:idx_db_host_id;column:db_host_id"`
-}
-
-func (dbHostBlocklistEntryHost) TableName() string {
-	return "host_blocklist_entry_hosts"
-}
-
 // migrateShards performs the migrations necessary for removing the 'shards'
 // table.
 func migrateShards(ctx context.Context, db *gorm.DB, l glogger.Interface) error {
@@ -183,6 +174,13 @@ func performMigrations(db *gorm.DB, logger glogger.Interface) error {
 			},
 			Rollback: nil,
 		},
+		{
+			ID: "00008_jointableindices",
+			Migrate: func(tx *gorm.DB) error {
+				return performMigration00008_jointableindices(tx, logger)
+			},
+			Rollback: nil,
+		},
 	}
 
 	// Create migrator.
@@ -206,7 +204,14 @@ func performMigrations(db *gorm.DB, logger glogger.Interface) error {
 // initSchema is executed only on a clean database. Otherwise the individual
 // migrations are executed.
 func initSchema(tx *gorm.DB) error {
-	err := tx.AutoMigrate(tables...)
+	// Setup join tables.
+	err := setupJoinTables(tx)
+	if err != nil {
+		return fmt.Errorf("failed to setup join tables: %w", err)
+	}
+
+	// Run auto migrations.
+	err = tx.AutoMigrate(tables...)
 	if err != nil {
 		return fmt.Errorf("failed to init schema: %w", err)
 	}
@@ -217,6 +222,41 @@ func initSchema(tx *gorm.DB) error {
 	return nil
 }
 
+func setupJoinTables(tx *gorm.DB) error {
+	jointables := []struct {
+		model     interface{}
+		joinTable interface{ TableName() string }
+		field     string
+	}{
+		{
+			&dbAllowlistEntry{},
+			&dbHostAllowlistEntryHost{},
+			"Hosts",
+		},
+		{
+			&dbBlocklistEntry{},
+			&dbHostBlocklistEntryHost{},
+			"Hosts",
+		},
+		{
+			&dbSector{},
+			&dbContractSector{},
+			"Contracts",
+		},
+		{
+			&dbContractSet{},
+			&dbContractSetContract{},
+			"Contracts",
+		},
+	}
+	for _, t := range jointables {
+		if err := tx.SetupJoinTable(t.model, t.field, t.joinTable); err != nil {
+			return fmt.Errorf("failed to setup join table '%s': %w", t.joinTable.TableName(), err)
+		}
+	}
+	return nil
+}
+
 // performMigration00001_gormigrate performs the first migration before
 // introducing gormigrate.
 func performMigration00001_gormigrate(txn *gorm.DB, logger glogger.Interface) error {
@@ -476,3 +516,41 @@ func performMigration00007_archivedcontractspending(txn *gorm.DB, logger glogger
 	logger.Info(context.Background(), "migration 00007_archivedcontractspending complete")
 	return nil
 }
+
+func performMigration00008_jointableindices(txn *gorm.DB, logger glogger.Interface) error {
+	logger.Info(context.Background(), "performing migration 00008_jointableindices")
+
+	indices := []struct {
+		joinTable interface{ TableName() string }
+		column    string
+	}{
+		{
+			&dbHostAllowlistEntryHost{},
+			"DBHostID",
+		},
+		{
+			&dbHostBlocklistEntryHost{},
+			"DBHostID",
+		},
+		{
+			&dbContractSector{},
+			"DBContractID",
+		},
+		{
+			&dbContractSetContract{},
+			"DBContractID",
+		},
+	}
+
+	m := txn.Migrator()
+	for _, idx := range indices {
+		if !m.HasIndex(idx.joinTable, idx.column) {
+			if err := m.CreateIndex(idx.joinTable, idx.column); err != nil {
+				return fmt.Errorf("failed to create index on column '%s' of table '%s': %w", idx.column, idx.joinTable.TableName(), err)
+			}
+		}
+	}
+
+	logger.Info(context.Background(), "migration 00008_jointableindices complete")
+	return nil
+}

stores/sql_test.go

@@ -4,7 +4,9 @@ import (
 	"bytes"
 	"context"
 	"encoding/hex"
+	"fmt"
 	"os"
+	"strings"
 	"testing"
 	"time"
 
@@ -112,3 +114,47 @@ func TestConsensusReset(t *testing.T) {
 		t.Fatal("wrong id", db.chainIndex.ID, types.BlockID{})
 	}
 }
+
+type queryPlanExplain struct {
+	ID      int    `json:"id"`
+	Parent  int    `json:"parent"`
+	NotUsed bool   `json:"notused"`
+	Detail  string `json:"detail"`
+}
+
+func TestQueryPlan(t *testing.T) {
+	db, _, _, err := newTestSQLStore(t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	queries := []string{
+		// allow_list
+		"SELECT * FROM host_allowlist_entry_hosts WHERE db_host_id = 1",
+		"SELECT * FROM host_allowlist_entry_hosts WHERE db_allowlist_entry_id = 1",
+
+		// block_list
+		"SELECT * FROM host_blocklist_entry_hosts WHERE db_host_id = 1",
+		"SELECT * FROM host_blocklist_entry_hosts WHERE db_blocklist_entry_id = 1",
+
+		// contract_sectors
+		"SELECT * FROM contract_sectors WHERE db_contract_id = 1",
+		"SELECT * FROM contract_sectors WHERE db_sector_id = 1",
+
+		// contract_set_contracts
+		"SELECT * FROM contract_set_contracts WHERE db_contract_id = 1",
+		"SELECT * FROM contract_set_contracts WHERE db_contract_set_id = 1",
+	}
+
+	for _, query := range queries {
+		var explain queryPlanExplain
+		err = db.db.Raw(fmt.Sprintf("EXPLAIN QUERY PLAN %s;", query)).Scan(&explain).Error
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !(strings.Contains(explain.Detail, "USING INDEX") ||
+			strings.Contains(explain.Detail, "USING COVERING INDEX")) {
+			t.Fatalf("query '%s' should use an index, instead the plan was '%s'", query, explain.Detail)
+		}
+	}
+}