-
Notifications
You must be signed in to change notification settings - Fork 904
GODRIVER-3444 Adjust getMore maxTimeMS Calculation for tailable awaitData Cursors #1925
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.
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
Conversation
API Change ReportNo changes found! |
…iver into GODRIVER-3444
internal/driverutil/operation.go
Outdated
@@ -30,3 +37,35 @@ const ( | |||
UpdateOp = "update" // UpdateOp is the name for updating | |||
BulkWriteOp = "bulkWrite" // BulkWriteOp is the name for client-level bulk write | |||
) | |||
|
|||
func CalculateMaxTimeMS(ctx context.Context, rttMin time.Duration, rttStats string, err error) (int64, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's unconventional to pass the error to be returned in as a parameter. Instead, consider letting CalculateMaxTimeMS
return just the int64
value, and let the caller form the error value.
E.g. use in batch_cursor.go
:
maxTimeMS = driverutil.CalculateMaxTimeMS(ctx, rttMonitor.Min())
if maxTimeMS <= 0 {
return nil, fmt.Errorf(
"calculated server-side timeout (%v ms) is less than or equal to 0 (%v): %w",
maxTimeMS,
rttMonitor.Stats(),
ErrDeadlineWouldBeExceeded)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Co-authored-by: Copilot <[email protected]>
GODRIVER-3444
Summary
If
maxAwaitTime
option is set, use themin(maxAwaitTimeMS, remaining timeoutMS)
as themaxTimeMS
field on getMore commands.Background & Motivation
From the DRIVERS-2868 proposal: