Skip to content

Remove RTT check before reading a response from the server. #1391

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

Merged
merged 1 commit into from
May 18, 2024

Conversation

vbabanin
Copy link
Member

@@ -509,6 +502,16 @@ private <T> void sendCommandMessageAsync(final int messageId, final Decoder<T> d
final SingleResultCallback<T> callback, final ByteBufferBsonOutput bsonOutput,
final CommandEventSender commandEventSender, final boolean responseExpected) {
List<ByteBuf> byteBuffers = bsonOutput.getByteBuffers();

boolean[] shouldReturn = {false};
Copy link
Member Author

@vbabanin vbabanin May 15, 2024

Choose a reason for hiding this comment

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

I've moved this check from "after the write is complete" to "before a message is send."

According to the CSOT spec:

After wire message construction, drivers MUST check for timeout before writing the message to the server.

@@ -349,9 +349,6 @@ public <T> T sendAndReceive(final CommandMessage message, final Decoder<T> decod
CommandEventSender commandEventSender;

try (ByteBufferBsonOutput bsonOutput = new ByteBufferBsonOutput(this)) {
Timeout.onExistsAndExpired(operationContext.getTimeoutContext().timeoutIncludingRoundTrip(), () -> {
Copy link
Member Author

@vbabanin vbabanin May 15, 2024

Choose a reason for hiding this comment

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

According to the specification:

After wire message construction, drivers MUST check for timeout before writing the message to the server.

I have removed this check from its previous position as it was occurring before the wire message was constructed, not after, which is contrary to the spec. We still perform this necessary timeout check later in the code within the trySendMessage method, just before writing the message to the socket.

@vbabanin vbabanin requested a review from rozza May 15, 2024 01:52
@vbabanin vbabanin marked this pull request as ready for review May 15, 2024 03:14
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

@vbabanin vbabanin merged commit 0d2d8e8 into mongodb:CSOT May 18, 2024
56 of 58 checks passed
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.

2 participants