Skip to content
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

OAK-9447: Upgrade Mongo java driver to 5.2 #1789

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from
Open

OAK-9447: Upgrade Mongo java driver to 5.2 #1789

wants to merge 3 commits into from

Conversation

reschke
Copy link
Contributor

@reschke reschke commented Oct 14, 2024

No description provided.

reschke and others added 2 commits October 14, 2024 10:27
* OAK-9447

* Conflict with trunk resolved

* Conflict resolved

* conflict resolved

* Reordered imports like original classes and fixed oak-run about new
mongodb-driver-sync

* Reordered imports like original classes

* Upgraded com.mongodb and org.bson versions in Import-Package

* Restored original formatting

* Restored original import ordering

* Restored original import ordering

---------

Co-authored-by: raffaega <raffaega@CI00298583>
Co-authored-by: raffaega <[email protected]>
Co-authored-by: Raffaele Gambelli <[email protected]>
Co-authored-by: Raffaele Gambelli <[email protected]>
@reschke reschke requested review from rishabhdaim, nfsantos and mreutegg and removed request for rishabhdaim and nfsantos October 14, 2024 09:09
@@ -508,7 +507,7 @@ private void configureEstimators(IndexingProgressReporter progressReporter) {
private long getEstimatedDocumentCount() {
MongoConnection mongoConnection = indexHelper.getService(MongoConnection.class);
if (mongoConnection != null) {
return mongoConnection.getDatabase().getCollection("nodes").count();
return mongoConnection.getDatabase().getCollection("nodes").countDocuments();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return mongoConnection.getDatabase().getCollection("nodes").countDocuments();
return mongoConnection.getDatabase().getCollection("nodes").estimatedDocumentCount();

countDcuments is very slow. I would use estimatedDocumentCount for faster results.

See https://www.mongodb.com/community/forums/t/countdocuments-is-extremly-slow/207357

@@ -43,8 +43,8 @@
<configuration>
<instructions>
<Import-Package>
com.mongodb*;version="[3.8, 4)";resolution:=optional,
org.bson*;version="[3.8, 4)";resolution:=optional,
com.mongodb*;version="[5.0, 5.2)";resolution:=optional,
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are switching to 5.2.0, shouldn't it be

Suggested change
com.mongodb*;version="[5.0, 5.2)";resolution:=optional,
com.mongodb*;version="[5.2, 6)";resolution:=optional,

com.mongodb*;version="[3.8, 4)";resolution:=optional,
org.bson*;version="[3.8, 4)";resolution:=optional,
com.mongodb*;version="[5.0, 5.2)";resolution:=optional,
org.bson*;version="[5.0, 5.2)";resolution:=optional,
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

Copy link
Member

@Joscorbe Joscorbe Nov 5, 2024

Choose a reason for hiding this comment

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

Yes, definitely, or 5.3) may be better, as sometimes they also introduce breaking changes or deprecated methods in minor versions. Example in 5.1: https://www.mongodb.com/docs/drivers/java/sync/v5.2/whats-new/#what-s-new-in-5.1
And in 4.x it happened quite more frequently: https://www.mongodb.com/docs/drivers/java/sync/v5.2/upgrade/#version-4.8-breaking-changes

@@ -2345,11 +2339,6 @@ private boolean withClientSession() {
return connection.getStatus().isClientSessionSupported() && useClientSession;
}

private boolean secondariesWithinAcceptableLag() {
return getClient().getReplicaSetStatus() == null
|| connection.getStatus().getReplicaSetLagEstimate() < acceptableLagMillis;
Copy link
Contributor

Choose a reason for hiding this comment

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

acceptableLagMillis should be marked as deprecated since it won't be used anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants