Postgres: avoid tiny writes to improve performance in read-heavy scenarios

[Tener]

Currently, for Postgres, we read individual server messages one by one before forwarding them to the client, also one by one. This limits the performance if the messages are small, as sending them in this manner is costly: each takes at least one syscall to perform. In particular, this can be observed in read-heavy scenarios, such as pg_dump or SELECT * FROM large_table.

This PR restructures the handling of the server byte stream. The main driver is io.Copy call, which ensures performance due to the large buffers involved. For analysis purposes, we copy the byte stream for consumption in a separate goroutine, which counts the parsed messages. The byte stream is copied synchronously courtesy of io.Pipe, which ensures the stream does not consume significant memory resources.

I have tested the performance of this part of the code using home-grown scripts available here.

I ran the tests on my local MacBook, with native Postgres (postgres (PostgreSQL) 15.3 (Homebrew)).

version	command	runs	average time	slowdown
N/A (without Teleport)	make read-benchmark	5	0.612s	x1.00
master @ 9960697d35	make read-benchmark	5	8.390s	x13.71
tener/postgres-io-copy @ bf8ca89573	make read-benchmark	5	0.682s	x1.15

master is CPU-bound and limited by single-core performance on the Teleport side and is ~13.7 times slower compared to native performance (without Teleport). This PR closes the gap by a large margin, leaving only 15% overhead.

Results from the synthetic benchmark also added in PR:

$ go test ./lib/srv/db -bench=. -run=^$ -benchtime=3x
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/srv/db
BenchmarkPostgresReadLargeTable/size=11-10         	       3	 286618514 ns/op
BenchmarkPostgresReadLargeTable/size=20-10         	       3	 253457917 ns/op
BenchmarkPostgresReadLargeTable/size=100-10        	       3	 222804292 ns/op
BenchmarkPostgresReadLargeTable/size=1000-10       	       3	 216612764 ns/op
BenchmarkPostgresReadLargeTable/size=2000-10       	       3	 214121861 ns/op
BenchmarkPostgresReadLargeTable/size=8000-10       	       3	 215046472 ns/op

Contrast with master:

$ git checkout master -- lib/srv/db/postgres/engine.go
$ go test ./lib/srv/db -bench=. -run=^$ -benchtime=3x
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/srv/db
BenchmarkPostgresReadLargeTable/size=11-10         	       3	55277246278 ns/op
BenchmarkPostgresReadLargeTable/size=20-10         	       3	31743084695 ns/op
BenchmarkPostgresReadLargeTable/size=100-10        	       3	6678759083 ns/op
BenchmarkPostgresReadLargeTable/size=1000-10       	       3	 920221875 ns/op
BenchmarkPostgresReadLargeTable/size=2000-10       	       3	 567684528 ns/op
BenchmarkPostgresReadLargeTable/size=8000-10       	       3	 297351194 ns/op

For this benchmark, the new code achieves a speedup of at least 1.38x, or as much as 192x - depending on the message size. Running the benchmark with -benchmem (results not shown for brevity) indicates the updated code allocates less memory in fewer individual allocations, ultimately leading to decreased GC pressure.

Changelog: Postgres: improve performance in read-heavy scenarios.

Fixes #26868.

[Tener]

@smallinsky FYI.

[Tener]

@smallinsky e352893 implements message parsing loop running in a separate goroutine, enabling analysis. No performance impact observed.

[smallinsky]

@Tener
Could update a PR comment and provided performance insights and provide some data ?

[Tener]

@Tener Could update a PR comment and provided performance insights and provide some data ?

Yep, just working on it right now. Just made a repo with my scripts: https://github.com/Tener/teleport-bench-dbs, I'll reference it in my findings.

[rosstimothy]

Can we write a benchmark test that exercises this before and after the change so we have some tangible numbers on just how much better this solution is?

[Tener]

Can we write a benchmark test that exercises this before and after the change so we have some tangible numbers on just how much better this solution is?

@rosstimothy Sure, I just added the results from the benchmark I've been using throughout this work.

[rosstimothy]

Can we write a benchmark test that exercises this before and after the change so we have some tangible numbers on just how much better this solution is?

@rosstimothy Sure, I just added the results from the benchmark I've been using throughout this work.

That's not quite what I was talking about. The numbers look good but it'd be nice if we had a BenchmarkPostgres(b *testing.b) that could be exercised regularly.

[Tener]

Can we write a benchmark test that exercises this before and after the change so we have some tangible numbers on just how much better this solution is?

@rosstimothy Sure, I just added the results from the benchmark I've been using throughout this work.

That's not quite what I was talking about. The numbers look good but it'd be nice if we had a BenchmarkPostgres(b *testing.b) that could be exercised regularly.

That would be nice indeed, and we do have relevant RFD open:

• RFD 141: tsh bench db #28727.

It isn't implemented, however, which is why I went with custom scripts.

[rosstimothy]

Can we write a benchmark test that exercises this before and after the change so we have some tangible numbers on just how much better this solution is?

@rosstimothy Sure, I just added the results from the benchmark I've been using throughout this work.

That's not quite what I was talking about. The numbers look good but it'd be nice if we had a BenchmarkPostgres(b *testing.b) that could be exercised regularly.

That would be nice indeed, and we do have relevant RFD open:

• RFD 141: tsh bench db #28727.

It isn't implemented, however, which is why I went with custom scripts.

I don't think that RFD applies. That is implementing tests to exercise db access via tsh bench postgres. I'm asking if we can implement a go benchmark test that covers the changes in this specific PR.

[Tener]

I can write a limited benchmark, for example exercising only receiveFromServer, if this is what you mean here. Shouldn't take too much work to do so.

I believe that the tests proposed in RFD#141 will ultimately be much more useful, being end-to-end and representing actual real-world workloads.

[Tener]

@rosstimothy I've added the benchmark to the PR. The results are in the PR description.

[rosstimothy]

Thanks for the benchmark test! LGTM other than a few nits.

[public-teleport-github-review-bot]

@Tener See the table below for backport results.

Branch	Result
branch/v13	Failed