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

Horizon reaper uses the same db connection pool which is used to serve requests #5338

Closed
tamirms opened this issue Jun 10, 2024 · 0 comments
Closed

Comments

@tamirms
Copy link
Contributor

tamirms commented Jun 10, 2024

When the horizon reaper is initialized, it uses the same db connection pool which is used to serve Horizon requests:

a.reaper = reap.New(
a.config.HistoryRetentionCount,
a.config.HistoryRetentionReapCount,
a.HorizonSession(),
a.ledgerState)

This is problematic for two reasons:

  1. SDF uses an architecture where ingesting horzon nodes are connected to a RW database and request serving horizon nodes are connected to a RO replica database. Reaping will fail in this scenario because delete statements will result in errors when executed on a read only database.
  2. The db connection pool used for serving horizon requests is configured with a statement timeout which defaults to 10 seconds. The reaping DB statements are not guaranteed to execute within this time limit and in the scenario where there is a lengthy reap DB query it will block the progress of reaping.

To fix this issue we should only reap in Horizon when ingestion is enabled because that guarantees that we have a RW db connection. Also, we should create a new DB connection pool solely for the purpose of reaping. That connection pool should be configured to not have any statement timeouts. Or, if we decide to have statement timeouts for reaping, that configuration should be separate from the configuration for the statement timeout on the db queries connected to horizon HTTP requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

3 participants
@tamirms @mollykarcher and others