## What changed?
SQL config now supports passing a connect function.
## Why?
It's currently not possible to embed a Temporal server to a host
application where the database connection is managed by the host
application and is provided as a database connector. This allows the
host application to pass a db connector rather than the the individual
sql config strings.
## How did you test it?
I have embedded this code into a custom application and verified that
the connector is working properly.
## Potential risks
This is a non-breaking change with very minimal risks.
## Is hotfix candidate?
No.
## What changed?
Both our PostgreSQL and MySQL database backends will now automatically
reconnect to the database when certain errors occur: all errors chosen
have been experienced when testing this behavior through an AWS Aurora
RDS failover of either MySQL or PostgreSQL.
For both backends we will reconnect when we see:
- `ECONNRESET`
- `ECONNABORTED`
- `ECONNREFUSED`
- `io.EOF`
- `io.ErrUnexpectedEOF`
- `database/sql/driver.ErrBadConn`
for postgres we will also reconnect on the following SQLStates:
- `25006` read-only transaction
- `57P03` cannot connect now
- `0A000` feature not supported, but ONLY when the message is `cannot
set transaction read-write mode during recovery`
for mysql we will also reconnect when we see the following error codes:
- `1040` too many connections
- `1792` read-only transaction (SQLstate `25006`)
- `1836` running in read-only mode
This logic is easily extensible should we discover more failure modes
over time
## Why?
We've had multiple community reports of Temporal problems during RDS
failover. One part of this is the fact that we wouldn't necessarily
reconnect; we were at the whims of our chosen SQL abstraction's
connection pooling logic.
## How did you test it?
I manually tested this functionality in the presence of repeated RDS
failovers:
- [x] postgres12 plugin with pq driver
- [x] postgres12 plugin with pgx driver
- [x] mysql plugin
Automated testing will be added to our regular testing pipelines once
our infrastructure friends have added the support I need (it's in
progress)
## Potential risks
We're concerned there's a correctness issue in our PostgreSQL backend
that's related to our behavior during an RDS failover. If we merge this
before I figure out what's going on, we could hide the issue and make it
harder to reproduce.
## Documentation
N/A
## Is hotfix candidate?
Yes?
## What changed?
<!-- Describe what has changed in this PR -->
Rename vars related to visibility.
## Why?
<!-- Tell your future self why have you made these changes -->
Ease to read code.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Existing unit tests.
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
No.
## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
No
## What changed?
<!-- Describe what has changed in this PR -->
Remove support for standard visibility
## Why?
<!-- Tell your future self why have you made these changes -->
Standard visibility is deprecated.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
<!-- Describe what has changed in this PR -->
**What changed?**
`Driver` interface (with two implementations) was added to the
postgresql persistence package in order to support other drivers than
`lib/pq`. Fixes https://github.com/temporalio/temporal/issues/1775
Essentially, this PR does
https://github.com/temporalio/temporal/pull/4914 in a
backward-compatible and extendable way
<!-- Tell your future self why have you made these changes -->
**Why?**
Since `lib/pq` does not support multi-node clusters
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Existing tests
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
No, since you should explicitly change the driver in your config file.
However, `connectAttributes` depend on the driver (e.g.,
`binary_parameters` is supported only by `lib/pq` and is not a part of
postresql standard), so one should be careful changing drivers in
production.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No
* Use serviceerror.Unavailable error type instead of serviceerror.Internal
* serviceerror.Unavailable indicates errors like service's dependency is down, retryable
* serviceerror.Internal indicates errors like NPE in service logic, non-retryable
* Try using a set of default database names when initializing admin connection
Aiven DB is a PostgreSQL compatible DB, but comes with a different default DB name
* Add version check method to SQL interface
* Remove tools version check method in favor of functions defined in persistence package
* Refactor tests to not depend on tools package
