CDRIVER-6182 break resume loop after two timeouts #2194

kevinAlbs · 2026-01-07T19:23:05Z

Summary

Break the change stream resume loop after two socket timeouts.

Background & Motivation

DRIVERS-1404 describes the problem this PR intends to address:

If the getMore needs 5 minutes to do this and the socket timeout on the MongoClient is 1 min, I think we'll enter a repeated resume loop because the aggregate will succeed and the getMore will always time out.

This scenario has known impact (HELP-83560). SERVER-48526 may help address this in the server. This PR proposes a driver solution: break after two timeouts in the resume loop. This is similarly suggested in DRIVERS-1309:

short-circuiting the retry loop after two socket timeouts (not necessarily consecutive) seems like a good compromise.

To propagate a timeout from mongoc_stream_t, a bool is set on mongoc_server_stream_t. mongoc_stream_t is destroyed after a network error in _handle_network_error, so it cannot later be checked with mongoc_stream_timed_out. This is similarly propagated in mongoc_cursor_t, which only retrieves a mongoc_server_stream for the duration of a command.

Rejected alternative: bound resume loop by `socketTimeoutMS`

Another idea considered was to bound the resume loop by the duration socketTimeoutMS. Bounding by socketTimeoutMS may better align with timeoutMS:

If a resume is required for a next call on a change stream, the timeout MUST apply to the entirety of the initial getMore and all commands sent as part of the resume attempt.

However, I think it further removes socketTimeoutMS from the documented behavior "to send or receive on a socket". This idea was rejected.

kevinAlbs · 2026-01-08T13:28:15Z

src/libmongoc/tests/test-conveniences.c


   ctx.is_command = is_command;
-   matches = match_bson_with_ctx(doc, pattern, &ctx);
+   bson_t empty = BSON_INITIALIZER;


Drive-by fix to support NULL for doc and matches the comment above this function:

A NULL doc or NULL json_pattern means "{}".

eramongodb

Minor suggestions; otherwise, LGTM.

eramongodb · 2026-01-09T15:58:02Z

src/libmongoc/src/mongoc/mongoc-cursor-private.h


   bson_error_t error;
-   bson_t error_doc; /* always initialized, and set with server errors. */
+   bson_t error_doc;        /* always initialized, and set with server errors. */


Suggested change

bson_t error_doc; /* always initialized, and set with server errors. */

bson_t error_doc; // always initialized, and set with server errors.

Local comment style consistency.

eramongodb · 2026-01-09T16:02:34Z

src/libmongoc/src/mongoc/mongoc-change-stream.c

         if (err_doc) {
            resumable = _is_resumable_error(stream, err_doc);
+            if (stream->cursor->had_stream_timeout) {
+               iteration_timeout_count++;
+            }
         } else {
            resumable = false;
         }
+
+         if (iteration_timeout_count >= 2) {
+            // CDRIVER-6182: Do not resume if two iteration timeouts occur. Intended to avoid a possible resume loop
+            // when `aggregate` succeeds but `getMore` consistently times out.
+            resumable = false;
+         }


Suggested change

if (err_doc) {

resumable = _is_resumable_error(stream, err_doc);

if (stream->cursor->had_stream_timeout) {

iteration_timeout_count++;

}

} else {

resumable = false;

}

if (iteration_timeout_count >= 2) {

// CDRIVER-6182: Do not resume if two iteration timeouts occur. Intended to avoid a possible resume loop

// when `aggregate` succeeds but `getMore` consistently times out.

resumable = false;

}

if (err_doc) {

if (stream->cursor->had_stream_timeout) {

iteration_timeout_count++;

}

// CDRIVER-6182: Do not resume if two iteration timeouts occur. Intended to avoid a possible resume loop

// when `aggregate` succeeds but `getMore` consistently times out.

if (iteration_timeout_count >= 2) {

resumable = false;

} else {

resumable = _is_resumable_error(stream, err_doc);

}

} else {

resumable = false;

}

Suggest tweaking the layout of blocks as suggested to consistently ensure resumable is set once the if (err_doc) block is evaluated by moving each resumable = ... to the end of every possible branch.

One more suggestion: perhaps the CDRIVER-6182 branch should include a MONGOC_WARNING to notify users when this scenario is triggered?

Suggest tweaking the layout

This led me to realize the if(err_doc) check was redundant. This is only entered if mongoc_cursor_error_document previously returned true. mongoc_cursor_error_document guarantees:

If the function returns true and reply is not NULL, then reply is set to a pointer to a BSON document, which is either empty or the server’s error response.

Changed if (err_doc) to BSON_ASSERT (err_doc) and also applied the suggested tweaking to use if/else so resumable is only set once.

