roachtest: actually fail TPCC bench if reached max warehouses

In 1bfe55b, we attempted to fail the TPCC bench test run if the
configured maximum warehouses were reached and the success criteria
were met. The idea was that this would prompt us to increase the max
warehouses. However, that commit failed only the specific line search
run, not the full test run.

This commit moves the max warehouses check out of the result handling
and actually fatals the test.

Part of: #148235

Release note: None

Author: miraradeva

pkg/cmd/roachtest/tests/tpcc.go

@@ -2250,7 +2250,22 @@ func runTPCCBench(ctx context.Context, t test.Test, c cluster.Cluster, b tpccBen
 				results = append(results, partial)
 			}
 			res = tpcc.MergeResults(results...)
-			failErr = res.FailureError(b.LoadWarehouses(c.Cloud()))
+			failErr = res.FailureError()
+			// If the active warehouses have reached the load warehouses, fail the test;
+			// it needs to be updated to allow for more warehouses. Note that the line
+			// search assumes that the test fails at the number of load warehouses, so it
+			// never attempts to reach it exactly. Therefore, active warehouses can be at
+			// most LoadWarehouses-1.
+			if res.ActiveWarehouses >= b.LoadWarehouses(c.Cloud())-1 {
+				err = errors.CombineErrors(
+					failErr,
+					errors.Errorf(
+						"the number of active warehouses (%d) reached the maximum number of "+
+							"warehouses; consider updating LoadWarehouses and EstimatedMax", res.ActiveWarehouses,
+					),
+				)
+				t.Fatal(err)
+			}
 		}
 
 		// Print the result.
@@ -2681,7 +2696,7 @@ func runTPCCPublished(
 				results = append(results, partial)
 			}
 			res = tpcc.MergeResults(results...)
-			failErr = res.FailureError(opts.LoadWarehousesGCE)
+			failErr = res.FailureError()
 		}
 
 		// Print result for current iteration

pkg/workload/tpcc/result.go

@@ -169,7 +169,7 @@ func (r *Result) Efficiency() float64 {
 
 // FailureError returns nil if the Result is passing or an error describing
 // the failure if the result is failing.
-func (r *Result) FailureError(loadWarehouses int) error {
+func (r *Result) FailureError() error {
 	if _, newOrderExists := r.Cumulative["newOrder"]; !newOrderExists {
 		return errors.Errorf("no newOrder data exists")
 	}
@@ -193,19 +193,5 @@ func (r *Result) FailureError(loadWarehouses int) error {
 					query, v, max90th))
 		}
 	}
-	// If the active warehouses have reached the load warehouses, fail the test;
-	// it needs to be updated to allow for more warehouses. Note that the line
-	// search assumes that the test fails at the number of load warehouses, so it
-	// never attempts to reach it exactly. Therefore, active warehouses can be at
-	// most loadWarehouses - 1.
-	if r.ActiveWarehouses >= loadWarehouses-1 {
-		err = errors.CombineErrors(
-			err,
-			errors.Errorf(
-				"the number of active warehouses (%d) reached the maximum number of "+
-					"warehouses; consider updating LoadWarehouses and EstimatedMax", r.ActiveWarehouses,
-			),
-		)
-	}
 	return err
 }

pkg/workload/tpcc/result_test.go

@@ -15,7 +15,7 @@ func TestNewResult(t *testing.T) {
 	// Ensure you don't get panics when calling common methods
 	// on a trivial Result that doesn't have any data attached.
 	res := NewResult(1000, 0, 0, nil)
-	require.Error(t, res.FailureError(0))
+	require.Error(t, res.FailureError())
 	require.Zero(t, res.Efficiency())
 	require.Zero(t, res.TpmC())
 }