services/horizon: Apply and check migrations on ingesting instances o…

…nly (#3586)

This commit reverts 7c92b23 and changes the DB migrations behaviour to only
trigger on ingesting instances.

When checking if there are any migrations required Horizon calls
`migrate.PlanMigration` which sends `CREATE TABLE gorp_migrations`. This write
query is forbidden when using read only database (like replica).

The new flag in 7c92b23 added unnecessary complexity. Ingesting instances are
guaranteed to have write access to a DB.

Thanks @jacekn for the idea!

services/horizon/internal/config.go

@@ -74,9 +74,6 @@ type Config struct {
 	// ApplyMigrations will apply pending migrations to the horizon database
 	// before starting the horizon service
 	ApplyMigrations bool
-	// SkipMigrationsCheck will skip checking if there are any migrations
-	// required
-	SkipMigrationsCheck bool
 	// CheckpointFrequency establishes how many ledgers exist between checkpoints
 	CheckpointFrequency uint32
 	// BehindCloudflare determines if Horizon instance is behind Cloudflare. In

services/horizon/internal/flags.go

@@ -386,14 +386,6 @@ func Flags() (*Config, support.ConfigOptions) {
 			Required:    false,
 			Usage:       "applies pending migrations before starting horizon",
 		},
-		&support.ConfigOption{
-			Name:        "skip-migrations-check",
-			ConfigKey:   &config.SkipMigrationsCheck,
-			OptType:     types.Bool,
-			FlagDefault: false,
-			Required:    false,
-			Usage:       "skips checking if there are migrations required",
-		},
 		&support.ConfigOption{
 			Name:        "checkpoint-frequency",
 			ConfigKey:   &config.CheckpointFrequency,
@@ -446,19 +438,17 @@ func ApplyFlags(config *Config, flags support.ConfigOptions) {
 	flags.Require()
 	flags.SetValues()
 
-	if config.ApplyMigrations {
-		applyMigrations(*config)
-	}
-
-	// Migrations should be checked as early as possible
-	if !config.SkipMigrationsCheck {
-		checkMigrations(*config)
-	}
-
 	// Validate options that should be provided together
 	validateBothOrNeither("tls-cert", "tls-key")
 
 	if config.Ingest {
+		// Migrations should be checked as early as possible. Apply and check
+		// only on ingesting instances which are required to have write-access
+		// to the DB.
+		if config.ApplyMigrations {
+			applyMigrations(*config)
+		}
+		checkMigrations(*config)
 
 		// config.HistoryArchiveURLs contains a single empty value when empty so using
 		// viper.GetString is easier.