[arvados] created: 2.1.0-3172-g228028e52
git repository hosting
git at public.arvados.org
Mon Dec 12 21:46:13 UTC 2022
at 228028e523983a847239fd4a5530d3e54b27cfa5 (commit)
commit 228028e523983a847239fd4a5530d3e54b27cfa5
Author: Tom Clegg <tom at curii.com>
Date: Mon Dec 12 16:46:03 2022 -0500
19709: Test schema_migrations vs. db/migrate/*.rb.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/boot/rails_db.go b/lib/boot/rails_db.go
index 44ceb263c..ad9124d7d 100644
--- a/lib/boot/rails_db.go
+++ b/lib/boot/rails_db.go
@@ -6,6 +6,7 @@ package boot
import (
"context"
+ "fmt"
"io/fs"
"os"
"path/filepath"
@@ -47,28 +48,10 @@ func (runner railsDatabase) Run(ctx context.Context, fail func(error), super *Su
// 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 {
- 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
- })
+ todo, err := migrationList(appdir)
+ if err != nil {
+ return err
+ }
// read schema_migrations table (list of migrations already
// applied) and remove those entries from todo
@@ -113,3 +96,35 @@ func (runner railsDatabase) Run(ctx context.Context, fail func(error), super *Su
defer dblock.RailsMigrations.Unlock()
return super.RunProgram(ctx, appdir, runOptions{env: railsEnv}, "bundle", "exec", "rake", "db:migrate")
}
+
+func migrationList(dir string) (map[string]bool, error) {
+ todo := map[string]bool{}
+
+ // list versions in db/migrate/{version}_{name}.rb
+ err := fs.WalkDir(os.DirFS(dir), "db/migrate", func(path string, d fs.DirEntry, err error) error {
+ if d.IsDir() {
+ return nil
+ }
+ fnm := d.Name()
+ if !strings.HasSuffix(fnm, ".rb") {
+ return fmt.Errorf("unexpected file in db/migrate dir: %s", fnm)
+ }
+ 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
+ return fmt.Errorf("unexpected file in db/migrate dir: %s", fnm)
+ }
+ }
+ return nil
+ })
+ if err != nil {
+ return nil, err
+ }
+ return todo, nil
+}
diff --git a/lib/boot/rails_db_test.go b/lib/boot/rails_db_test.go
new file mode 100644
index 000000000..ac78a3c9a
--- /dev/null
+++ b/lib/boot/rails_db_test.go
@@ -0,0 +1,47 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package boot
+
+import (
+ "git.arvados.org/arvados.git/lib/config"
+ "git.arvados.org/arvados.git/sdk/go/arvadostest"
+ "git.arvados.org/arvados.git/sdk/go/ctxlog"
+ "gopkg.in/check.v1"
+)
+
+type railsDBSuite struct{}
+
+var _ = check.Suite(&railsDBSuite{})
+
+// Check services/api/db/migrate/*.rb match schema_migrations
+func (s *railsDBSuite) TestMigrationList(c *check.C) {
+ todo, err := migrationList("../../services/api")
+ c.Check(err, check.IsNil)
+ c.Check(todo["20220804133317"], check.Equals, true)
+
+ cfg, err := config.NewLoader(nil, ctxlog.TestLogger(c)).Load()
+ c.Assert(err, check.IsNil)
+ cluster, err := cfg.GetCluster("")
+ c.Assert(err, check.IsNil)
+ db := arvadostest.DB(c, cluster)
+ rows, err := db.Query(`SELECT version FROM schema_migrations`)
+ for rows.Next() {
+ var v string
+ err = rows.Scan(&v)
+ c.Assert(err, check.IsNil)
+ if !todo[v] {
+ c.Errorf("version is in schema_migrations but not services/api/db/migrate/: %q", v)
+ }
+ delete(todo, v)
+ }
+ err = rows.Close()
+ c.Assert(err, check.IsNil)
+
+ // In the test suite, the database should be fully migrated.
+ // So, if there's anything left in todo here, there is
+ // something wrong with our "db/migrate/*.rb ==
+ // schema_migrations" reasoning.
+ c.Check(todo, check.HasLen, 0)
+}
commit 31d3138660ee51eccaceb52f99730b6124755b93
Author: Tom Clegg <tom at curii.com>
Date: Fri Dec 9 15:01:17 2022 -0500
19709: Explain design in comments. Improve migration filename check.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/boot/rails_db.go b/lib/boot/rails_db.go
index 3f8151144..44ceb263c 100644
--- 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
})
commit 41b9b83812826b77034fe13ea34047e194b1027f
Author: Tom Clegg <tom at curii.com>
Date: Mon Nov 14 15:31:42 2022 -0500
19709: Apply pending rails migrations at service start/restart.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/boot/rails_db.go b/lib/boot/rails_db.go
new file mode 100644
index 000000000..3f8151144
--- /dev/null
+++ b/lib/boot/rails_db.go
@@ -0,0 +1,89 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package boot
+
+import (
+ "context"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+
+ "git.arvados.org/arvados.git/lib/controller/dblock"
+ "git.arvados.org/arvados.git/lib/ctrlctx"
+)
+
+type railsDatabase struct{}
+
+func (runner railsDatabase) String() string {
+ return "railsDatabase"
+}
+
+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 {
+ return err
+ }
+
+ // determine path to installed rails app or source tree
+ var appdir string
+ if super.ClusterType == "production" {
+ appdir = "/var/lib/arvados/railsapi"
+ } else {
+ appdir = filepath.Join(super.SourcePath, "services/api")
+ }
+
+ // list versions in db/migrate/{version}_{name}.rb
+ todo := map[string]bool{}
+ 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
+ }
+ return nil
+ })
+
+ // read schema_migrations table (list of migrations already
+ // applied) and remove those entries from todo
+ dbconnector := ctrlctx.DBConnector{PostgreSQL: super.cluster.PostgreSQL}
+ defer dbconnector.Close()
+ db, err := dbconnector.GetDB(ctx)
+ if err != nil {
+ return err
+ }
+ rows, err := db.QueryContext(ctx, `SELECT version FROM schema_migrations`)
+ if err != nil {
+ if super.ClusterType == "production" {
+ return err
+ }
+ super.logger.WithError(err).Info("schema_migrations query failed, trying db:setup")
+ return super.RunProgram(ctx, "services/api", runOptions{env: railsEnv}, "bundle", "exec", "rake", "db:setup")
+ }
+ for rows.Next() {
+ var v string
+ err = rows.Scan(&v)
+ if err != nil {
+ return err
+ }
+ delete(todo, v)
+ }
+ err = rows.Close()
+ if err != nil {
+ return err
+ }
+
+ // if nothing remains in todo, all available migrations are
+ // done, so return without running any [relatively slow]
+ // ruby/rake commands
+ if len(todo) == 0 {
+ return nil
+ }
+
+ super.logger.Infof("%d migrations pending", len(todo))
+ if !dblock.RailsMigrations.Lock(ctx, dbconnector.GetDB) {
+ return context.Canceled
+ }
+ defer dblock.RailsMigrations.Unlock()
+ return super.RunProgram(ctx, appdir, runOptions{env: railsEnv}, "bundle", "exec", "rake", "db:migrate")
+}
diff --git a/lib/boot/seed.go b/lib/boot/seed.go
deleted file mode 100644
index b43d90720..000000000
--- a/lib/boot/seed.go
+++ /dev/null
@@ -1,31 +0,0 @@
-// Copyright (C) The Arvados Authors. All rights reserved.
-//
-// SPDX-License-Identifier: AGPL-3.0
-
-package boot
-
-import (
- "context"
-)
-
-// Populate a blank database with arvados tables and seed rows.
-type seedDatabase struct{}
-
-func (seedDatabase) String() string {
- return "seedDatabase"
-}
-
-func (seedDatabase) Run(ctx context.Context, fail func(error), super *Supervisor) error {
- err := super.wait(ctx, runPostgreSQL{}, installPassenger{src: "services/api"})
- if err != nil {
- return err
- }
- if super.ClusterType == "production" {
- return nil
- }
- err = super.RunProgram(ctx, "services/api", runOptions{env: railsEnv}, "bundle", "exec", "rake", "db:setup")
- if err != nil {
- return err
- }
- return nil
-}
diff --git a/lib/boot/supervisor.go b/lib/boot/supervisor.go
index ca88653fa..0f0600f18 100644
--- a/lib/boot/supervisor.go
+++ b/lib/boot/supervisor.go
@@ -356,20 +356,24 @@ func (super *Supervisor) runCluster() error {
createCertificates{},
runPostgreSQL{},
runNginx{},
- runServiceCommand{name: "controller", svc: super.cluster.Services.Controller, depends: []supervisedTask{seedDatabase{}}},
+ railsDatabase{},
+ runServiceCommand{name: "controller", svc: super.cluster.Services.Controller, depends: []supervisedTask{railsDatabase{}}},
runServiceCommand{name: "git-httpd", svc: super.cluster.Services.GitHTTP},
runServiceCommand{name: "health", svc: super.cluster.Services.Health},
runServiceCommand{name: "keepproxy", svc: super.cluster.Services.Keepproxy, depends: []supervisedTask{runPassenger{src: "services/api"}}},
runServiceCommand{name: "keepstore", svc: super.cluster.Services.Keepstore},
runServiceCommand{name: "keep-web", svc: super.cluster.Services.WebDAV},
- runServiceCommand{name: "ws", svc: super.cluster.Services.Websocket, depends: []supervisedTask{seedDatabase{}}},
+ runServiceCommand{name: "ws", svc: super.cluster.Services.Websocket, depends: []supervisedTask{railsDatabase{}}},
installPassenger{src: "services/api", varlibdir: "railsapi"},
- runPassenger{src: "services/api", varlibdir: "railsapi", svc: super.cluster.Services.RailsAPI, depends: []supervisedTask{createCertificates{}, seedDatabase{}, installPassenger{src: "services/api", varlibdir: "railsapi"}}},
- seedDatabase{},
+ runPassenger{src: "services/api", varlibdir: "railsapi", svc: super.cluster.Services.RailsAPI, depends: []supervisedTask{
+ createCertificates{},
+ installPassenger{src: "services/api", varlibdir: "railsapi"},
+ railsDatabase{},
+ }},
}
if !super.NoWorkbench1 {
tasks = append(tasks,
- installPassenger{src: "apps/workbench", varlibdir: "workbench1", depends: []supervisedTask{seedDatabase{}}}, // dependency ensures workbench doesn't delay api install/startup
+ installPassenger{src: "apps/workbench", varlibdir: "workbench1", depends: []supervisedTask{railsDatabase{}}}, // dependency ensures workbench doesn't delay api install/startup
runPassenger{src: "apps/workbench", varlibdir: "workbench1", svc: super.cluster.Services.Workbench1, depends: []supervisedTask{installPassenger{src: "apps/workbench", varlibdir: "workbench1"}}},
)
}
diff --git a/lib/controller/dblock/dblock.go b/lib/controller/dblock/dblock.go
index ad2733abf..c59bcef0b 100644
--- a/lib/controller/dblock/dblock.go
+++ b/lib/controller/dblock/dblock.go
@@ -22,6 +22,7 @@ var (
KeepBalanceService = &DBLocker{key: 10003} // keep-balance service in periodic-sweep loop
KeepBalanceActive = &DBLocker{key: 10004} // keep-balance sweep in progress (either -once=true or service loop)
Dispatch = &DBLocker{key: 10005} // any dispatcher running
+ RailsMigrations = &DBLocker{key: 10006}
retryDelay = 5 * time.Second
)
diff --git a/lib/ctrlctx/db.go b/lib/ctrlctx/db.go
index 2a05096ce..b711b3e65 100644
--- a/lib/ctrlctx/db.go
+++ b/lib/ctrlctx/db.go
@@ -168,8 +168,20 @@ func (dbc *DBConnector) GetDB(ctx context.Context) (*sqlx.DB, error) {
}
if err := db.Ping(); err != nil {
ctxlog.FromContext(ctx).WithError(err).Error("postgresql connect succeeded but ping failed")
+ db.Close()
return nil, errDBConnection
}
dbc.pgdb = db
return db, nil
}
+
+func (dbc *DBConnector) Close() error {
+ dbc.mtx.Lock()
+ defer dbc.mtx.Unlock()
+ var err error
+ if dbc.pgdb != nil {
+ err = dbc.pgdb.Close()
+ dbc.pgdb = nil
+ }
+ return err
+}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list