Add SYNCV3_MAX_DB_CONN: use it in e2e tests

This is designed to catch a class of SQL transaction bugs where
we BEGIN a transaction and then forget to use that `txn` var, and
do other things on `sql.DB` which will use a different connection.

By testing with max conns = 1 this will deadlock. We also test with
max conns = 2 to try to catch more pernicious failure modes. Using
max conns = 1 effectively serialises access to the database, but
some bugs may only be apparent when there is some limited amount
of concurrency available e.g mid-processing this event, do X. With
max conns = 1 we cannot test this, which is why we also test with
max conns = 2.
This commit is contained in:
Kegan Dougal 2023-07-12 17:10:47 +01:00
parent 76e241c4f9
commit 8be09840d0
2 changed files with 15 additions and 2 deletions

View File

@ -88,6 +88,10 @@ jobs:
if-no-files-found: error
end_to_end:
runs-on: ubuntu-latest
strategy:
matrix:
# test with unlimited + 1 + 2 max db conns. If we end up double transacting in the tests anywhere, conn=1 tests will fail.
max_db_conns: [0,1,2]
services:
synapse:
# Custom image built from https://github.com/matrix-org/synapse/tree/v1.72.0/docker/complement with a dummy /complement/ca set
@ -142,6 +146,7 @@ jobs:
SYNCV3_DB: user=postgres dbname=syncv3 sslmode=disable password=postgres host=localhost
SYNCV3_SERVER: http://localhost:8008
SYNCV3_SECRET: itsasecret
SYNCV3_MAX_DB_CONN: ${{ matrix.max_db_conns }}
E2E_TEST_SERVER_STDOUT: test-e2e-server.log
- name: Upload test log

View File

@ -6,6 +6,7 @@ import (
_ "net/http/pprof"
"os"
"os/signal"
"strconv"
"strings"
"syscall"
"time"
@ -40,6 +41,7 @@ const (
EnvJaeger = "SYNCV3_JAEGER_URL"
EnvSentryDsn = "SYNCV3_SENTRY_DSN"
EnvLogLevel = "SYNCV3_LOG_LEVEL"
EnvMaxConns = "SYNCV3_MAX_DB_CONN"
)
var helpMsg = fmt.Sprintf(`
@ -55,7 +57,8 @@ Environment var
%s Default: unset. The Jaeger URL to send spans to e.g http://localhost:14268/api/traces - if unset does not send OTLP traces.
%s Default: unset. The Sentry DSN to report events to e.g https://sliding-sync@sentry.example.com/123 - if unset does not send sentry events.
%s Default: info. The level of verbosity for messages logged. Available values are trace, debug, info, warn, error and fatal
`, EnvServer, EnvDB, EnvSecret, EnvBindAddr, EnvTLSCert, EnvTLSKey, EnvPPROF, EnvPrometheus, EnvJaeger, EnvSentryDsn, EnvLogLevel)
%s Default: unset. Max database connections to use when communicating with postgres. Unset or 0 means no limit.
`, EnvServer, EnvDB, EnvSecret, EnvBindAddr, EnvTLSCert, EnvTLSKey, EnvPPROF, EnvPrometheus, EnvJaeger, EnvSentryDsn, EnvLogLevel, EnvMaxConns)
func defaulting(in, dft string) string {
if in == "" {
@ -81,6 +84,7 @@ func main() {
EnvJaeger: os.Getenv(EnvJaeger),
EnvSentryDsn: os.Getenv(EnvSentryDsn),
EnvLogLevel: os.Getenv(EnvLogLevel),
EnvMaxConns: defaulting(os.Getenv(EnvMaxConns), "0"),
}
requiredEnvVars := []string{EnvServer, EnvDB, EnvSecret, EnvBindAddr}
for _, requiredEnvVar := range requiredEnvVars {
@ -162,9 +166,13 @@ func main() {
panic(err)
}
maxConnsInt, err := strconv.Atoi(args[EnvMaxConns])
if err != nil {
panic("invalid value for " + EnvMaxConns + ": " + args[EnvMaxConns])
}
h2, h3 := syncv3.Setup(args[EnvServer], args[EnvDB], args[EnvSecret], syncv3.Opts{
AddPrometheusMetrics: args[EnvPrometheus] != "",
DBMaxConns: 100,
DBMaxConns: maxConnsInt,
DBConnMaxIdleTime: time.Hour,
})