Skip to content

Conversation

@jyemin
Copy link
Collaborator

@jyemin jyemin commented Mar 1, 2021

Previously the driver assumed that Mono#doOnTerminate is executed
prior to the subscriber being notified of completion. But it turns
out that behavior is not guaranteed in the Californium release of
Project Reactor (though it is in later releases). So instead,
now the SingleResultCallback that is used to notify the Mongo of
completion is wrapped by one that first releases the binding, and
Mono#doOnTerminate is no longer used.

JAVA-4027

@jyemin jyemin requested review from rozza and stIncMale March 1, 2021 17:07
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does anyone else find syncToCallback confusing? It seems more like the reverse, callbackToSink, makes more sense.

Copy link
Member

Choose a reason for hiding this comment

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

It converts the sink to a SingleResultCallback so makes sense to me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know what's going on with the indentation settings here.

…ification

Previously the driver assumed that Mono#doOnTerminate is executed
prior to the subscriber being notified of completion.  But it turns
out that behavior is not guaranteed in the Californium release of
Project Reactor (though it is in later releases).  So instead,
now the SingleResultCallback that is used to notify the Mongo of
completion is wrapped by one that first releases the binding, and
Mono#doOnTerminate is no longer used.

JAVA-4027
@jyemin jyemin changed the title Ensure that AsyncReadWriteBinding is released prior to subscriber not… Ensure that AsyncReadWriteBinding is released prior to subscriber notification Mar 1, 2021
Copy link
Member

@rozza rozza left a comment

Choose a reason for hiding this comment

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

LGTM - did this cause any tests to fail?

Copy link
Member

Choose a reason for hiding this comment

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

It converts the sink to a SingleResultCallback so makes sense to me.

@jyemin
Copy link
Collaborator Author

jyemin commented Mar 2, 2021

Yes, see the analysis in JAVA-3969. It's the root cause of the failure of one of the sessions spec tests.

@rozza
Copy link
Member

rozza commented Mar 2, 2021

Good detective work :)

@jyemin jyemin requested a review from stIncMale March 2, 2021 19:05
@jyemin jyemin merged commit dfff51f into mongodb:master Mar 2, 2021
@jyemin jyemin deleted the j3969 branch March 2, 2021 21:50
jyemin added a commit that referenced this pull request Mar 2, 2021
…ification (#676)

Previously the driver assumed that Mono#doOnTerminate is executed
prior to the subscriber being notified of completion.  But it turns
out that behavior is not guaranteed in the Californium release of
Project Reactor (though it is in later releases).  So instead,
now the SingleResultCallback that is used to notify the Mongo of
completion is wrapped by one that first releases the binding, and
Mono#doOnTerminate is no longer used.

JAVA-4027
jfitzu pushed a commit to nextworld-tools/mongo-java-driver that referenced this pull request Mar 3, 2021
…ification (mongodb#676)

Previously the driver assumed that Mono#doOnTerminate is executed
prior to the subscriber being notified of completion.  But it turns
out that behavior is not guaranteed in the Californium release of
Project Reactor (though it is in later releases).  So instead,
now the SingleResultCallback that is used to notify the Mongo of
completion is wrapped by one that first releases the binding, and
Mono#doOnTerminate is no longer used.

JAVA-4027
jfitzu added a commit to nextworld-tools/mongo-java-driver that referenced this pull request Mar 3, 2021
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