Skip to content

Commit

Permalink
roachtest: collect qps metrics over longer window in gracefuldrain test
Browse files Browse the repository at this point in the history
The gracefuldrain test was modernized in cf30717.
Prior to that commit, QPS metrics were collected over a 10s interval,
whereas the modernization refactor changed this to 1 second intervals.
Looking at a few recent test failures, I see QPS metrics above the failure
threshold, which makes me think suspect that this 1s interval is causing
the sorts of inaccuracies MeasureQPS warns against.
Also See cockroachdb#133020 (comment).

One thing that doesn't line up is the timeline of this tests failure and
cf30717. Still, this patch changes the
metric's interval back to 10s.

References cockroachdb#133020

Release note: None
  • Loading branch information
arulajmani committed Jan 20, 2025
1 parent 1cce4f2 commit c5ff483
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion pkg/cmd/roachtest/tests/kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"sync/atomic"
"time"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster"
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
Expand Down Expand Up @@ -602,7 +603,7 @@ func registerKVGracefulDraining(r registry.Registry) {
}()

verifyQPS := func(ctx context.Context) error {
if qps := measureQPS(ctx, t, c, time.Second, c.Range(1, nodes-1)); qps < expectedQPS {
if qps := measureQPS(ctx, t, c, base.DefaultMetricsSampleInterval, c.Range(1, nodes-1)); qps < expectedQPS {
return errors.Newf(
"QPS of %.2f at time %v is below minimum allowable QPS of %.2f",
qps, timeutil.Now(), expectedQPS)
Expand Down

0 comments on commit c5ff483

Please sign in to comment.