roachtest: use the rate from the log files

[andrewbaptist]

Previously we would track the throughput and latency from the listener
on the nodes directly. This had problems in certain cases and added a
new failure mode. This change switches to pulling the rate from the log
files after the test completes.

Fixes: #129618
Fixes: #129113

Epic: none

Release note: None

[cockroach-teamcity]

This change is 

[kvoli]

Left some comments. It would help understand why we're making these changes if there were an example you could share on the PR, which motivates switching from the earlier workload recording to this.

Reviewed 2 of 2 files at r1, 1 of 1 files at r2, 2 of 2 files at r3, 3 of 3 files at r4, 1 of 1 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andrewbaptist, @DarrylWong, and @nameisbhaskar)

---

-- commits line 12 at r2:
For what test? This could apply to many of the hundreds of roachtests under roachtest

---

-- commits line 24 at r3:
Same as above, although the change is in a util file, the only user is the admission_control_latency test.

---

-- commits line 35 at r4:
Same as above.

---

-- commits line 37 at r4:
nit: Previously

---

-- commits line 49 at r5:
Which test?

---

-- commits line 58 at r6:
For which test? This isn't done generally for all roachtests.

---

pkg/cmd/roachtest/tests/admission_control_latency.go line 578 at r2 (raw file):

	clusterMaxRate := make(chan int)
	m.Go(func(ctx context.Context) error {
		waitDuration(ctx, 3*v.fillDuration/4)

Could you add a comment explaining the variables? I understand the /4 but not the 3 *.

---

pkg/cmd/roachtest/tests/admission_control_latency.go line 685 at r4 (raw file):

	t.Status("T5: validating results")

	// collect files

nit: for this and the below comment, use proper sentences.

---

pkg/cmd/roachtest/tests/admission_control_latency.go line 73 at r6 (raw file):

// TODO(baptist): Add a timeline describing the test in more detail.
type variations struct {
	// cluster is set up at the start of the test run

nit: period.

---

pkg/cmd/roachtest/tests/admission_control_latency.go line 74 at r6 (raw file):

type variations struct {
	// cluster is set up at the start of the test run
	cluster.Cluster

nit: I'm not sure how much value this gets us, none of the call lines are that long or tedious that it makes sense. I'm fine merging this though.

[andrewbaptist]

TFTR

I cleaned up the commit messages and some of the logs. I also added the two tests that this fixes.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DarrylWong, @kvoli, and @nameisbhaskar)

---

-- commits line 12 at r2:

Previously, kvoli (Austen) wrote…

For what test? This could apply to many of the hundreds of roachtests under roachtest

I added perturbation/* to the commit message

---

-- commits line 24 at r3:

Previously, kvoli (Austen) wrote…

Same as above, although the change is in a util file, the only user is the admission_control_latency test.

I clarified that this is only used in perturbation tests today, but will likely be used in other tests in the future.

---

-- commits line 35 at r4:

Previously, kvoli (Austen) wrote…

Same as above.

Fixed

---

-- commits line 37 at r4:

Previously, kvoli (Austen) wrote…

nit: Previously

Fixed

---

-- commits line 49 at r5:

Previously, kvoli (Austen) wrote…

Which test?

Added

---

-- commits line 58 at r6:

Previously, kvoli (Austen) wrote…

For which test? This isn't done generally for all roachtests.

Added

---

pkg/cmd/roachtest/tests/admission_control_latency.go line 578 at r2 (raw file):

Previously, kvoli (Austen) wrote…

Could you add a comment explaining the variables? I understand the /4 but not the 3 *.

I added a comment and made it more clear that this is 3/4 of the time waiting and 1/4 collecting stats.

---

pkg/cmd/roachtest/tests/admission_control_latency.go line 685 at r4 (raw file):

Previously, kvoli (Austen) wrote…

nit: for this and the below comment, use proper sentences.

I removed these comments, these were placeholders for me initally that I left in. They weren't necessary based on the context.

---

pkg/cmd/roachtest/tests/admission_control_latency.go line 73 at r6 (raw file):

Previously, kvoli (Austen) wrote…

nit: period.

Done

---

pkg/cmd/roachtest/tests/admission_control_latency.go line 74 at r6 (raw file):

Previously, kvoli (Austen) wrote…

nit: I'm not sure how much value this gets us, none of the call lines are that long or tedious that it makes sense. I'm fine merging this though.

It was slightly annoying to have to keep referring to the cluster. It was a fairly minor change though, so I'll leave it.

[kvoli]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @andrewbaptist, @DarrylWong, and @nameisbhaskar)

---

-- commits line 12 at r2:

Previously, andrewbaptist (Andrew Baptist) wrote…

I added perturbation/* to the commit message

Could you also add something to the commit header, like:

roachtest: query throughput using SQL for pertubation tests

[andrewbaptist]

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @DarrylWong, @kvoli, and @nameisbhaskar)

---

-- commits line 12 at r2:

Previously, kvoli (Austen) wrote…

Could you also add something to the commit header, like:

roachtest: query throughput using SQL for pertubation tests

Done

[andrewbaptist]

TFTR!

bors r=kvoli

[craig]

Build succeeded:

• docker_image_amd64
• unit_tests
• local_roachtest_fips
• linux_arm64_build
• acceptance
• macos_amd64_build
• check_generated_code
• macos_arm64_build
• linux_amd64_fips_build
• lint
• linux_amd64_build
• examples_orms
• windows_build
• local_roachtest