Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

requestbatcher: deflake TestBatchTimeout #98512

Merged
merged 1 commit into from
Mar 14, 2023
Merged

requestbatcher: deflake TestBatchTimeout #98512

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions pkg/internal/client/requestbatcher/batcher_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,6 @@ func TestBatchTimeout(t *testing.T) {
const timeout = 5 * time.Millisecond
stopper := stop.NewStopper()
defer stopper.Stop(context.Background())
sc := make(chanSender)
testCases := []struct {
requestTimeout time.Duration
maxTimeout time.Duration
Expand Down Expand Up @@ -350,6 +349,7 @@ func TestBatchTimeout(t *testing.T) {
for _, tc := range testCases {
t.Run(fmt.Sprintf("With%sRequestTimeout%sMaxTimeout", tc.requestTimeout, tc.maxTimeout),
func(t *testing.T) {
sc := make(chanSender)
b := New(Config{
// MaxMsgsPerBatch of 1 is chosen so that the first call to Send will
// immediately lead to a batch being sent.
Expand Down Expand Up @@ -387,8 +387,9 @@ func TestBatchTimeout(t *testing.T) {
)
}
t.Run("WithTimeoutAndPagination", func(t *testing.T) {
maxTimeout := 20 * time.Millisecond
maxTimeout := 45 * time.Second
firstAndSecondSendTimeDiff := 10 * time.Millisecond
sc := make(chanSender)
b := New(Config{
// MaxMsgsPerBatch of 1 is chosen so that the first call to Send will
// immediately lead to a batch being sent.
Expand All @@ -398,13 +399,14 @@ func TestBatchTimeout(t *testing.T) {
MaxTimeout: maxTimeout,
})
// This test will simulate multiple calls to Send (due to pagination) and
// test that the MaxTimeout set is a timeout per batch rather than a
// timeout per request.
// test that the MaxTimeout set is a timeout per batch rather than a timeout
// per request. MaxTimeout is set to a large value, so the timeout should
// never actually be hit. This means that the subtest does not face the same
// timing issues that the subtests above do.
ctx := context.Background()
respChan := make(chan Response, 1)
if err := b.SendWithChan(context.Background(), respChan, 1, &kvpb.GetRequest{}); err != nil {
testutils.IsError(err, context.DeadlineExceeded.Error())
return
}
err := b.SendWithChan(ctx, respChan, 1, &kvpb.GetRequest{})
require.NoError(t, err)
// First call to Send.
s := <-sc
time.Sleep(firstAndSecondSendTimeDiff)
Expand Down Expand Up @@ -432,6 +434,7 @@ func TestBatchTimeout(t *testing.T) {
s.respChan <- batchResp{}
})
t.Run("NoTimeout", func(t *testing.T) {
sc := make(chanSender)
b := New(Config{
// MaxMsgsPerBatch of 2 is chosen so that the second call to Send will
// immediately lead to a batch being sent.
Expand Down