Skip to content

Commit 6e53270

Browse files
craig[bot]sambhav-jain-16
andcommitted
Merge #143876
143876: roachtest/tpccbench: fix double openmetrics emission r=csgourav a=sambhav-jain-16 `tpcc.NewResultWithSnapshots(warehouses, 0, snapshots)` appends the `snapshots` map with a value of the merged histogram for each key. So when the test was exporting metrics into openmetrics, the result file would have double counts. This was resulting in wrong total values which calculating aggregated results. This commit fixes the issue by moving `tpcc.NewResultWithSnapshots` to after conversion Epic: none Release note: None Co-authored-by: Sambhav Jain <[email protected]>
2 parents 3ae1c3c + e6f031f commit 6e53270

File tree

1 file changed

+24
-14
lines changed

1 file changed

+24
-14
lines changed

pkg/cmd/roachtest/tests/tpcc.go

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,25 @@ func getMaxWarehousesAboveEfficiency(
177177
return aggregatedMetrics, nil
178178
}
179179

180+
func convertSnapshotsToOpenmetrics(
181+
ctx context.Context,
182+
t test.Test,
183+
c cluster.Cluster,
184+
nodes option.NodeListOption,
185+
warehouses int,
186+
snapshots map[string][]exporter.SnapshotTick,
187+
) error {
188+
statsFilePrefix := fmt.Sprintf("warehouses=%d/", warehouses)
189+
190+
// Create buffer for performance metrics
191+
perfBuf := bytes.NewBuffer([]byte{})
192+
exporter := roachtestutil.CreateWorkloadHistogramExporterWithLabels(t, c, map[string]string{warehouseLabel: fmt.Sprintf("%d", warehouses)})
193+
writer := io.Writer(perfBuf)
194+
exporter.Init(&writer)
195+
defer roachtestutil.CloseExporter(ctx, exporter, t, c, perfBuf, nodes, statsFilePrefix)
196+
return exportOpenMetrics(exporter, snapshots)
197+
}
198+
180199
type tpccOptions struct {
181200
DB string // database name
182201
Warehouses int
@@ -2051,26 +2070,17 @@ func runTPCCBench(ctx context.Context, t test.Test, c cluster.Cluster, b tpccBen
20512070
// overload but something that deserves failing the whole test.
20522071
t.Fatal(err)
20532072
}
2054-
result := tpcc.NewResultWithSnapshots(warehouses, 0, snapshots)
2055-
20562073
// This roachtest uses the stats.json emitted from hdr histogram to compute Tpmc and show it in the run log
20572074
// Since directly emitting openmetrics and computing Tpmc from it is not supported, it is better to convert the
20582075
// stats.json emitted to openmetrics in the test itself and upload it to the cluster
20592076
if t.ExportOpenmetrics() {
2060-
// Creating a prefix
2061-
statsFilePrefix := fmt.Sprintf("warehouses=%d/", warehouses)
2062-
2063-
// Create buffer for performance metrics
2064-
perfBuf := bytes.NewBuffer([]byte{})
2065-
exporter := roachtestutil.CreateWorkloadHistogramExporterWithLabels(t, c, map[string]string{warehouseLabel: fmt.Sprintf("%d", warehouses)})
2066-
writer := io.Writer(perfBuf)
2067-
exporter.Init(&writer)
2068-
defer roachtestutil.CloseExporter(ctx, exporter, t, c, perfBuf, group.LoadNodes, statsFilePrefix)
2069-
2070-
if err := exportOpenMetrics(exporter, snapshots); err != nil {
2071-
return errors.Wrapf(err, "error converting histogram to openmetrics")
2077+
if err = convertSnapshotsToOpenmetrics(ctx, t, c, group.LoadNodes, warehouses, snapshots); err != nil {
2078+
// If we are not able to convert, then there's an error
2079+
t.Fatal(err)
20722080
}
20732081
}
2082+
2083+
result := tpcc.NewResultWithSnapshots(warehouses, 0, snapshots)
20742084
resultChan <- result
20752085
return nil
20762086
})

0 commit comments

Comments
 (0)