Skip to content

Commit

Permalink
kvstreamer: increase default avg response multiple
Browse files Browse the repository at this point in the history
This commit increases the default value for
`sql.distsql.streamer.avg_response_size_multiple` cluster setting from
1.5 to 3.0. This setting controls the factor by which the current "avg
response size" estimate is multiplied and allows for TargetBytes
parameter to grow over time. In the reproduction query from the
previous commit it was determined that the growth might not be as quick
as desirable.

The effect of this change is as follows:
- if we have responses of varying sizes, then we're now likely to be more
effective since we'll end up issuing less BatchRequests
- if we have responses of similar sizes, then we might pre-reserve too
much budget upfront, so we'll end up with lower concurrency across
ranges.

Thus, we don't want to increase the multiple by too much; however,
keeping it at 1.5 can be quite suboptimal in some cases - 3.0 seems like
a decent middle ground. This number was chosen based on running TPCH
queries (both via InOrder and OutOfOrder modes of the streamer) and the
reproduction query. (For the latter this change reduces the number of
gRPC calls by a factor of 3 or so.)

Release note: None
  • Loading branch information
yuzefovich committed Nov 8, 2023
1 parent b71afcd commit 116bf04
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 8 deletions.
5 changes: 3 additions & 2 deletions pkg/kv/kvclient/kvstreamer/avg_response_estimator.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ const (
// response.
// TODO(yuzefovich): use the optimizer-driven estimates.
InitialAvgResponseSize = 1 << 10 // 1KiB
// This value was determined using tpchvec/bench test on all TPC-H queries.
defaultAvgResponseSizeMultiple = 1.5
// This value was determined using tpchvec/bench test on all TPC-H queries
// as well as the query in TestStreamerVaryingResponseSizes.
defaultAvgResponseSizeMultiple = 3.0
)

// streamerAvgResponseSizeMultiple determines the multiple used when calculating
Expand Down
8 changes: 4 additions & 4 deletions pkg/kv/kvclient/kvstreamer/avg_response_estimator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@ func TestAvgResponseSizeForPartialResponses(t *testing.T) {
}
// We started with the TargetBytes equal to the initial estimate of 1KiB,
// and then with each update the estimate should have grown. In particular,
// we expect 7 BatchRequests total that fetch 1, 1, 3, 7, 18, 45, 25 rows
// respectively (note that the growth is faster than 2x because we use 1.5
// multiple on top of the average).
require.Equal(t, 7, batchRequestsCount)
// we expect 5 BatchRequests total that fetch 1, 3, 12, 48, 36 rows
// respectively (note that the growth is 3-4x because we use 3.0 multiple on
// top of the average).
require.Equal(t, 5, batchRequestsCount)
// From the perspective of the response estimator, we received only one
// response (that happened to be paginated across BatchRequests), so our
// estimate should match exactly the total size.
Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvclient/kvstreamer/streamer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ ALTER TABLE t SPLIT AT SELECT generate_series(1, 30000, 3000);

// The meat of the test - run the query that performs an index join to fetch
// all rows via the streamer, both in the OutOfOrder and InOrder modes. Each
// time assert that the number of BatchRequests issued is in low hundreds
// time assert that the number of BatchRequests issued is in double digits
// (if not, then the streamer was extremely suboptimal).
kvGRPCCallsRegex := regexp.MustCompile(`KV gRPC calls: (\d+,)`)
for inOrder := range []bool{false, true} {
Expand All @@ -600,7 +600,7 @@ ALTER TABLE t SPLIT AT SELECT generate_series(1, 30000, 3000);
break
}
}
require.Greater(t, 300, gRPCCalls, rows)
require.Greater(t, 100, gRPCCalls, rows)
}
}
}

0 comments on commit 116bf04

Please sign in to comment.