One more suggestion

I like that idea. Added a warning.

eramongodb · 2026-01-09T16:25:00Z

src/libmongoc/src/mongoc/mongoc-cursor.c


   ENTRY;

+   cursor->had_stream_timeout = false; // Reset before running next command.


Suggest moving further below just before the if (parts.assembled.session) block to group together with cursor-state-modifying operations, e.g. cursor->client_session = ...;, cursor->explicit_session = ...;, etc.

kevinAlbs

Latest changes applies skip_if_high_server_runtime_variance from #2195

kevinAlbs · 2026-01-09T18:24:12Z

src/libmongoc/src/mongoc/mongoc-change-stream.c

         if (err_doc) {
            resumable = _is_resumable_error(stream, err_doc);
+            if (stream->cursor->had_stream_timeout) {
+               iteration_timeout_count++;
+            }
         } else {
            resumable = false;
         }
+
+         if (iteration_timeout_count >= 2) {
+            // CDRIVER-6182: Do not resume if two iteration timeouts occur. Intended to avoid a possible resume loop
+            // when `aggregate` succeeds but `getMore` consistently times out.
+            resumable = false;
+         }


Suggest tweaking the layout

This led me to realize the if(err_doc) check was redundant. This is only entered if mongoc_cursor_error_document previously returned true. mongoc_cursor_error_document guarantees:

If the function returns true and reply is not NULL, then reply is set to a pointer to a BSON document, which is either empty or the server’s error response.

Changed if (err_doc) to BSON_ASSERT (err_doc) and also applied the suggested tweaking to use if/else so resumable is only set once.

One more suggestion

I like that idea. Added a warning.

support NULL for doc in ASSERT_MATCH

5a6b54a

kevinAlbs commented Jan 8, 2026

View reviewed changes

CDRIVER-6182 break resume loop after two timeouts

df213b8

kevinAlbs force-pushed the 2timeouts.D1404.changestream branch from 9fd63a6 to df213b8 Compare January 8, 2026 13:32

kevinAlbs requested review from connorsmacd and eramongodb January 8, 2026 13:32

kevinAlbs marked this pull request as ready for review January 8, 2026 13:32

kevinAlbs requested a review from a team as a code owner January 8, 2026 13:33

connorsmacd approved these changes Jan 8, 2026

View reviewed changes

eramongodb approved these changes Jan 9, 2026

View reviewed changes

kevinAlbs added 7 commits January 9, 2026 11:52

make comment style more consistent

3e33772

remove redundant if (err_doc) check

dd3c772

use if/else

776f3c7

add warning

c179b2a

relocate assignment of had_stream_timeout

f02fb99

Merge branch 'master' into 2timeouts.D1404.changestream

5c76467

skip if high server runtime variance

0a288e4

kevinAlbs commented Jan 13, 2026

View reviewed changes

kevinAlbs merged commit 230f8bd into mongodb:master Jan 13, 2026
46 of 49 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CDRIVER-6182 break resume loop after two timeouts #2194

CDRIVER-6182 break resume loop after two timeouts #2194

kevinAlbs commented Jan 7, 2026 •

edited

Loading

Uh oh!

kevinAlbs Jan 8, 2026

Uh oh!

eramongodb left a comment

Uh oh!

eramongodb Jan 9, 2026

Uh oh!

eramongodb Jan 9, 2026

Uh oh!

eramongodb Jan 9, 2026 •

edited

Loading

Uh oh!

kevinAlbs Jan 9, 2026

Uh oh!

eramongodb Jan 9, 2026

Uh oh!

kevinAlbs left a comment

Uh oh!

kevinAlbs Jan 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	bson_t error_doc; /* always initialized, and set with server errors. */
	bson_t error_doc; // always initialized, and set with server errors.


		ENTRY;

		cursor->had_stream_timeout = false; // Reset before running next command.

CDRIVER-6182 break resume loop after two timeouts #2194

CDRIVER-6182 break resume loop after two timeouts #2194

Conversation

kevinAlbs commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Background & Motivation

Rejected alternative: bound resume loop by socketTimeoutMS

Uh oh!

kevinAlbs Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

eramongodb left a comment

Choose a reason for hiding this comment

Uh oh!

eramongodb Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

eramongodb Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

eramongodb Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinAlbs Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

eramongodb Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

kevinAlbs left a comment

Choose a reason for hiding this comment

Uh oh!

kevinAlbs Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kevinAlbs commented Jan 7, 2026 •

edited

Loading

Rejected alternative: bound resume loop by `socketTimeoutMS`

eramongodb Jan 9, 2026 •

edited

Loading