sean's feedback

master/cmd/determined-master/migrate.go

@@ -52,7 +52,7 @@ func runMigrate(cmd *cobra.Command, args []string) error {
 		}
 	}()
 
-	if _, err = database.Migrate(config.DB.Migrations, config.DB.DatabaseCode, args); err != nil {
+	if _, err = database.Migrate(config.DB.Migrations, config.DB.ViewsAndTriggers, args); err != nil {
 		return errors.Wrap(err, "running migrations")
 	}
 

master/internal/config/config.go

@@ -63,15 +63,15 @@ type CacheConfig struct {
 
 // DBConfig hosts configuration fields of the database.
 type DBConfig struct {
-	User         string `json:"user"`
-	Password     string `json:"password"`
-	Migrations   string `json:"migrations"`
-	DatabaseCode string `json:"database_code"`
-	Host         string `json:"host"`
-	Port         string `json:"port"`
-	Name         string `json:"name"`
-	SSLMode      string `json:"ssl_mode"`
-	SSLRootCert  string `json:"ssl_root_cert"`
+	User             string `json:"user"`
+	Password         string `json:"password"`
+	Migrations       string `json:"migrations"`
+	ViewsAndTriggers string `json:"views_and_triggers"`
+	Host             string `json:"host"`
+	Port             string `json:"port"`
+	Name             string `json:"name"`
+	SSLMode          string `json:"ssl_mode"`
+	SSLRootCert      string `json:"ssl_root_cert"`
 }
 
 // WebhooksConfig hosts configuration fields for webhook functionality.
@@ -320,7 +320,7 @@ func (c *Config) Resolve() error {
 	c.Root = root
 
 	c.DB.Migrations = fmt.Sprintf("file://%s", filepath.Join(c.Root, "static/migrations"))
-	c.DB.DatabaseCode = filepath.Join(c.Root, "static/db_code")
+	c.DB.ViewsAndTriggers = filepath.Join(c.Root, "static/views_and_triggers")
 
 	// We must resolve resources before we apply pool defaults.
 	if err := c.ResolveResource(); err != nil {

master/internal/db/migrations.go

@@ -143,28 +143,28 @@ func (db *PgDB) readDBCodeAndCheckIfDifferent(dbCodeDir string) (map[string]stri
 	hash := sha256.Sum256([]byte(allCode))
 	ourHash := hex.EncodeToString(hash[:])
 
-	// Check if the db_code_hash table exists. If it doesn't return that we need to create db code.
+	// Check if the views_and_triggers_hash table exists. If it doesn't return that we need to create db code.
 	var tableExists bool
 	if err = db.sql.QueryRow(
-		"SELECT EXISTS(SELECT 1 FROM information_schema.tables WHERE table_name = 'db_code_hash')").
+		"SELECT EXISTS(SELECT 1 FROM information_schema.tables WHERE table_name = 'views_and_triggers_hash')").
 		Scan(&tableExists); err != nil {
-		return nil, false, fmt.Errorf("checking db_code_hash exists: %w", err)
+		return nil, false, fmt.Errorf("checking views_and_triggers_hash exists: %w", err)
 	}
 	if !tableExists {
 		return fileNamesToSQL, true, nil
 	}
 
 	// Check if our hashes match. If they do we can just return we don't need to do anything.
 	var databaseHash string
-	if err := db.sql.QueryRow("SELECT hash FROM db_code_hash").Scan(&databaseHash); err != nil {
-		return nil, false, fmt.Errorf("getting hash from db_code_hash: %w", err)
+	if err := db.sql.QueryRow("SELECT hash FROM views_and_triggers_hash").Scan(&databaseHash); err != nil {
+		return nil, false, fmt.Errorf("getting hash from views_and_triggers_hash: %w", err)
 	}
 	if databaseHash == ourHash {
 		return fileNamesToSQL, false, nil
 	}
 
-	// Update our hash and return we need to create db code.
-	if _, err := db.sql.Exec("UPDATE db_code_hash SET hash = $1", ourHash); err != nil {
+	// Update our hash and return we need to create views and triggers.
+	if _, err := db.sql.Exec("UPDATE views_and_triggers_hash SET hash = $1", ourHash); err != nil {
 		return nil, false, fmt.Errorf("updating our database hash: %w", err)
 	}
 	return fileNamesToSQL, false, nil
@@ -198,6 +198,9 @@ CREATE SCHEMA determined_code;`); err != nil {
 	return nil
 }
 
+// This is set in an init in postgres_test_utils.go behind the intg feature flag.
+// For normal usages this won't build. For tests we need to serialize access to
+// run migrations.
 var testOnlyDBLock func(sql *sqlx.DB) (unlock func())
 
 // Migrate runs the migrations from the specified directory URL.

master/internal/db/postgres_test_utils.go

@@ -197,7 +197,7 @@ func MigrateTestPostgres(db *PgDB, migrationsPath string, actions ...string) err
 		actions = []string{"up"}
 	}
 	_, err := db.Migrate(
-		migrationsPath, strings.ReplaceAll(migrationsPath+"/../db_code", "file://", ""), actions)
+		migrationsPath, strings.ReplaceAll(migrationsPath+"/../views_and_triggers", "file://", ""), actions)
 	if err != nil {
 		return fmt.Errorf("failed to migrate postgres: %w", err)
 	}

master/internal/db/setup.go

@@ -90,7 +90,7 @@ func Setup(opts *config.DBConfig) (db *PgDB, isNew bool, err error) {
 		return db, false, err
 	}
 
-	isNew, err = db.Migrate(opts.Migrations, opts.DatabaseCode, []string{"up"})
+	isNew, err = db.Migrate(opts.Migrations, opts.ViewsAndTriggers, []string{"up"})
 	if err != nil {
 		return nil, false, fmt.Errorf("error running migrations: %s", err)
 	}

master/static/migrations/20240502203516_database-code.tx.up.sql

@@ -1,7 +1,7 @@
-CREATE TABLE db_code_hash (
+CREATE TABLE views_and_triggers_hash (
     hash text PRIMARY KEY NOT NULL
 );
-INSERT INTO db_code_hash VALUES('blank');
+INSERT INTO views_and_triggers_hash VALUES('blank');
 
 DROP VIEW public.proto_checkpoints_view;
 DROP VIEW public.checkpoints_view;

master/static/db_code/README.md → master/static/views_and_triggers/README.md

@@ -1,27 +1,27 @@
-### What is db_code?
+### What is views_and_triggers?
 
-```db_code``` is a way to avoid needing to make really difficult changes to update database code (views / triggers / functions).
+```views_and_triggers``` is a way to avoid needing to make really difficult changes to update database code (views / triggers / functions). Any stateless database thing can be stored here. The name is not 100% accurate.
 
 This migration is an example of what we want to avoid. A change adding a field should just add a field.
 
 https://github.com/determined-ai/determined/blob/main/master/static/migrations/20230306115327_add-checkpoint-size.tx.up.sql
 
-### How do I use db_code?
+### How do I use views_and_triggers?
 
 If you need to change views / other db code, just change them and the changes will apply next time master restarts.
 
 If you need to delete views / other db code, just delete them from the file.
 
 If you need to add a view, just make sure you add it into the ```determined_code``` schema (aka do ```CREATE VIEW determined_code.test ...```). Exception are triggers inherit from the table, therefore just create them without the schema name. The procedure being executed should still be in the ```determined_code``` so that way when the ```determined_code``` schema get's dropped the trigger will get cascaded.
 
-### How does db_code work?
+### How does dbviews_and_triggers work?
 
 On everytime the Determined master starts up
 
 - The Postgres schema ```determined_code``` will be dropped if it exists.
 - Migrations run as they did before ```determined_code``` was added.
 - The Postgres schema ```determined_code``` will be recreated.
-- All SQL files in the ``db_code`` will be run in lexicographically order.
+- All SQL files in the ``views_and_triggers`` will be run in lexicographical order.
 
 ### Limitations