Potential performance issue with LoadTypes
- Dominant language
- Go
- Stars
- 14.3k
- Forks
- 1.1k
- Avg merge
- 6d 9h
- Merged PRs (30d)
- 11
Description
**Describe the bug**
Recently updated from `v5.5.5` to `v5.7.1` and used the `LoadTypes` func to load up custom types en masse instead of 1-by-1. We hit some hiccups in that our cluster of 5 servers would spawn up and then our postgres server would crash. Upon troubleshooting the logs showed a pretty massive recursive CTE which led us to the [buildLoadDerivedTypesSQL](https://github.com/jackc/pgx/blob/e3c81cc1535ea6a5c3298ea04666754709d5d71e/derived_types.go#L19) func. We rolled back our release (which had more than just pgx upgrade) and it became stable again. To be clear, I'm not saying this caused the crash - haven't replicated it on any other dev/test environment to figure out for sure. Just wanted to share these findings below.
We have 53 custom types that we load up in an after connect hook with the driver. I then cloned the pgx repo and made use of the test setup to benchmark both ways. I'm not expert at benchmarking, but if this is relatively correct there's a fairly major penalty for using `LoadTypes` as opposed to iterating the custom types and one-by-one using `LoadType`. My findings are roughly 20x slower to use the recursive CTE pathway.
**To Reproduce**
I added tests like this to the `derived_types_test.go` file against custom types I already have defined in my db.
```go
var myCTypes = []string{
// all custom type names here
}
func BenchmarkLoadMyCustomTypesBulk(b *testing.B) {
for i := 0; i < b.N; i++ {
testRunner := pgxtest.DefaultConnTestRunner()
testRunner.CreateConfig = func(ctx context.Context, t testing.TB) *pgx.ConnConfig {
config, err := pgx.ParseConfig("postgres://pguser:pgpass@localhost:5432/pgdb?connect_timeout=30&sslmode=disable")
require.NoError(t, err)
return config
}
testRunner.RunTest(context.Background(), b, func(ctx context.Context, t testing.TB, conn *pgx.Conn) {
_, err := conn.LoadTypes(ctx, myCTypes)
require.NoError(t, err)
})
}
}
func BenchmarkLoadMyCustomTypesSerial(b *testing.B) {
for i := 0; i < b.N; i++ {
testRunner := pgxtest.DefaultConnTestRunner()
testRunner.CreateConfig = func(ctx context.Context, t testing.TB) *pgx.ConnConfig {
config, err := pgx.ParseConfig("postgres://pguser:pgpass@localhost:5432/pgdb?connect_timeout=30&sslmode=disable")
require.NoError(t, err)
return config
}
testRunner.RunTest(context.Background(), b, func(ctx context.Context, t testing.TB, conn *pgx.Conn) {
for _, ct := range myCTypes {
cTyp, err := conn.LoadType(context.Background(), ct)
require.NoError(t, err)
conn.TypeMap().RegisterType(cTyp)
}
})
}
}
```
Then I execute the benchmark tests for 30 seconds and get the result:
```
go test github.com/jackc/pgx/v5 -run MyCustomTypes -bench=MyCustomTypes -benchtime 30s -benchmem -v
BenchmarkLoadPOSAPICustomTypesBulk
BenchmarkLoadPOSAPICustomTypesBulk-10 26 1330200502 ns/op 140919 B/op 964 allocs/op
BenchmarkLoadPOSAPICustomTypesSerial
BenchmarkLoadPOSAPICustomTypesSerial-10 553 63574098 ns/op 195709 B/op 1896 allocs/op
```
**Expected behavior**
Bulk loading, especially via such a massive recursive CTE, would ideally provide some benefit. Otherwise, the bulk operation could just iterate over the slice of strings and do `LoadType` for you and register it.
**Actual behavior**
Seems to be significantly less performant, and potentially risky combination of recursive CTE and dynamic sql 😅
**Version**
- Go: `go version go1.22.9 darwin/arm64`
- PostgreSQL: `PostgreSQL 15.5 (Debian 15.5-1.pgdg110+1) on x86_64-pc-linux-gnu, compiled by gcc (Debian 10.2.1-6) 10.2.1 20210110, 64-bit`
- pgx: `v5.7.1`
Contributor guide
Research direction
Start with buildLoadDerivedTypesSQL in derived_types.go and the benchmarks described in derived_types_test.go. Reproduce the bulk and serial LoadTypes/LoadType timings against PostgreSQL, then determine whether the bulk path can avoid its reported recursive CTE cost without changing loading behavior. Done means the performance concern is addressed and covered by a benchmark or test.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, postgresql
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100