workload: add idle-conns flag for adding idle connections to tpcc

[RichardJCai]

workload: add idle-conns flag for adding idle connections to tpcc

Release note: None

#62526

[cockroach-teamcity]

This change is 

[RichardJCai]

@rafiss

Did some manual testing with the flag on a local setup. Do we actually want to add some configurations to roachtest?

Also there is a flag for configuring number of connections conns which is active connections so I didn't change that. We can add a flag for number of conns per pool but not sure if that's valuable right now.

These examples only show a loose correlation between latencies increasing and idle conns. I want to do more testing with diff configurations.

idle_0.txt
idle_10.txt
idle_100.txt
idle_1000.txt
idle_10000.txt

[RichardJCai]

Ran more tests with idle conns, tpcc for 300s on 3 node roachprod cluster, don't see a trend for latencies and idle conns.

idle_0.txt
idle_10.txt
idle_100.txt
idle_1000.txt
idle_5000.txt

[rafiss]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @RichardJCai)

---

pkg/workload/tpcc/tpcc.go, line 681 at r1 (raw file):

	fmt.Printf("Initializing %d idle connections...\n", w.idleConns)
	var conns []*pgxv4.Conn

any reason to use pgx/v4 just for this part? i think it'd be better to use the same version everywhere in the same package

---

pkg/workload/tpcc/tpcc.go, line 682 at r1 (raw file):

	fmt.Printf("Initializing %d idle connections...\n", w.idleConns)
	var conns []*pgxv4.Conn
	for i := 0; i < w.idleConns; i++ {

ah interesting. this seems like a good knob to have!

i didn't quite have this mind -- i was imagining that we would want more/better configuration for the MultiConnPool that is set up above (lines 620-630).

i thought the place to start would be to make MaxConnsPerPool not be hardcoded to 50. hmm but reading how pg_helpers.NewMultiConnPool works, maybe that setting isn't so relevant. we already have configuration for MaxTotalConnections which is more relevant for determining pool size (and influence the number of idle connections)

i think the issue here is that the pgxpool isn't that configurable. turns out all the interesting options (like MaxConnLifetime and MaxConnIdleTime) are only in v4... https://pkg.go.dev/github.com/jackc/pgx/v4/pgxpool

do you have an idea of how complicated it would be to upgrade this code to use pgx/v4 everywhere?

[RichardJCai]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)

---

pkg/workload/tpcc/tpcc.go, line 681 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

any reason to use pgx/v4 just for this part? i think it'd be better to use the same version everywhere in the same package

Yeah going to use the same version.

---

pkg/workload/tpcc/tpcc.go, line 682 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

ah interesting. this seems like a good knob to have!

i didn't quite have this mind -- i was imagining that we would want more/better configuration for the MultiConnPool that is set up above (lines 620-630).

i thought the place to start would be to make MaxConnsPerPool not be hardcoded to 50. hmm but reading how pg_helpers.NewMultiConnPool works, maybe that setting isn't so relevant. we already have configuration for MaxTotalConnections which is more relevant for determining pool size (and influence the number of idle connections)

i think the issue here is that the pgxpool isn't that configurable. turns out all the interesting options (like MaxConnLifetime and MaxConnIdleTime) are only in v4... https://pkg.go.dev/github.com/jackc/pgx/v4/pgxpool

do you have an idea of how complicated it would be to upgrade this code to use pgx/v4 everywhere?

It would be a fair bit of manual work to upgrade it to pgx/v4, we have to update MultiConnPool in pgx_helpers.go to pgx/v4 as well which has a lot of uses. We would have to completely rework MultiConnPool since it has a pgx.ConnPool which from what I've seen, pgx/v4 does not actually have, decent amount of plumbing to replace it but I'll make an issue for it.

[rafiss]

lgtm! hopefully either you or i can get to that followup issue later after the first batch of work is done

Reviewed 2 of 2 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)

[RichardJCai]

lgtm! hopefully either you or i can get to that followup issue later after the first batch of work is done

Reviewed 2 of 2 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)

Yep my plan is to tackle in during flex week.

[RichardJCai]

TFTR bors r=rafiss

[RichardJCai]

bors r=rafiss

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build failed:

• GitHub CI (Cockroach)

[RichardJCai]

I think it flaked, will try again
bors r=rafiss

[craig]

Build succeeded:

• GitHub CI (Cockroach)