Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 14 commits into from
Jun 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ingest/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we use value from Config instead of --captive-core-storage-path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ironically, --captive-core-storage-path means something more to Horizon than Captive Core. The corresponding BUCKET_DIR_PATH within Stellar-Core's config is a relative path to the directory from which the stellar-core command is run, so it's essentially path.Join(config.CaptiveCoreStoragePath, config.Toml.BucketDirPath) (psuedocode). Horizon uses this arg to create the directory, and that means the TOML file doesn't really apply.

We could merge these two options, but that's out of scope and requires more discussion since we'd finally be diverging from a Core-compatible TOML file.



## v2.0.0

Expand Down
25 changes: 6 additions & 19 deletions ingest/ledgerbackend/stellar_core_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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))
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -397,5 +384,5 @@ func (r *stellarCoreRunner) close() error {
r.pipe.Reader.Close()
}

return os.RemoveAll(storagePath)
return nil
}
4 changes: 2 additions & 2 deletions ingest/ledgerbackend/stellar_core_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
11 changes: 8 additions & 3 deletions services/horizon/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.**
Expand Down Expand Up @@ -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)).
Expand Down
6 changes: 6 additions & 0 deletions services/horizon/internal/integration/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
17 changes: 12 additions & 5 deletions services/horizon/internal/test/integration/integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down