19709: Explain design in comments. Improve migration filename check.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/lib/boot/rails_db.go b/lib/boot/rails_db.go
index 3f815114457952dc6ce676eb35cbb70d655f0cb1..44ceb263c370f04b9cb5897d3f10c72d18a22fa8 100644 (file)
--- a/lib/boot/rails_db.go
+++ b/lib/boot/rails_db.go
@@ -21,6 +21,10 @@ func (runner railsDatabase) String() string {
        return "railsDatabase"
 }
 
+// Run checks for and applies any pending Rails database migrations.
+//
+// If running a dev/test environment, and the database is empty, it
+// initializes the database.
 func (runner railsDatabase) Run(ctx context.Context, fail func(error), super *Supervisor) error {
        err := super.wait(ctx, runPostgreSQL{}, installPassenger{src: "services/api"})
        if err != nil {
@@ -35,11 +39,33 @@ func (runner railsDatabase) Run(ctx context.Context, fail func(error), super *Su
                appdir = filepath.Join(super.SourcePath, "services/api")
        }
 
-       // list versions in db/migrate/{version}_{name}.rb
+       // Check for pending migrations before running rake.
+       //
+       // In principle, we could use "rake db:migrate:status" or skip
+       // this check entirely and let "rake db:migrate" be a no-op
+       // most of the time.  However, in the most common case when
+       // there are no new migrations, that would add ~2s to startup
+       // time / downtime during service restart.
+
        todo := map[string]bool{}
+
+       // list versions in db/migrate/{version}_{name}.rb
        fs.WalkDir(os.DirFS(appdir), "db/migrate", func(path string, d fs.DirEntry, err error) error {
-               if cut := strings.Index(d.Name(), "_"); cut > 0 && strings.HasSuffix(d.Name(), ".rb") {
-                       todo[d.Name()[:cut]] = true
+               fnm := d.Name()
+               if !strings.HasSuffix(fnm, ".rb") {
+                       return nil
+               }
+               for i, c := range fnm {
+                       if i > 0 && c == '_' {
+                               todo[fnm[:i]] = true
+                               break
+                       }
+                       if c < '0' || c > '9' {
+                               // non-numeric character before the
+                               // first '_' means this is not a
+                               // migration
+                               break
+                       }
                }
                return nil
        })