From 322370940ea913f5b1fc359b246b48d3f2cca682 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Mon, 4 Mar 2024 16:00:09 -0500 Subject: [PATCH 1/4] Add deprecation warning and test --- services/horizon/internal/flags.go | 5 +++ .../internal/integration/parameters_test.go | 33 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/services/horizon/internal/flags.go b/services/horizon/internal/flags.go index 87deb28c48..64ad7f340e 100644 --- a/services/horizon/internal/flags.go +++ b/services/horizon/internal/flags.go @@ -242,6 +242,11 @@ func Flags() (*Config, support.ConfigOptions) { Usage: `when enabled, Horizon ingestion will instruct the captive core invocation to use an external db url for ledger states rather than in memory(RAM). Will result in several GB of space shifting out of RAM and to the external db persistence. The external db url is determined by the presence of DATABASE parameter in the captive-core-config-path or if absent, the db will default to sqlite and the db file will be stored at location derived from captive-core-storage-path parameter.`, CustomSetValue: func(opt *support.ConfigOption) error { if val := viper.GetBool(opt.Name); val { + stdLog.Printf( + "DEPRECATED - The usage of the flag --captive-core-use-db has been deprecated. This in-memory " + + "functionality will soon be removed in the future. We recommend keeping the default value of True, " + + "to use the external DB rather than in-memory mode. ", + ) config.CaptiveCoreConfigUseDB = val config.CaptiveCoreTomlParams.UseDB = val } diff --git a/services/horizon/internal/integration/parameters_test.go b/services/horizon/internal/integration/parameters_test.go index ebe3c3bfda..25f3f8d70a 100644 --- a/services/horizon/internal/integration/parameters_test.go +++ b/services/horizon/internal/integration/parameters_test.go @@ -541,6 +541,39 @@ func TestDeprecatedOutputs(t *testing.T) { "Configuring section in the developer documentation on how to use them - "+ "https://developers.stellar.org/docs/run-api-server/configuring") }) + t.Run("deprecated output for --captive-core-use-db", func(t *testing.T) { + originalStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + stdLog.SetOutput(os.Stderr) + + testConfig := integration.GetTestConfig() + testConfig.HorizonIngestParameters = map[string]string{"captive-core-use-db": "false"} + test := integration.NewTest(t, *testConfig) + err := test.StartHorizon() + assert.NoError(t, err) + test.WaitForHorizon() + + // Use a wait group to wait for the goroutine to finish before proceeding + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + if err := w.Close(); err != nil { + t.Errorf("Failed to close Stdout") + return + } + }() + + outputBytes, _ := io.ReadAll(r) + wg.Wait() // Wait for the goroutine to finish before proceeding + _ = r.Close() + os.Stderr = originalStderr + + assert.Contains(t, string(outputBytes), "DEPRECATED - The usage of the flag --captive-core-use-db has been deprecated. This in-memory "+ + "functionality will soon be removed in the future. We recommend keeping the default value of True, "+ + "to use the external DB rather than in-memory mode. ") + }) } func TestGlobalFlagsOutput(t *testing.T) { From 11e62e0f2cde3b808b6793315288067eee22cdde Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Wed, 6 Mar 2024 17:40:05 -0500 Subject: [PATCH 2/4] Update services/horizon/internal/flags.go Co-authored-by: shawn --- services/horizon/internal/flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/horizon/internal/flags.go b/services/horizon/internal/flags.go index 41162da114..96844878f1 100644 --- a/services/horizon/internal/flags.go +++ b/services/horizon/internal/flags.go @@ -243,7 +243,7 @@ func Flags() (*Config, support.ConfigOptions) { CustomSetValue: func(opt *support.ConfigOption) error { if val := viper.GetBool(opt.Name); val { stdLog.Printf( - "DEPRECATED - The usage of the flag --captive-core-use-db has been deprecated. This in-memory " + +The usage of the flag --captive-core-use-db has been deprecated. Setting it to false to achieve in-memory functionality on captive core will be removed in future releases. We recommend removing usage of this flag now in preparation. "functionality will soon be removed in the future. We recommend keeping the default value of True, " + "to use the external DB rather than in-memory mode. ", ) From 1f64adefcf996ecbd1973077be4ddfbbd288b890 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Wed, 6 Mar 2024 17:42:14 -0500 Subject: [PATCH 3/4] Update parameters_test.go --- services/horizon/internal/integration/parameters_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/horizon/internal/integration/parameters_test.go b/services/horizon/internal/integration/parameters_test.go index 25f3f8d70a..caa54ad639 100644 --- a/services/horizon/internal/integration/parameters_test.go +++ b/services/horizon/internal/integration/parameters_test.go @@ -570,9 +570,9 @@ func TestDeprecatedOutputs(t *testing.T) { _ = r.Close() os.Stderr = originalStderr - assert.Contains(t, string(outputBytes), "DEPRECATED - The usage of the flag --captive-core-use-db has been deprecated. This in-memory "+ - "functionality will soon be removed in the future. We recommend keeping the default value of True, "+ - "to use the external DB rather than in-memory mode. ") + assert.Contains(t, string(outputBytes), "The usage of the flag --captive-core-use-db has been deprecated. "+ + "Setting it to false to achieve in-memory functionality on captive core will be removed in future releases. "+ + "We recommend removing usage of this flag now in preparation.") }) } From 5821c1271d9ec819c4c4a6516ff7b65c2f17600e Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Mon, 25 Mar 2024 11:11:58 -0400 Subject: [PATCH 4/4] Change the printed message --- services/horizon/internal/flags.go | 8 +++----- services/horizon/internal/integration/parameters_test.go | 4 ++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/services/horizon/internal/flags.go b/services/horizon/internal/flags.go index 2f4a7ff821..54e76fbc56 100644 --- a/services/horizon/internal/flags.go +++ b/services/horizon/internal/flags.go @@ -245,11 +245,9 @@ func Flags() (*Config, support.ConfigOptions) { Usage: `when enabled, Horizon ingestion will instruct the captive core invocation to use an external db url for ledger states rather than in memory(RAM). Will result in several GB of space shifting out of RAM and to the external db persistence. The external db url is determined by the presence of DATABASE parameter in the captive-core-config-path or if absent, the db will default to sqlite and the db file will be stored at location derived from captive-core-storage-path parameter.`, CustomSetValue: func(opt *support.ConfigOption) error { if val := viper.GetBool(opt.Name); val { - stdLog.Printf( -The usage of the flag --captive-core-use-db has been deprecated. Setting it to false to achieve in-memory functionality on captive core will be removed in future releases. We recommend removing usage of this flag now in preparation. - "functionality will soon be removed in the future. We recommend keeping the default value of True, " + - "to use the external DB rather than in-memory mode. ", - ) + stdLog.Printf("The usage of the flag --captive-core-use-db has been deprecated. " + + "Setting it to false to achieve in-memory functionality on captive core will be removed in " + + "future releases. We recommend removing usage of this flag now in preparation.") config.CaptiveCoreConfigUseDB = val config.CaptiveCoreTomlParams.UseDB = val } diff --git a/services/horizon/internal/integration/parameters_test.go b/services/horizon/internal/integration/parameters_test.go index caa54ad639..7d487c556f 100644 --- a/services/horizon/internal/integration/parameters_test.go +++ b/services/horizon/internal/integration/parameters_test.go @@ -571,8 +571,8 @@ func TestDeprecatedOutputs(t *testing.T) { os.Stderr = originalStderr assert.Contains(t, string(outputBytes), "The usage of the flag --captive-core-use-db has been deprecated. "+ - "Setting it to false to achieve in-memory functionality on captive core will be removed in future releases. "+ - "We recommend removing usage of this flag now in preparation.") + "Setting it to false to achieve in-memory functionality on captive core will be removed in "+ + "future releases. We recommend removing usage of this flag now in preparation.") }) }