Add indices for mysql8 + postgres to optimize ASG lookups

Author: geofffranks

src/code.cloudfoundry.org/policy-server/integration/migrate_db_test.go

@@ -3,6 +3,7 @@ package integration_test
 import (
 	"fmt"
 	"math"
+	"strings"
 	"time"
 
 	"code.cloudfoundry.org/cf-networking-helpers/db"
@@ -96,6 +97,22 @@ func assertMigrationsSucceeded(conn *db.ConnWrapper, conf config.Config) {
 		len(migrations.V3ModifiedMigrationsToPerform) +
 		len(migrations.MigrationsToPerform)
 
+	var skippedMigrations int
+	for _, migration := range migrations.MigrationsToPerform {
+		if migration.SkipMySQL57 {
+			skippedMigrations++
+		}
+	}
+
+	if conn.DriverName() == "mysql" {
+		var version string
+		err := conn.QueryRow("SELECT VERSION()").Scan(&version)
+		Expect(err).ToNot(HaveOccurred())
+		if strings.HasPrefix(version, "5.7.") {
+			numMigrations = numMigrations - skippedMigrations
+		}
+	}
+
 	var migrationCount int
 	conn.QueryRow("SELECT COUNT(*) FROM gorp_migrations").Scan(&migrationCount)
 	Expect(migrationCount).To(Equal(numMigrations))

src/code.cloudfoundry.org/policy-server/store/migrations/fakes/migrations_provider.go

@@ -432,4 +432,22 @@ var MigrationsToPerform = PolicyServerMigrations{
 		Id: "81",
 		Up: migration_v0081,
 	},
+	PolicyServerMigration{
+		Id:          "82",
+		Up:          migration_v0082,
+		SkipMySQL57: true,
+	},
+	PolicyServerMigration{
+		Id:          "83",
+		Up:          migration_v0083,
+		SkipMySQL57: true,
+	},
+	PolicyServerMigration{
+		Id: "84",
+		Up: migration_v0084,
+	},
+	PolicyServerMigration{
+		Id: "85",
+		Up: migration_v0085,
+	},
 }

src/code.cloudfoundry.org/policy-server/store/migrations/migrations.go

@@ -13,7 +13,7 @@ type MigrationsProvider struct {
 	Store migrationStore
 }
 
-func (m *MigrationsProvider) MigrationsToPerform() (PolicyServerMigrations, error) {
+func (m *MigrationsProvider) MigrationsToPerform(mysql57 bool) (PolicyServerMigrations, error) {
 	policyServerMigrations := PolicyServerMigrations{}
 
 	hasV1, err := m.Store.HasV1MigrationOccurred()
@@ -67,10 +67,12 @@ func (m *MigrationsProvider) MigrationsToPerform() (PolicyServerMigrations, erro
 		)
 	}
 
-	policyServerMigrations = append(
-		policyServerMigrations,
-		MigrationsToPerform...,
-	)
+	for _, migration := range MigrationsToPerform {
+		if mysql57 && migration.SkipMySQL57 {
+			continue
+		}
+		policyServerMigrations = append(policyServerMigrations, migration)
+	}
 
 	return policyServerMigrations, nil
 }

src/code.cloudfoundry.org/policy-server/store/migrations/migrations_provider.go

@@ -25,7 +25,7 @@ var _ = Describe("Migrations Provider", func() {
 	})
 
 	It("returns a list of migrations to perform", func() {
-		migrationsToPerform, err := migrationsProvider.MigrationsToPerform()
+		migrationsToPerform, err := migrationsProvider.MigrationsToPerform(false)
 		Expect(err).ToNot(HaveOccurred())
 		expectedMigrations := migrations.PolicyServerMigrations{
 			migrations.V1ModifiedMigrationsToPerform[0],
@@ -47,32 +47,51 @@ var _ = Describe("Migrations Provider", func() {
 
 	It("returns a helpful error message for V1MigrationOccurred errors", func() {
 		migrationStore.HasV1MigrationOccurredReturns(false, errors.New("I AM ERROR"))
-		_, err := migrationsProvider.MigrationsToPerform()
+		_, err := migrationsProvider.MigrationsToPerform(false)
 
 		Expect(err).To(MatchError("failed to check V1 Migration status: I AM ERROR"))
 	})
 
 	It("returns a helpful error message for V2MigrationOccurred errors", func() {
 		migrationStore.HasV2MigrationOccurredReturns(false, errors.New("I AM ERROR"))
-		_, err := migrationsProvider.MigrationsToPerform()
+		_, err := migrationsProvider.MigrationsToPerform(false)
 
 		Expect(err).To(MatchError("failed to check V2 Migration status: I AM ERROR"))
 	})
 
 	It("returns a helpful error message for V3MigrationOccurred errors", func() {
 		migrationStore.HasV3MigrationOccurredReturns(false, errors.New("I AM ERROR"))
-		_, err := migrationsProvider.MigrationsToPerform()
+		_, err := migrationsProvider.MigrationsToPerform(false)
 
 		Expect(err).To(MatchError("failed to check V3 Migration status: I AM ERROR"))
 	})
 
+	Context("when a migration is set to skip on mysql 5.7", func() {
+		Context("and the database is not mysql 5.7", func() {
+			It("lists the migration", func() {
+				migrationsToPerform, err := migrationsProvider.MigrationsToPerform(false)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(migrationsToPerform).To(ContainElement(migrations.MigrationsToPerform[78]))
+				Expect(migrationsToPerform).To(ContainElement(migrations.MigrationsToPerform[79]))
+			})
+		})
+		Context("and the database is mysql 5.7", func() {
+			It("doesn't list the migration", func() {
+				migrationsToPerform, err := migrationsProvider.MigrationsToPerform(true)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(migrationsToPerform).ToNot(ContainElement(migrations.MigrationsToPerform[78]))
+				Expect(migrationsToPerform).ToNot(ContainElement(migrations.MigrationsToPerform[79]))
+			})
+		})
+	})
+
 	Context("when legacy v1 migration has already occurred", func() {
 		BeforeEach(func() {
 			migrationStore.HasV1MigrationOccurredReturns(true, nil)
 		})
 
 		It("returns a legacy v1 migration in the list of migrations to perform", func() {
-			migrationsToPerform, err := migrationsProvider.MigrationsToPerform()
+			migrationsToPerform, err := migrationsProvider.MigrationsToPerform(false)
 			Expect(err).ToNot(HaveOccurred())
 			expectedMigrations := migrations.PolicyServerMigrations{
 				migrations.V1LegacyMigrationsToPerform[0],
@@ -99,7 +118,7 @@ var _ = Describe("Migrations Provider", func() {
 		})
 
 		It("returns a legacy v2 migration in the list of migrations to perform", func() {
-			migrationsToPerform, err := migrationsProvider.MigrationsToPerform()
+			migrationsToPerform, err := migrationsProvider.MigrationsToPerform(false)
 			Expect(err).ToNot(HaveOccurred())
 			expectedMigrations := migrations.PolicyServerMigrations{
 				migrations.V2LegacyMigrationsToPerform[0],
@@ -123,7 +142,7 @@ var _ = Describe("Migrations Provider", func() {
 		})
 
 		It("returns a legacy v3 migration in the list of migrations to perform", func() {
-			migrationsToPerform, err := migrationsProvider.MigrationsToPerform()
+			migrationsToPerform, err := migrationsProvider.MigrationsToPerform(false)
 			Expect(err).ToNot(HaveOccurred())
 			expectedMigrations := migrations.PolicyServerMigrations{
 				migrations.V3LegacyMigrationsToPerform[0],

src/code.cloudfoundry.org/policy-server/store/migrations/migrations_provider_test.go

@@ -3,11 +3,14 @@ package migrations
 import (
 	"database/sql"
 	"fmt"
+	"strings"
 
 	migrate "github.com/cf-container-networking/sql-migrate"
 	"github.com/jmoiron/sqlx"
 )
 
+//go:generate counterfeiter -generate
+
 //counterfeiter:generate -o fakes/migrate_adapter.go --fake-name MigrateAdapter . migrateAdapter
 type migrateAdapter interface {
 	ExecMax(db MigrationDb, dialect string, m migrate.MigrationSource, dir migrate.MigrationDirection, maxNumMigrations int) (int, error)
@@ -24,7 +27,7 @@ type MigrationDb interface {
 
 //counterfeiter:generate -o fakes/migrations_provider.go --fake-name MigrationsProvider . migrationsProvider
 type migrationsProvider interface {
-	MigrationsToPerform() (PolicyServerMigrations, error)
+	MigrationsToPerform(bool) (PolicyServerMigrations, error)
 }
 
 type Migrator struct {
@@ -33,7 +36,27 @@ type Migrator struct {
 }
 
 func (m *Migrator) PerformMigrations(driverName string, migrationDb MigrationDb, maxNumMigrations int) (int, error) {
-	migrationsToPerform, err := m.MigrationsProvider.MigrationsToPerform()
+	var mysql57 bool
+	if driverName == "mysql" {
+		row := migrationDb.QueryRow("SELECT VERSION()")
+		// if no rows are returned, we're definitely not mysql 5.7, so short circuit out
+		if row != nil {
+			var version string
+			err := row.Scan(&version)
+			if err != nil {
+				return 0, fmt.Errorf("Unable to detect MySQL Version: %s", err)
+			}
+			// mysql returns major.minor.patch version number strings
+			// e.g. `5.7.43` would be the data returned for the VERSION() query
+			mysql57 = strings.HasPrefix(version, "5.7.")
+		}
+	}
+	// at this point, the mysql57 bool is false if postgres, false if no rows were returned
+	// for SELECT VERSION(), false if the prefix doesn't start with `5.7.` (in case there's ever a 5.70)
+	// and true if 5.7.x. the above should only have thrown an error if there was a problem talking
+	// to the database to execute the query, or the row couldn't be marshalled into a string variable
+
+	migrationsToPerform, err := m.MigrationsProvider.MigrationsToPerform(mysql57)
 	if err != nil {
 		return 0, fmt.Errorf("error retrieving migrations to perform: %s", err)
 	}
@@ -79,8 +102,9 @@ func (s PolicyServerMigrations) supportsDriver(driverName string) bool {
 }
 
 type PolicyServerMigration struct {
-	Id string
-	Up map[string][]string
+	Id          string
+	SkipMySQL57 bool
+	Up          map[string][]string
 }
 
 func (psm *PolicyServerMigration) forDriver(driverName string) *migrate.Migration {

src/code.cloudfoundry.org/policy-server/store/migrations/migrator.go

@@ -2237,7 +2237,7 @@ func queryTableColumnNames(tableName string, realDb *db.ConnWrapper) []string {
 }
 
 func getMigrationIndex(migrationsProvider *migrations.MigrationsProvider, migrationId string) int {
-	migrationsToPerform, err := migrationsProvider.MigrationsToPerform()
+	migrationsToPerform, err := migrationsProvider.MigrationsToPerform(false)
 	Expect(err).NotTo(HaveOccurred())
 	for i, migration := range migrationsToPerform {
 		if migration.Id == migrationId {

src/code.cloudfoundry.org/policy-server/store/migrations/migrator_test.go

@@ -0,0 +1,8 @@
+package migrations
+
+var migration_v0082 = map[string][]string{
+	"mysql": {
+		`CREATE INDEX staging_spaces_idx ON security_groups ((CAST(staging_spaces -> '$[*]' AS CHAR(36) ARRAY)))`,
+	},
+	"postgres": {},
+}

src/code.cloudfoundry.org/policy-server/store/migrations/v0082.go

@@ -0,0 +1,8 @@
+package migrations
+
+var migration_v0083 = map[string][]string{
+	"mysql": {
+		`CREATE INDEX running_spaces_idx ON security_groups ((CAST(running_spaces -> '$[*]' AS CHAR(36) ARRAY)))`,
+	},
+	"postgres": {},
+}

src/code.cloudfoundry.org/policy-server/store/migrations/v0083.go

@@ -0,0 +1,8 @@
+package migrations
+
+var migration_v0084 = map[string][]string{
+	"mysql": {
+		`CREATE INDEX global_staging_idx ON security_groups (staging_default)`,
+	},
+	"postgres": {},
+}

src/code.cloudfoundry.org/policy-server/store/migrations/v0084.go

@@ -0,0 +1,8 @@
+package migrations
+
+var migration_v0085 = map[string][]string{
+	"mysql": {
+		`CREATE INDEX global_running_idx ON security_groups (running_default)`,
+	},
+	"postgres": {},
+}