Skip to content

JAVA-4320 fix server selection flaky test #1847

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.

Sign up for GitHub

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

Open
strogiyotec wants to merge 4 commits into
base: main
Choose a base branch
from
Open

JAVA-4320 fix server selection flaky test #1847

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d00af73
JAVA-4320 fix server selection flaky test
Dec 2, 2025
4f00529
Merge branch 'main' into JAVA-4320
Dec 5, 2025
1b68542
set server selection threshold to 3seconds
Dec 9, 2025
5ab863e
serverSelection flaky test, update comments
Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions driver-sync/src/test/functional/com/mongodb/client/AbstractServerSelectionProseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
import com.mongodb.ServerAddress;
import com.mongodb.event.CommandStartedEvent;
import com.mongodb.internal.connection.TestCommandListener;

import java.util.concurrent.TimeUnit;

import org.bson.BsonArray;
import org.bson.BsonBoolean;
import org.bson.BsonDocument;
Expand Down Expand Up @@ -77,6 +80,8 @@ void operationCountBasedSelectionWithinLatencyWindow() throws InterruptedExcepti
MongoClientSettings clientSettings = getMongoClientSettingsBuilder()
.applicationName(appName)
.applyConnectionString(multiMongosConnectionString)
// set it to 3000 ms according to specification
Copy link
Member

@vbabanin vbabanin Dec 10, 2025

Choose a reason for hiding this comment

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

[nit] The comment can give the impression that only this setting is spec-compliant, while the rest might not be. Should we remove this one?

Copy link
Member

@stIncMale stIncMale Dec 10, 2025

Choose a reason for hiding this comment

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

Instead of removing the comment, let's update it to explain the full story:

  • our default localThreshold value is 15 ms, which deviates from the default value 3000 ms required by the specification;
  • such a small localThreshold disrupts server selection in this probabilistic test enough to make it fail noticeably often.

Copy link
Contributor Author

@strogiyotec strogiyotec Dec 11, 2025

Choose a reason for hiding this comment

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

I updated the comment as was mentioned by @stIncMale

Copy link
Member

@stIncMale stIncMale Dec 11, 2025

Choose a reason for hiding this comment

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

1. Could we please make the comment to look like correct English: proper use of capital letters, periods at the end of sentences?
2. We actually do not need the comment because my proposal was based on an incorrect understanding. I thought, you mean that the Java driver deviates from the spec, which requires the default localThreshold to be 3000 ms. But that is not the case, the spec requires 15 ms, which is our default, that is, we are compliant. It's the prose test that requires setting localThreshold to 30000 ms (not 3000 ms!), which we did not set. So let's update the test code accordingly, and remove the comment, as it is not needed at all.

.applyToClusterSettings(builder -> builder.localThreshold(3000L, TimeUnit.MILLISECONDS))
.applyToConnectionPoolSettings(builder -> builder
.minSize(tasks))
.addCommandListener(commandListener)
Expand Down