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: Release DB connection in /paths when no longer needed #4228

Merged
merged 4 commits into from
Feb 18, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
7 changes: 7 additions & 0 deletions services/horizon/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ file. This project adheres to [Semantic Versioning](http://semver.org/).

## Unreleased

## v2.14.0

* Restart Stellar-Core when it's context is cancelled. ([4192](https://github.com/stellar/go/pull/4192))
* Resume ingestion immediately when catching up. ([4196](https://github.com/stellar/go/pull/4196))
* Check if there are newer ledger when requested ledger does not exist. ([4198](https://github.com/stellar/go/pull/4198))
* Properly check against the HA array being empty. ([4152](https://github.com/stellar/go/pull/4152))

## v2.13.0

### DB Schema Migration
Expand Down
24 changes: 24 additions & 0 deletions services/horizon/internal/actions/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,18 @@ func (handler FindPathsHandler) GetResource(w HeaderWriter, r *http.Request) (in
}
}

// Rollback REPEATABLE READ transaction so that a DB connection is released
Copy link
Contributor

Choose a reason for hiding this comment

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

@bartekn I think it would be cleaner to modify the StateMiddleware so that it can be configured without the repeatable read transactions.

Copy link
Contributor

Choose a reason for hiding this comment

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

actually, nevermind. I just realized that approach would not work because I forgot that we have to do a db query to fetch assets belonging to an account. you can ignore this comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this was exactly my first thought but then I moved it to action. I think this is actually clearer because there are no special cases in StateMiddleware.

// to be used by other http requests.
historyQ, err := horizonContext.HistoryQFromRequest(r)
if err != nil {
return nil, errors.Wrap(err, "could not obtain historyQ from request")
}

err = historyQ.Rollback()
if err != nil {
return nil, errors.Wrap(err, "error in rollback")
}

records := []paths.Path{}
if len(query.SourceAssets) > 0 {
var lastIngestedLedger uint32
Expand Down Expand Up @@ -301,6 +313,18 @@ func (handler FindFixedPathsHandler) GetResource(w HeaderWriter, r *http.Request
}
}

// Rollback REPEATABLE READ transaction so that a DB connection is released
// to be used by other http requests.
historyQ, err := horizonContext.HistoryQFromRequest(r)
if err != nil {
return nil, errors.Wrap(err, "could not obtain historyQ from request")
}

err = historyQ.Rollback()
if err != nil {
return nil, errors.Wrap(err, "error in rollback")
}

sourceAsset := qp.SourceAsset()
amountToSpend := qp.Amount()

Expand Down