From 97f248dfe2c755c405d864e228dae62271b2c918 Mon Sep 17 00:00:00 2001 From: tamirms Date: Mon, 24 May 2021 18:24:21 +0100 Subject: [PATCH] services/horizon/cmd: Fix prepare range bug (#3625) The `horizon db reingest range` command failed because it requires that the top level horizon ingest flag to be true. In other words, `horizon --ingest db reingest range` works but `horizon db reingest range` responds with the following error: 2021/05/03 12:49:43 Invalid config: one or more captive core params passed (--stellar-core-binary-path or --captive-core-config-append-path) but --ingest not set. This PR fixes this bug by making all ingestion subcommands set ingest to true implicitly. This PR also fixes the captive core config validation by only requiring the config file to be present when ingesting in online mode. It is not necessary to have a captive core config file when ingesting bounded ledger ranges. --- services/horizon/CHANGELOG.md | 1 + services/horizon/cmd/db.go | 16 +++++----- services/horizon/cmd/ingest.go | 10 +++--- services/horizon/cmd/root.go | 6 ++-- services/horizon/cmd/serve.go | 2 +- services/horizon/cmd/version.go | 2 +- services/horizon/internal/flags.go | 31 ++++++++++++++----- .../horizon/internal/integration/db_test.go | 29 ++++++++++++++++- 8 files changed, 72 insertions(+), 25 deletions(-) diff --git a/services/horizon/CHANGELOG.md b/services/horizon/CHANGELOG.md index cbdf557130..688de695cf 100644 --- a/services/horizon/CHANGELOG.md +++ b/services/horizon/CHANGELOG.md @@ -9,6 +9,7 @@ file. This project adheres to [Semantic Versioning](http://semver.org/). * Add more in-depth Prometheus metrics (count & duration) for db queries. +* Fix bug in `horizon db reingest range` command which required the `--ingest` flag to be set [3625](https://github.com/stellar/go/pull/3625)). ## v2.3.0 diff --git a/services/horizon/cmd/db.go b/services/horizon/cmd/db.go index 91957b4062..7cc5b3d1c9 100644 --- a/services/horizon/cmd/db.go +++ b/services/horizon/cmd/db.go @@ -202,16 +202,18 @@ var dbReingestRangeCmd = &cobra.Command{ argsUInt32 := make([]uint32, 2) for i, arg := range args { - seq, err := strconv.Atoi(arg) - if err != nil { + if seq, err := strconv.Atoi(arg); err != nil { cmd.Usage() log.Fatalf(`Invalid sequence number "%s"`, arg) + } else if seq < 0 { + log.Fatalf("sequence number %s cannot be negative", arg) + } else { + argsUInt32[i] = uint32(seq) } - argsUInt32[i] = uint32(seq) } - horizon.ApplyFlags(config, flags) - err := RunDBReingestRange(argsUInt32[0], argsUInt32[1], reingestForce, parallelWorkers, *config) + horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true}) + err := runDBReingestRange(argsUInt32[0], argsUInt32[1], reingestForce, parallelWorkers, *config) if err != nil { if errors.Cause(err) == ingest.ErrReingestRangeConflict { message := ` @@ -232,7 +234,7 @@ var dbReingestRangeCmd = &cobra.Command{ }, } -func RunDBReingestRange(from, to uint32, reingestForce bool, parallelWorkers uint, config horizon.Config) error { +func runDBReingestRange(from, to uint32, reingestForce bool, parallelWorkers uint, config horizon.Config) error { if reingestForce && parallelWorkers > 1 { return errors.New("--force is incompatible with --parallel-workers > 1") } @@ -303,7 +305,7 @@ func init() { viper.BindPFlags(dbReingestRangeCmd.PersistentFlags()) - rootCmd.AddCommand(dbCmd) + RootCmd.AddCommand(dbCmd) dbCmd.AddCommand( dbInitCmd, dbMigrateCmd, diff --git a/services/horizon/cmd/ingest.go b/services/horizon/cmd/ingest.go index d66aad2676..fab195fac2 100644 --- a/services/horizon/cmd/ingest.go +++ b/services/horizon/cmd/ingest.go @@ -71,7 +71,7 @@ var ingestVerifyRangeCmd = &cobra.Command{ co.SetValue() } - horizon.ApplyFlags(config, flags) + horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true}) if ingestVerifyDebugServerPort != 0 { go func() { @@ -170,7 +170,7 @@ var ingestStressTestCmd = &cobra.Command{ co.SetValue() } - horizon.ApplyFlags(config, flags) + horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true}) horizonSession, err := db.Open("postgres", config.DatabaseURL) if err != nil { @@ -229,7 +229,7 @@ var ingestTriggerStateRebuildCmd = &cobra.Command{ Short: "updates a database to trigger state rebuild, state will be rebuilt by a running Horizon instance, DO NOT RUN production DB, some endpoints will be unavailable until state is rebuilt", Run: func(cmd *cobra.Command, args []string) { ctx := context.Background() - horizon.ApplyFlags(config, flags) + horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true}) horizonSession, err := db.Open("postgres", config.DatabaseURL) if err != nil { @@ -251,7 +251,7 @@ var ingestInitGenesisStateCmd = &cobra.Command{ Short: "ingests genesis state (ledger 1)", Run: func(cmd *cobra.Command, args []string) { ctx := context.Background() - horizon.ApplyFlags(config, flags) + horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true}) horizonSession, err := db.Open("postgres", config.DatabaseURL) if err != nil { @@ -322,7 +322,7 @@ func init() { viper.BindPFlags(ingestVerifyRangeCmd.PersistentFlags()) - rootCmd.AddCommand(ingestCmd) + RootCmd.AddCommand(ingestCmd) ingestCmd.AddCommand( ingestVerifyRangeCmd, ingestStressTestCmd, diff --git a/services/horizon/cmd/root.go b/services/horizon/cmd/root.go index 5f6a88f862..92f5980256 100644 --- a/services/horizon/cmd/root.go +++ b/services/horizon/cmd/root.go @@ -12,7 +12,7 @@ import ( var ( config, flags = horizon.Flags() - rootCmd = &cobra.Command{ + RootCmd = &cobra.Command{ Use: "horizon", Short: "client-facing api server for the Stellar network", Long: "Client-facing API server for the Stellar network. It acts as the interface between Stellar Core and applications that want to access the Stellar network. It allows you to submit transactions to the network, check the status of accounts, subscribe to event streams and more.", @@ -23,14 +23,14 @@ var ( ) func init() { - err := flags.Init(rootCmd) + err := flags.Init(RootCmd) if err != nil { stdLog.Fatal(err.Error()) } } func Execute() { - if err := rootCmd.Execute(); err != nil { + if err := RootCmd.Execute(); err != nil { fmt.Println(err) os.Exit(1) } diff --git a/services/horizon/cmd/serve.go b/services/horizon/cmd/serve.go index 812f07f4c3..f88e2d6e8e 100644 --- a/services/horizon/cmd/serve.go +++ b/services/horizon/cmd/serve.go @@ -15,5 +15,5 @@ var serveCmd = &cobra.Command{ } func init() { - rootCmd.AddCommand(serveCmd) + RootCmd.AddCommand(serveCmd) } diff --git a/services/horizon/cmd/version.go b/services/horizon/cmd/version.go index 3187c73daa..cbfedb0a66 100644 --- a/services/horizon/cmd/version.go +++ b/services/horizon/cmd/version.go @@ -19,5 +19,5 @@ var versionCmd = &cobra.Command{ } func init() { - rootCmd.AddCommand(versionCmd) + RootCmd.AddCommand(versionCmd) } diff --git a/services/horizon/internal/flags.go b/services/horizon/internal/flags.go index 9a5831cebc..f534bb333d 100644 --- a/services/horizon/internal/flags.go +++ b/services/horizon/internal/flags.go @@ -428,7 +428,7 @@ func Flags() (*Config, support.ConfigOptions) { // NewAppFromFlags constructs a new Horizon App from the given command line flags func NewAppFromFlags(config *Config, flags support.ConfigOptions) *App { - ApplyFlags(config, flags) + ApplyFlags(config, flags, ApplyOptions{RequireCaptiveCoreConfig: true, AlwaysIngest: false}) // Validate app-specific arguments if config.StellarCoreURL == "" { log.Fatalf("flag --%s cannot be empty", StellarCoreURLFlagName) @@ -443,8 +443,13 @@ func NewAppFromFlags(config *Config, flags support.ConfigOptions) *App { return app } +type ApplyOptions struct { + AlwaysIngest bool + RequireCaptiveCoreConfig bool +} + // ApplyFlags applies the command line flags on the given Config instance -func ApplyFlags(config *Config, flags support.ConfigOptions) { +func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOptions) { // Verify required options and load the config struct flags.Require() flags.SetValues() @@ -452,6 +457,10 @@ func ApplyFlags(config *Config, flags support.ConfigOptions) { // Validate options that should be provided together validateBothOrNeither("tls-cert", "tls-key") + if options.AlwaysIngest { + config.Ingest = true + } + 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 @@ -492,11 +501,19 @@ func ApplyFlags(config *Config, flags support.ConfigOptions) { } if config.RemoteCaptiveCoreURL == "" && (binaryPath == "" || config.CaptiveCoreConfigPath == "") { - stdLog.Fatalf("Invalid config: captive core requires that both --%s and --%s are set. %s", - StellarCoreBinaryPathName, CaptiveCoreConfigAppendPathName, captiveCoreMigrationHint) - } - - if config.RemoteCaptiveCoreURL == "" { + if options.RequireCaptiveCoreConfig { + stdLog.Fatalf("Invalid config: captive core requires that both --%s and --%s are set. %s", + StellarCoreBinaryPathName, CaptiveCoreConfigAppendPathName, captiveCoreMigrationHint) + } else { + var err error + config.CaptiveCoreTomlParams.HistoryArchiveURLs = config.HistoryArchiveURLs + config.CaptiveCoreTomlParams.NetworkPassphrase = config.NetworkPassphrase + config.CaptiveCoreToml, err = ledgerbackend.NewCaptiveCoreToml(config.CaptiveCoreTomlParams) + if err != nil { + stdLog.Fatalf("Invalid captive core toml file %v", err) + } + } + } else if config.RemoteCaptiveCoreURL == "" { var err error config.CaptiveCoreTomlParams.HistoryArchiveURLs = config.HistoryArchiveURLs config.CaptiveCoreTomlParams.NetworkPassphrase = config.NetworkPassphrase diff --git a/services/horizon/internal/integration/db_test.go b/services/horizon/internal/integration/db_test.go index 572710d703..e94a3440fc 100644 --- a/services/horizon/internal/integration/db_test.go +++ b/services/horizon/internal/integration/db_test.go @@ -1,7 +1,9 @@ package integration import ( + "fmt" "path/filepath" + "strconv" "testing" "time" @@ -85,9 +87,34 @@ func TestReingestDB(t *testing.T) { filepath.Dir(horizonConfig.CaptiveCoreConfigPath), "captive-core-reingest-range-integration-tests.cfg", ) + horizoncmd.RootCmd.SetArgs([]string{ + "--stellar-core-url", + horizonConfig.StellarCoreURL, + "--history-archive-urls", + horizonConfig.HistoryArchiveURLs[0], + "--db-url", + horizonConfig.DatabaseURL, + "--stellar-core-db-url", + horizonConfig.StellarCoreDatabaseURL, + "--stellar-core-binary-path", + horizonConfig.CaptiveCoreBinaryPath, + "--captive-core-config-append-path", + horizonConfig.CaptiveCoreConfigPath, + "--enable-captive-core-ingestion=" + strconv.FormatBool(horizonConfig.EnableCaptiveCoreIngestion), + "--network-passphrase", + horizonConfig.NetworkPassphrase, + // due to ARTIFICIALLY_ACCELERATE_TIME_FOR_TESTING + "--checkpoint-frequency", + "8", + "db", + "reingest", + "range", + "1", + fmt.Sprintf("%d", toLedger), + }) + err = horizoncmd.RootCmd.Execute() // Reingest into the DB - err = horizoncmd.RunDBReingestRange(1, toLedger, false, 1, horizonConfig) tt.NoError(err) }