Skip to content

Commit 62f8bc4

Browse files
GODRIVER-3444 Resovle merge conflicts
2 parents db060be + 92b8b5b commit 62f8bc4

File tree

5 files changed

+47
-17
lines changed

5 files changed

+47
-17
lines changed

internal/driverutil/operation.go

+9-11
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ package driverutil
88

99
import (
1010
"context"
11-
"fmt"
1211
"math"
1312
"time"
1413
)
@@ -38,10 +37,14 @@ const (
3837
BulkWriteOp = "bulkWrite" // BulkWriteOp is the name for client-level bulk write
3938
)
4039

41-
func CalculateMaxTimeMS(ctx context.Context, rttMin time.Duration, rttStats string, err error) (int64, error) {
40+
// CalculateMaxTimeMS calculates the maxTimeMS value to send to the server
41+
// based on the context deadline and the minimum round trip time. If the
42+
// calculated maxTimeMS is likely to cause a socket timeout, then this function
43+
// will return 0 and false.
44+
func CalculateMaxTimeMS(ctx context.Context, rttMin time.Duration) (int64, bool) {
4245
deadline, ok := ctx.Deadline()
4346
if !ok {
44-
return 0, nil
47+
return 0, true
4548
}
4649

4750
remainingTimeout := time.Until(deadline)
@@ -50,12 +53,7 @@ func CalculateMaxTimeMS(ctx context.Context, rttMin time.Duration, rttStats stri
5053
// maxTimeMS value (e.g. 400 microseconds evaluates to 1ms, not 0ms).
5154
maxTimeMS := int64((remainingTimeout - rttMin) / time.Millisecond)
5255
if maxTimeMS <= 0 {
53-
return 0, fmt.Errorf(
54-
"remaining time %v until context deadline is less than or equal to min network round-trip time %v (%v): %w",
55-
remainingTimeout,
56-
rttMin,
57-
rttStats,
58-
err)
56+
return 0, false
5957
}
6058

6159
// The server will return a "BadValue" error if maxTimeMS is greater
@@ -64,8 +62,8 @@ func CalculateMaxTimeMS(ctx context.Context, rttMin time.Duration, rttStats stri
6462
// and let the client-side timeout handle cancelling the op if the
6563
// timeout is ever reached.
6664
if maxTimeMS > math.MaxInt32 {
67-
return 0, nil
65+
return 0, true
6866
}
6967

70-
return maxTimeMS, nil
68+
return maxTimeMS, true
7169
}

internal/integration/unified/cursor_operation_execution.go

-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ func executeIterateOnce(ctx context.Context, operation *operation) (*operationRe
3030

3131
return newDocumentResult(res, nil), nil
3232
}
33-
3433
return newErrorResult(cursor.Err()), nil
3534
}
3635

internal/spectest/skip.go

+19
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,10 @@ var skipTests = map[string][]string{
346346
"TestUnifiedSpec/client-side-operations-timeout/tests/retryability-timeoutMS.json/operation_is_retried_multiple_times_for_non-zero_timeoutMS_-_aggregate_on_collection",
347347
"TestUnifiedSpec/client-side-operations-timeout/tests/retryability-timeoutMS.json/operation_is_retried_multiple_times_for_non-zero_timeoutMS_-_aggregate_on_database",
348348
"TestUnifiedSpec/client-side-operations-timeout/tests/gridfs-find.json/timeoutMS_applied_to_find_command",
349+
"TestUnifiedSpec/client-side-operations-timeout/tests/tailable-awaitData.json/timeoutMS_applied_to_find",
350+
"TestUnifiedSpec/client-side-operations-timeout/tests/tailable-awaitData.json/timeoutMS_is_refreshed_for_getMore_if_maxAwaitTimeMS_is_not_set",
351+
"TestUnifiedSpec/client-side-operations-timeout/tests/tailable-awaitData.json/timeoutMS_is_refreshed_for_getMore_if_maxAwaitTimeMS_is_set",
352+
"TestUnifiedSpec/client-side-operations-timeout/tests/tailable-awaitData.json/timeoutMS_is_refreshed_for_getMore_-_failure",
349353
},
350354

351355
// TODO(GODRIVER-3411): Tests require "getMore" with "maxTimeMS" settings. Not
@@ -819,6 +823,21 @@ var skipTests = map[string][]string{
819823
"TestUnifiedSpec/transactions-convenient-api/tests/unified/transaction-options.json/withTransaction_explicit_transaction_options_override_client_options",
820824
"TestUnifiedSpec/transactions-convenient-api/tests/unified/commit.json/withTransaction_commits_after_callback_returns",
821825
},
826+
827+
// GODRIVER-3473: the implementation of DRIVERS-2868 makes it clear that the
828+
// Go Driver does not correctly implement the following validation for
829+
// tailable awaitData cursors:
830+
//
831+
// Drivers MUST error if this option is set, timeoutMS is set to a
832+
// non-zero value, and maxAwaitTimeMS is greater than or equal to
833+
// timeoutMS.
834+
//
835+
// Once GODRIVER-3473 is completed, we can continue running these tests.
836+
"When constructing tailable awaitData cusors must validate, timeoutMS is set to a non-zero value, and maxAwaitTimeMS is greater than or equal to timeoutMS (GODRIVER-3473)": {
837+
"TestUnifiedSpec/client-side-operations-timeout/tailable-awaitData.json/apply_remaining_timeoutMS_if_less_than_maxAwaitTimeMS",
838+
"TestUnifiedSpec/client-side-operations-timeout/tailable-awaitData.json/error_if_maxAwaitTimeMS_is_equal_to_timeoutMS",
839+
"TestUnifiedSpec/client-side-operations-timeout/tailable-awaitData.json/error_if_maxAwaitTimeMS_is_greater_than_timeoutMS",
840+
},
822841
}
823842

824843
// CheckSkip checks if the fully-qualified test name matches a list of skipped test names for a given reason.

x/mongo/driver/batch_cursor.go

+8-4
Original file line numberDiff line numberDiff line change
@@ -391,10 +391,14 @@ func (bc *BatchCursor) getMore(ctx context.Context) {
391391
if ctxDeadlineSet {
392392
rttMonitor := bc.Server().RTTMonitor()
393393

394-
var err error
395-
maxTimeMS, err = driverutil.CalculateMaxTimeMS(ctx, rttMonitor.Min(), rttMonitor.Stats(), ErrDeadlineWouldBeExceeded)
396-
if err != nil {
397-
return nil, err
394+
var ok bool
395+
maxTimeMS, ok = driverutil.CalculateMaxTimeMS(ctx, rttMonitor.Min())
396+
if !ok && maxTimeMS <= 0 {
397+
return nil, fmt.Errorf(
398+
"calculated server-side timeout (%v ms) is less than or equal to 0 (%v): %w",
399+
maxTimeMS,
400+
rttMonitor.Stats(),
401+
ErrDeadlineWouldBeExceeded)
398402
}
399403
}
400404

x/mongo/driver/operation.go

+11-1
Original file line numberDiff line numberDiff line change
@@ -1724,7 +1724,17 @@ func (op Operation) calculateMaxTimeMS(ctx context.Context, rttMin time.Duration
17241724
return 0, nil
17251725
}
17261726

1727-
return driverutil.CalculateMaxTimeMS(ctx, rttMin, rttStats, ErrDeadlineWouldBeExceeded)
1727+
// Calculate maxTimeMS value to potentially be appended to the wire message.
1728+
maxTimeMS, ok := driverutil.CalculateMaxTimeMS(ctx, rttMin)
1729+
if !ok && maxTimeMS <= 0 {
1730+
return 0, fmt.Errorf(
1731+
"calculated server-side timeout (%v ms) is less than or equal to 0 (%v): %w",
1732+
maxTimeMS,
1733+
rttStats,
1734+
ErrDeadlineWouldBeExceeded)
1735+
}
1736+
1737+
return maxTimeMS, nil
17281738
}
17291739

17301740
// updateClusterTimes updates the cluster times for the session and cluster clock attached to this

0 commit comments

Comments
 (0)