CSHARP-5695: CSOT: Unified test spec runner #1758

sanych-sun · 2025-08-18T16:24:28Z

No description provided.

sanych-sun · 2025-08-18T16:26:21Z

src/MongoDB.Driver/Core/Connections/TcpStreamFactory.cs

-
-            var stream = new NetworkStream(socket, true);
-
-            if (_settings.ReadTimeout.HasValue)


Read and Write timeout now is being applied by BinaryConnection.

I had to move this out, because socketTimeout is deprecated by CSOT timeout if it was provided. So we have to apply both in the same level, otherwise it could be a confusion which timeout should be enforced. Even if we applied socket_timeout here briefly and remove on BinaryConnection, we could run into socket timeout when SSL stream authentication.

src/MongoDB.Driver/Core/Connections/BinaryConnection.cs

BorisDog

Initial review

src/MongoDB.Driver/Core/Connections/BinaryConnection.cs

src/MongoDB.Driver/OperationContext.cs

src/MongoDB.Driver/Core/WireProtocol/CommandUsingCommandMessageWireProtocol.cs

src/MongoDB.Driver/Core/Misc/StreamExtensionMethods.cs

BorisDog · 2025-08-25T22:24:32Z

src/MongoDB.Driver/Core/ConnectionPools/ExclusiveConnectionPool.Helpers.cs

+                var waitQueueTimeout = _pool.Settings.WaitQueueTimeout;
+                if (operationContext.RemainingTimeout != Timeout.InfiniteTimeSpan)
+                {
+                    waitQueueTimeout = operationContext.RemainingTimeout < _pool.Settings.WaitQueueTimeout ? operationContext.RemainingTimeout : _pool.Settings.WaitQueueTimeout;


Don't we need Max(TimeSpan) extension method yet :) ?
And then MaxIgnoreInfinite...

Code was rewritten a little to be more readable.

BorisDog · 2025-08-25T22:28:00Z

src/MongoDB.Driver/Core/ConnectionPools/ExclusiveConnectionPool.Helpers.cs

@@ -223,7 +244,8 @@ public async Task<IConnectionHandle> AcquireConnectionAsync(OperationContext ope

                        using (var connectionCreator = new ConnectionCreator(_pool))
                        {
-                            pooledConnection = await connectionCreator.CreateOpenedOrReuseAsync(operationContext).ConfigureAwait(false);
+                            waitQueueTimeout = _pool.CalculateRemainingTimeout(waitQueueTimeout, stopwatch);
+                            pooledConnection = await connectionCreator.CreateOpenedOrReuseAsync(operationContext, waitQueueTimeout - stopwatch.Elapsed).ConfigureAwait(false);


Is this correct?

waitQueueTimeout := waitQueueTimeout - elapsed // Remaining
And then another waitQueueTimeout - stopwatch.Elapsed (==waitQueueTimeout - elapsed * 2) in
CreateOpenedOrReuseAsync(operationContext, waitQueueTimeout - stopwatch.Elapsed) ?

If this is a bug, no tests to catch this?

Good catch! Thank you! I've spotted the problem in sync code path, but forget to do the same fix into async code-path.
Fixed.

src/MongoDB.Driver/Core/WireProtocol/CommandUsingCommandMessageWireProtocol.cs

BorisDog · 2025-08-26T00:46:42Z

src/MongoDB.Driver/Core/Misc/StreamExtensionMethods.cs

            while (count > 0)
            {
-                var timeout = hasOperationTimeout ? operationContext.RemainingTimeout : streamTimeout;
+                var timeout = operationContext.Timeout == null ? socketTimeout : operationContext.RemainingTimeout;


.RemainingTimeoutOrDefaut(defaultTimeout) ?

src/MongoDB.Driver/Core/Connections/BinaryConnection.cs

BorisDog · 2025-08-26T00:51:11Z

src/MongoDB.Driver/Core/ConnectionPools/ExclusiveConnectionPool.Helpers.cs

@@ -223,7 +244,8 @@ public async Task<IConnectionHandle> AcquireConnectionAsync(OperationContext ope

                        using (var connectionCreator = new ConnectionCreator(_pool))
                        {
-                            pooledConnection = await connectionCreator.CreateOpenedOrReuseAsync(operationContext).ConfigureAwait(false);
+                            waitQueueTimeout = _pool.CalculateRemainingTimeout(waitQueueTimeout, stopwatch);
+                            pooledConnection = await connectionCreator.CreateOpenedOrReuseAsync(operationContext, waitQueueTimeout).ConfigureAwait(false);


So no failing tests for this previous issue?

cc @sanych-sun

Non of existing tests were failing.

BorisDog · 2025-08-26T00:53:16Z

tests/MongoDB.Driver.Tests/Specifications/UnifiedTestSpecRunner.cs

+            SkipNotSupportedTestCases("timeoutMS applies to whole operation, not individual attempts"); // blocked by DRIVERS-3247
+            SkipNotSupportedTestCases("WaitQueueTimeoutError does not clear the pool"); // TODO: CSOT: TimeoutMS is not implemented yet for runCommand
+            SkipNotSupportedTestCases("write concern error MaxTimeMSExpired is transformed"); // TODO: CSOT: investigate error transformation, implementing the requirement might be breaking change
+            SkipNotSupportedTestCases("operation succeeds after one socket timeout - listDatabases on client"); // TODO: listDatabases is not retryable in CSharp Driver, NEED TICKET!!!


Ticket created :)

adelinowona · 2025-08-26T14:11:41Z

src/MongoDB.Driver/Core/Bindings/CoreSession.cs

@@ -317,17 +317,17 @@ void ICoreSessionInternal.CommitTransaction(CommitTransactionOptions options, Ca

                try
                {
-                    var firstAttempt = CreateCommitTransactionOperation(IsFirstCommitAttemptRetry());
+                    var firstAttempt = CreateCommitTransactionOperation(operationContext,IsFirstCommitAttemptRetry());


missing space after comma

adelinowona · 2025-08-26T14:11:57Z

src/MongoDB.Driver/Core/Bindings/CoreSession.cs

                {
                    // unpin server if needed, then ignore exception and retry
                    TransactionHelper.UnpinServerIfNeededOnRetryableCommitException(_currentTransaction, exception);
                }

-                var secondAttempt = CreateCommitTransactionOperation(isCommitRetry: true);
+                var secondAttempt = CreateCommitTransactionOperation(operationContext,isCommitRetry: true);


missing space after comma

adelinowona · 2025-08-26T15:10:20Z

src/MongoDB.Driver/Core/Compression/SnappyCompressor.cs

@@ -33,7 +34,7 @@ public void Compress(Stream input, Stream output)
        {
            var uncompressedSize = (int)(input.Length - input.Position);
            var uncompressedBytes = new byte[uncompressedSize]; // does not include uncompressed message headers
-            input.ReadBytes(OperationContext.NoTimeout, uncompressedBytes, offset: 0, count: uncompressedSize);
+            input.ReadBytes(OperationContext.NoTimeout, uncompressedBytes, offset: 0, count: uncompressedSize, Timeout.InfiniteTimeSpan);


Could you use a named argument for the socketTimeout here? so its clear at a glance what that Timeout value at the end is for.

adelinowona · 2025-08-26T15:27:42Z

tests/MongoDB.Driver.Tests/UnifiedTestOperations/UnifiedWithTransactionOperation.cs

                        break;
                    case "readConcern":
-                        options = options ?? new TransactionOptions();
-                        options = options.With(readConcern: ReadConcern.FromBsonDocument(argument.Value.AsBsonDocument));
+                        readConcern =ReadConcern.FromBsonDocument(argument.Value.AsBsonDocument);


missing space after =

BorisDog

Few minor comments

BorisDog · 2025-08-27T00:11:18Z

src/MongoDB.Driver/Core/ConnectionPools/ExclusiveConnectionPool.Helpers.cs

@@ -223,7 +244,8 @@ public async Task<IConnectionHandle> AcquireConnectionAsync(OperationContext ope

                        using (var connectionCreator = new ConnectionCreator(_pool))
                        {
-                            pooledConnection = await connectionCreator.CreateOpenedOrReuseAsync(operationContext).ConfigureAwait(false);
+                            waitQueueTimeout = _pool.CalculateRemainingTimeout(waitQueueTimeout, stopwatch);
+                            pooledConnection = await connectionCreator.CreateOpenedOrReuseAsync(operationContext, waitQueueTimeout).ConfigureAwait(false);


cc @sanych-sun

BorisDog · 2025-08-27T00:15:08Z

src/MongoDB.Driver/OperationContextExtensions.cs

+        public static bool IsRootContextTimeoutConfigured(this OperationContext operationContext) =>
+            operationContext.RootContext.Timeout.HasValue;
+
+        public static TimeSpan RemainingTimeoutOrDefault(this OperationContext operationContext, TimeSpan defaultValue) =>


Worth adding a test.

OperationContextExtensionsTests test class was created.

adelinowona · 2025-08-27T17:54:43Z

src/MongoDB.Driver/Core/Servers/ServerMonitor.cs

@@ -322,6 +321,7 @@ private HelloResult GetHelloResult(

        private void Heartbeat(CancellationToken cancellationToken)
        {
+            using var operationContext = new OperationContext(null, cancellationToken);


I would rather not have this unless there is a timeout that encompasses the entire heartbeat cycle (I doubt we have that). InitializeConnection and GetHelloResult could use their own independent OperationContext instances.

As it was discussed offline: we will keep the code as is for now.

BorisDog

LGTM + nit comment

BorisDog · 2025-08-28T17:17:03Z

tests/MongoDB.Driver.Tests/OperationContextExtensionsTests.cs

+using MongoDB.Driver.Core.Misc;
+using Xunit;
+
+namespace MongoDB.Driver.Tests


File scoped namespace?

adelinowona

LGTM

BorisDog

LGTM

sanych-sun requested a review from a team as a code owner August 18, 2025 16:24

sanych-sun added the feature label Aug 18, 2025

sanych-sun requested review from ajcvickers and removed request for a team and ajcvickers August 18, 2025 16:24

sanych-sun commented Aug 18, 2025

View reviewed changes

src/MongoDB.Driver/Core/Connections/BinaryConnection.cs Show resolved Hide resolved

sanych-sun requested review from BorisDog and adelinowona August 20, 2025 22:07

BorisDog requested changes Aug 25, 2025

View reviewed changes

sanych-sun requested a review from BorisDog August 25, 2025 23:42

BorisDog requested changes Aug 26, 2025

View reviewed changes

adelinowona requested changes Aug 26, 2025

View reviewed changes

sanych-sun requested a review from adelinowona August 26, 2025 19:10

sanych-sun added 10 commits August 26, 2025 15:36

CSHARP-5695: CSOT: Unified test spec runner

e110795

pr

c060957

fix flaky tests

c212af9

Test fixes from CSHARP-5703

16d8156

Sync spec tests

b337bf0

PR

1d5168c

fix tests

11fe700

pr

9600030

pr

180b865

Rebase

1c1a150

sanych-sun force-pushed the csharp5695 branch from fe82d06 to 1c1a150 Compare August 26, 2025 22:39

sanych-sun requested a review from BorisDog August 26, 2025 23:09

BorisDog requested changes Aug 27, 2025

View reviewed changes

adelinowona requested changes Aug 27, 2025

View reviewed changes

sanych-sun added 2 commits August 27, 2025 13:08

Pr

bd9b293

pr

a7a3a3f

sanych-sun requested review from BorisDog and adelinowona August 27, 2025 23:37

BorisDog previously approved these changes Aug 28, 2025

View reviewed changes

adelinowona previously approved these changes Aug 28, 2025

View reviewed changes

pr

613f693

sanych-sun dismissed stale reviews from adelinowona and BorisDog via 613f693 August 28, 2025 19:48

sanych-sun requested review from BorisDog and adelinowona August 28, 2025 19:56

BorisDog approved these changes Aug 28, 2025

View reviewed changes

sanych-sun merged commit c91f85a into mongodb:main Aug 28, 2025
27 of 29 checks passed

sanych-sun deleted the csharp5695 branch August 28, 2025 20:30


		var stream = new NetworkStream(socket, true);

		if (_settings.ReadTimeout.HasValue)

CSHARP-5695: CSOT: Unified test spec runner #1758

CSHARP-5695: CSOT: Unified test spec runner #1758

Uh oh!

Conversation

sanych-sun commented Aug 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

BorisDog Aug 26, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adelinowona left a comment

BorisDog Aug 26, 2025 •

edited

Loading