ingest: Reuse Stellar-Core's cached buckets if possible.

.circleci/config.yml

@@ -147,7 +147,7 @@ commands:
     parameters:
       core-version:
         type: string
-        default: "17.0.0-568.ea5878e.xenial~buildtests"
+        default: "17.1.0-600.a5e7028.xenial"
     steps:
       - run:
           name: Install Stellar Core <<#parameters.core-version>> (version <<parameters.core-version>>)<</parameters.core-version>>

ingest/CHANGELOG.md

@@ -5,6 +5,9 @@ All notable changes to this project will be documented in this file. This projec
 
 ## Unreleased
 
+### New Features
+* **Performance improvement**: the Captive Core backend now reuses bucket files whenever it finds existing ones in the corresponding `--captive-core-storage-path` (introduced in [v2.0](#v2.0.0)) rather than generating a one-time temporary sub-directory ([#3670](https://github.com/stellar/go/pull/3670)). Note that taking advantage of this feature requires [Stellar-Core v17.1.0](https://github.com/stellar/stellar-core/releases/tag/v17.1.0) or later.
+
 
 ## v2.0.0
 

ingest/ledgerbackend/stellar_core_runner.go

@@ -68,24 +68,12 @@ type stellarCoreRunner struct {
 	log *log.Entry
 }
 
-func createRandomHexString(n int) string {
-	hex := []rune("abcdef1234567890")
-	b := make([]rune, n)
-	for i := range b {
-		b[i] = hex[rand.Intn(len(hex))]
-	}
-	return string(b)
-}
-
 func newStellarCoreRunner(config CaptiveCoreConfig, mode stellarCoreRunnerMode) (*stellarCoreRunner, error) {
 	// Use the specified directory to store Captive Core's data:
 	//    https://github.com/stellar/go/issues/3437
-	//
-	// However, first we ALWAYS append something to the base storage path,
-	// because we will delete the directory entirely when Horizon stops. We also
-	// add a random suffix in order to ensure that there aren't naming
-	// conflicts.
-	fullStoragePath := path.Join(config.StoragePath, "captive-core-"+createRandomHexString(8))
+	// but be sure to re-use rather than replace it:
+	//    https://github.com/stellar/go/issues/3631
+	fullStoragePath := path.Join(config.StoragePath, "captive-core")
 
 	info, err := os.Stat(fullStoragePath)
 	if os.IsNotExist(err) {
@@ -95,8 +83,7 @@ func newStellarCoreRunner(config CaptiveCoreConfig, mode stellarCoreRunnerMode)
 				"failed to create storage directory (%s)", fullStoragePath))
 		}
 	} else if !info.IsDir() {
-		return nil, errors.New(fmt.Sprintf(
-			"%s is not a directory", fullStoragePath))
+		return nil, errors.New(fmt.Sprintf("%s is not a directory", fullStoragePath))
 	} else if err != nil {
 		return nil, errors.Wrap(err, fmt.Sprintf(
 			"error accessing storage directory: %s", fullStoragePath))
@@ -266,7 +253,7 @@ func (r *stellarCoreRunner) catchup(from, to uint32) error {
 	cmd := r.createCmd(
 		"catchup", rangeArg,
 		"--metadata-output-stream", r.getPipeName(),
-		"--replay-in-memory",
+		"--in-memory",
 	)
 
 	var err error
@@ -397,5 +384,5 @@ func (r *stellarCoreRunner) close() error {
 		r.pipe.Reader.Close()
 	}
 
-	return os.RemoveAll(storagePath)
+	return nil
 }

ingest/ledgerbackend/stellar_core_runner_test.go

@@ -29,7 +29,7 @@ func TestCloseBeforeStart(t *testing.T) {
 
 	assert.NoError(t, runner.close())
 
+	// Directory no longer cleaned up on shutdown (perf. bump in v2.5.0)
 	_, err = os.Stat(tempDir)
-	assert.Error(t, err)
-	assert.True(t, os.IsNotExist(err))
+	assert.NoError(t, err)
 }

services/horizon/CHANGELOG.md

@@ -3,20 +3,25 @@
 All notable changes to this project will be documented in this
 file. This project adheres to [Semantic Versioning](http://semver.org/).
 
+
 ## Unreleased
 
+### New Features
 * Add new command `horizon db detect-gaps`, which detects ingestion gaps in the database. The command prints out the `db reingest` commands to run in order to fill the gaps found. 
 
+* Performance improvement: Captive Core now reuses bucket files whenever it finds existing ones in the corresponding `--captive-core-storage-path` (introduced in [v2.1.0](#v2.1.0) rather than generating a one-time temporary sub-directory ([](https://github.com/stellar/go/pull/XXXX)). **This feature requires Stellar-Core version 17.1 or later.**
+
+
 ## v2.4.1
 
 **Upgrading to this version from <= v2.1.1 will trigger a state rebuild. During this process (which can take up to 20 minutes), Horizon will not ingest new ledgers.**
 
-### Code Changes
-
+### Bug Fixes
 * Fix bug in `horizon db reingest range` command, which would throw a duplicate entry conflict error from the DB. ([3661](https://github.com/stellar/go/pull/3661)).
 * Fix bug in DB metrics preventing Horizon from starting when read-only replica middleware is enabled. ([3668](https://github.com/stellar/go/pull/3668)).
 * Fix bug in the value of `route` in the logs for rate-limited requests (previously it was set to `undefined`). ([3658](https://github.com/stellar/go/pull/3658)).
 
+
 ## v2.4.0
 
 **Upgrading to this version from <= v2.1.1 will trigger a state rebuild. During this process (which can take up to 20 minutes), Horizon will not ingest new ledgers.**
@@ -82,7 +87,7 @@ file. This project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Breaking changes
 
-* Add a flag `--captive-core-storage-path/CAPTIVE_CORE_STORAGE_PATH` that allows users to control the storage location for Captive Core bucket data ([3479](https://github.com/stellar/go/pull/3479)).
+* Add a flag `--captive-core-storage-path`/`CAPTIVE_CORE_STORAGE_PATH` that allows users to control the storage location for Captive Core bucket data ([3479](https://github.com/stellar/go/pull/3479)).
   - Previously, Horizon created a directory in `/tmp` to store Captive Core bucket data. Now, if the captive core storage path flag is not set, Horizon will default to using the current working directory.
 * Add a flag `--captive-core-log-path`/`CAPTIVE_CORE_LOG_PATH` that allows users to control the location of the logs emitted by Captive Core ([3472](https://github.com/stellar/go/pull/3472)). If you have a `LOG_FILE_PATH` entry in your Captive Core toml file remove that entry and use the horizon flag instead.
 * `--stellar-core-db-url` / `STELLAR_CORE_DATABASE_URL` should only be configured if Horizon ingestion is enabled otherwise Horizon will not start ([3477](https://github.com/stellar/go/pull/3477)).

services/horizon/docker/docker-compose.integration-tests.yml

@@ -10,7 +10,7 @@ services:
       - "5641:5641"
     command: ["-p", "5641"]
   core:
-    image: ${CORE_IMAGE:-stellar/stellar-core:v17.0.0}
+    image: ${CORE_IMAGE:-stellar/stellar-core:17.1.0-591.151c5a6.focal}
     depends_on:
       - core-postgres
     restart: on-failure

services/horizon/internal/integration/db_test.go

@@ -83,10 +83,16 @@ func TestReingestDB(t *testing.T) {
 		toLedger = latestCheckpoint
 	}
 
+	// We just want to test reingestion, so there's no reason for a background
+	// Horizon to run. Keeping it running will actually cause the Captive Core
+	// subprocesses to conflict.
+	itest.StopHorizon()
+
 	horizonConfig.CaptiveCoreConfigPath = filepath.Join(
 		filepath.Dir(horizonConfig.CaptiveCoreConfigPath),
 		"captive-core-reingest-range-integration-tests.cfg",
 	)
+
 	horizoncmd.RootCmd.SetArgs([]string{
 		"--stellar-core-url",
 		horizonConfig.StellarCoreURL,

services/horizon/internal/test/integration/integration.go

@@ -135,11 +135,7 @@ func NewTest(t *testing.T, config Config) *Test {
 }
 
 func (i *Test) RestartHorizon() {
-	i.app.Close()
-
-	// wait for horizon to shut down completely
-	<-i.appStopped
-
+	i.StopHorizon()
 	i.startHorizon(
 		i.horizonConfig.CaptiveCoreBinaryPath,
 		i.horizonConfig.CaptiveCoreConfigPath,
@@ -332,6 +328,17 @@ func (i *Test) Horizon() *horizon.App {
 	return i.app
 }
 
+// StopHorizon shuts down the running Horizon process
+func (i *Test) StopHorizon() {
+	i.app.CloseDB()
+	i.app.Close()
+
+	// Wait for Horizon to shut down completely.
+	<-i.appStopped
+
+	i.app = nil
+}
+
 // AdminPort returns Horizon admin port.
 func (i *Test) AdminPort() int {
 	return adminPort