all: Refactor db.Session to take ctx into each call, instead of storing it on the session level

[paulbellamy]

PR Checklist

PR Structure

• This PR has reasonably narrow scope (if not, break it down into smaller PRs).
• This PR avoids mixing refactoring changes with feature changes (split into two PRs
  otherwise).
• This PR's title starts with name of package that is most changed in the PR, ex.
  services/friendbot, or all or doc if the changes are broad or impact many
  packages.

Thoroughness

• This PR adds tests for the most critical parts of the new functionality or fixes.
• I've updated any docs (developer docs, .md
  files, etc... affected by this change). Take a look in the docs folder for a given service,
  like this one.

Release planning

• I've updated the relevant CHANGELOG (here for Horizon) if
  needed with deprecations, added features, breaking changes, and DB schema changes.
• I've decided if this PR requires a new major/minor version according to
  semver, or if it's mainly a patch change. The PR is targeted at the next
  release branch if it's not a patch change.

What

The root change is:

 package db

 type Session struct {
        // DB is the database connection that queries should be executed against.
        DB *sqlx.DB

-       // Ctx is the context in which the repo is operating under.
-       Ctx context.Context
-
        tx        *sqlx.Tx
        txOptions *sql.TxOptions
 }

And changing all the db.Session methods (where needed) to take a ctx context.Context params. Then passing that through all the places to get it there.

Why

For #3344, we want to pass different timeouts in different contexts. This change makes that easier, and generally follows the go norms of passing the context late into the method.

Known limitations

It's big. Will be difficult to review and merge. Most of the param passing is pretty mechanical. The interesting parts are around where we create the contexts initially. But, most of that is in tests.

Questions

• RemoteCaptiveStellarCore.createContext and cancellation semantics. Should we do it everywhere? Only for PrepareRange?
  • Ignored this, keeping the existing semantics, for now. But seems like we should cancel all outstanding requests, not just PrepareRange.
• OrderBookStream.Run, should we make a new timeout ctx each tick?

Bonus

I also fixed TestOpen_openAndPingFails, because it was intermittently failing and being annoying.

[tamirms]

this will change the behavior because now we cannot cancel a query via context termination. maybe it's best to move changes to this file to the ledgerbackend PR as well

[paulbellamy]

I think this is fixed, with the new changes, but double-check me on that, please.

[paulbellamy]

Stuff here is difficult without including the ledgerbackend changes. Going to try doing those first, and see if that makes this change simpler.

Edit: It did not. Updating the LedgerBackend interface changes in DatabaseBackend, depends on these changes. Instead, added some temporary code to simplify this and pull the LedgerBackend interface changes out of this PR.

[paulbellamy]

One catch with this, is that we now need to call an explicit .Close() method on places where we relied on the context cancellation to implicitly shutdown everything.

Places where I've found that so far:

• LedgerBackend shutdown (we were already doing that)
• LedgerHashStore shotdown (also already doing that)

[2opremio]

Why is it stored instead of using it in method calls? (it would only be OK if a database backend is ephemeral)

[paulbellamy]

Belt-and-braces (for now) to ensure shutdown. Normally the parent should call Close(), but apparently there are some intricacies around shutdown ordering, so I wanted to keep the existing behaviour (when the parent ctx is cancelled, this ends as well).

[paulbellamy]

To be clear, it's two separate contexts. This one, to shut down the whole backend; and the one passed in method calls which is per-call.

[2opremio]

Same here