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

services/horizon: Add cache toggle and use libary for on-disk caching #5197

Merged
merged 17 commits into from
Feb 8, 2024

Conversation

Shaptic
Copy link
Contributor

@Shaptic Shaptic commented Feb 5, 2024

What

  • Introduce a library to do on-disk caching (fscache) to avoid the issue we were seeing with the custom implementation.
  • Changes the cache to be size-based rather than count-based (we ensure it will always be <= 10GiB this way).
  • A new --history-archive-caching=(true|false) flag (default: true) controls whether or not there will be an on-disk cache used for history archive fetches. Most people will not need to set this to false unless they're encountering issues. This is to prevent folks from being blocked if issues are encountered with this feature in the future.
  • Add a metric for showing how much bandwidth is saved by caching.

Why

Battle-tested reliability.

Closes stellar/quickstart#557.
Closes #5186

@Shaptic Shaptic changed the title [Draft] services/horizon: Add cache toggle and fix caching for large files services/horizon: Add cache toggle and use libary for on-disk caching Feb 6, 2024
@Shaptic Shaptic requested review from tamirms, 2opremio, sreuland and a team February 6, 2024 20:12
historyarchive/archive.go Outdated Show resolved Hide resolved
historyarchive/archive.go Outdated Show resolved Hide resolved
@@ -107,7 +107,7 @@ func (x *XdrStream) ExpectedHash() ([sha256.Size]byte, bool) {
func (x *XdrStream) Close() error {
if x.validateHash {
// Read all remaining data from rdr
_, err := io.Copy(ioutil.Discard, x.rdr)
_, err := io.Copy(io.Discard, x.rdr)
Copy link
Contributor

Choose a reason for hiding this comment

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

oh, interesting, was this and ln 144 related to root cause of corruption seen in quickstart/557?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nah this was just a drive-by fix of deprecated usage - they're functionally equivalent

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, how do we know this fixed stellar/quickstart#557, build quickstart locally with this branch for horizon and run with --pubnet ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, that's how I've been doing it this whole time locally and I can confirm that it no longer encounters the read error when processing ledgers

historyarchive/archive.go Outdated Show resolved Hide resolved
historyarchive/archive.go Outdated Show resolved Hide resolved
historyarchive/archive.go Outdated Show resolved Hide resolved
historyarchive/archive.go Outdated Show resolved Hide resolved
historyarchive/archive.go Outdated Show resolved Hide resolved
historyarchive/archive.go Outdated Show resolved Hide resolved
Copy link
Contributor

@sreuland sreuland left a comment

Choose a reason for hiding this comment

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

nice work!

historyarchive/archive.go Outdated Show resolved Hide resolved
@Shaptic Shaptic merged commit 02cd784 into stellar:release-horizon-v2.28.2 Feb 8, 2024
28 checks passed
@Shaptic Shaptic deleted the add-caching-flag branch February 8, 2024 22:02
urvisavla pushed a commit to urvisavla/go that referenced this pull request Feb 13, 2024
…stellar#5197)

* Add a `--history-archive-caching` flag (default=true) to toggle behavior
* Refactor to use a library: fscache
* Hook new metric into prometheus
* Add parallel read test to stress cache
* Add tests for deadlocking and other misc. scenarios
urvisavla pushed a commit to urvisavla/go that referenced this pull request Feb 13, 2024
…stellar#5197)

* Add a `--history-archive-caching` flag (default=true) to toggle behavior
* Refactor to use a library: fscache
* Hook new metric into prometheus
* Add parallel read test to stress cache
* Add tests for deadlocking and other misc. scenarios
Shaptic added a commit to Shaptic/go that referenced this pull request Feb 14, 2024
…stellar#5197)

* Add a `--history-archive-caching` flag (default=true) to toggle behavior
* Refactor to use a library: fscache
* Hook new metric into prometheus
* Add parallel read test to stress cache
* Add tests for deadlocking and other misc. scenarios
Shaptic added a commit to Shaptic/go that referenced this pull request Feb 14, 2024
…stellar#5197)

* Add a `--history-archive-caching` flag (default=true) to toggle behavior
* Refactor to use a library: fscache
* Hook new metric into prometheus
* Add parallel read test to stress cache
* Add tests for deadlocking and other misc. scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants