Skip to content

Update cursors to refresh timeoutMS on close without affecting the timeout of the operation#1527

Merged
stIncMale merged 4 commits intomongodb:mainfrom
stIncMale:JAVA-5615
Nov 21, 2024
Merged

Update cursors to refresh timeoutMS on close without affecting the timeout of the operation#1527
stIncMale merged 4 commits intomongodb:mainfrom
stIncMale:JAVA-5615

Conversation

@stIncMale
Copy link
Copy Markdown
Member

@stIncMale stIncMale commented Oct 8, 2024

@stIncMale stIncMale requested a review from vbabanin October 8, 2024 22:40
@stIncMale stIncMale self-assigned this Oct 8, 2024
stIncMale
stIncMale commented Oct 8, 2024
View reviewed changes
Comment thread driver-core/src/test/functional/com/mongodb/internal/operation/AsyncCommandBatchCursorTest.java Outdated
connectionSource = mock(AsyncConnectionSource.class);
operationContext = mock(OperationContext.class);
timeoutContext = mock(TimeoutContext.class);
timeoutContext = new TimeoutContext(TimeoutSettings.create(MongoClientSettings.builder().timeout(3, TimeUnit.SECONDS).build()));
Copy link
Copy Markdown
Member Author

@stIncMale stIncMale Oct 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to get rid of unnecessary mocking here and in other places to make the tests pass with the new code in cursors.

stIncMale
stIncMale commented Oct 8, 2024
View reviewed changes
Comment thread driver-core/src/test/unit/com/mongodb/internal/async/AsyncFunctionsAbstractTest.java Outdated
@stIncMale
Copy link
Copy Markdown
Member Author

stIncMale commented Oct 9, 2024 •
edited
Loading

Given how flaky our tests are, it is difficult to tell whether the failures in the PR are caused by the PR, or they've been in the main branch, and Evergreen provides no tools to facilitate searching for an answer even for a single test failure. So I had to search manually, and try running tests from the main branch. Following are the result.

Failures that don't seem to be new:

  1. Load Balancer 8.0 Auth SSL JDK21 Ubuntu: com.mongodb.reactivestreams.client.unified.LoadBalancerTest.cursors_are_correctly_pinned_to_connections_for_load-balanced_clusters__pinned_connections_are_returned_after_a_network_error_during_a_killCursors_request (happened in main here)
  2. 4.2 Sharded Cluster NoAuth NoSSL JDK8 Linux: com.mongodb.client.ServerSelectionProseTest.operationCountBasedSelectionWithinLatencyWindow__ (happened in main, see here)
  3. 7.0 Replica Set Auth SSL JDK8 Linux: com.mongodb.reactivestreams.client.ClientSideOperationTimeoutProseTest.6._GridFS_Download (reproduced against main here)

Failures that may be new (I failed to manually find such failures in main, but my search was quite limited):

  1. 6.0 Sharded Cluster NoAuth NoSSL JDK8 Linux: com.mongodb.client.unified.ClientSideEncryptionTest.namedKMS-rewrapManyDataKey__rewrap_to_azure_name1
  2. the same test as in 0.6 driver does not pass through ID of new docs correctly #2 5.0 Replica Set NoAuth NoSSL JDK8 Linux: com.mongodb.client.unified.ClientSideEncryptionTest.namedKMS-rewrapManyDataKey__rewrap_to_kmip_name1

When I re-run the later ones here, both succeeded.

vbabanin
vbabanin reviewed Nov 9, 2024
View reviewed changes
Comment thread driver-core/src/main/com/mongodb/internal/operation/CommandBatchCursor.java Outdated
Comment thread driver-core/src/main/com/mongodb/internal/operation/CommandBatchCursor.java Outdated
Comment thread driver-core/src/test/unit/com/mongodb/internal/operation/CommandBatchCursorTest.java
Comment thread driver-core/src/main/com/mongodb/internal/operation/AsyncCommandBatchCursor.java Outdated
@stIncMale stIncMale requested a review from vbabanin November 16, 2024 09:48
@stIncMale stIncMale merged commit 600f2c6 into mongodb:main Nov 21, 2024
@stIncMale stIncMale deleted the JAVA-5615 branch November 21, 2024 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vbabanin vbabanin vbabanin approved these changes

Assignees

@stIncMale stIncMale

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@stIncMale @vbabanin