cmd/icingadb: automatically upgrade SQL schema #770
Conversation
| return database.AutoUpgradeSchema( | ||
| context.Background(), db, cmd.Config.Database.Database, "icingadb_schema", "version", "timestamp", | ||
| map[string]database.SchemaData{ | ||
| database.MySQL: { | ||
| Schema: []string{mysql.Schema}, | ||
| Upgrades: []database.SchemaUpgrade{ | ||
| {Version: "3", DDL: []string{mysql.Upgrade100}}, | ||
| {Version: "4", DDL: []string{mysql.Upgrade111}}, | ||
| {Version: "5", DDL: []string{mysql.Upgrade120}}, | ||
| }, | ||
| }, | ||
| database.PostgreSQL: { | ||
| Schema: []string{pgsql.Schema}, | ||
| Upgrades: []database.SchemaUpgrade{ | ||
| {Version: "2", DDL: []string{pgsql.Upgrade111}}, | ||
| {Version: "3", DDL: []string{pgsql.Upgrade120}}, |
There was a problem hiding this comment.
- @lippserd This is a raw* usage example of Introduce database.AutoUpgradeSchema() icinga-go-library#15
Do you consider the design fine?
* One has to maintain such schema file lists. A non-raw usage requires the specific project to have a function which auto-calculates schema upgrade lists from the embedded tree of schema/.
There was a problem hiding this comment.
Generally speaking, I am against embedding both the schema files and all schema upgrades in Icinga DB as they are being installed as part of the package. This only bloats up the binary.
There was a problem hiding this comment.
OK, then let's don't ship the separate files. 👍
| }, | ||
| retry.Retryable, | ||
| backoff.NewExponentialWithJitter(128*time.Millisecond, time.Minute), | ||
| db.GetDefaultRetrySettings(), |
There was a problem hiding this comment.
Please correct me if I'm wrong, but this would force (multiple) schema upgrades to be performed in five minutes. If this takes more time, the user ends up in a very messy situation, potentially don't even knowing how they got there.
There were already long running schema upgrades in the past, https://icinga.com/docs/icinga-db/latest/doc/04-Upgrading/#upgrading-to-icinga-db-v120.
| err := retry.WithBackoff( | ||
| context.Background(), | ||
| func(ctx context.Context) (err error) { | ||
| return database.AutoUpgradeSchema( |
There was a problem hiding this comment.
I would love to see some logging here. The AutoUpgradeSchema function just starts upgrading the schema without any notice.
From a user's point of view, Icinga DB just "blocked" and Icinga Web will hint that Icinga DB might have stopped. Thus, even when a user have consulted the logs and sees nothing, they will likely restart Icinga DB and crash the migration.
No description provided.