Skip to content

CDRIVER-4056 Send LB in handshake and reject non-LB connections #828

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 6 commits into from
Jul 26, 2021

Conversation

kevinAlbs
Copy link
Collaborator

@kevinAlbs kevinAlbs commented Jul 15, 2021

  • Add loadBalanced to the handshake.
  • Reject server connections that do not include loadBalanced in the reply.

@kevinAlbs kevinAlbs force-pushed the connection_pooling.loadbalancer branch from 2fffa97 to 8cc94b1 Compare July 22, 2021 21:10
@kevinAlbs kevinAlbs force-pushed the connection_pooling.loadbalancer branch from 8cc94b1 to b790410 Compare July 22, 2021 21:15
@kevinAlbs kevinAlbs marked this pull request as ready for review July 22, 2021 21:59
@kevinAlbs kevinAlbs requested a review from chardan July 22, 2021 21:59
Copy link
Contributor

@chardan chardan left a comment

Choose a reason for hiding this comment

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

This looks good to me! I'm going a little bit on faith that the tests test what they're supposed to test (looks like they do)...

*/
bool
mongoc_server_description_service_id (
const mongoc_server_description_t *description, bson_oid_t *oid);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: It's probably overly-pedantic, as basically nobody writes this, but did you mean:

bool
mongoc_server_description_service_id (
const mongoc_server_description_t * const description, bson_oid_t *oid);

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 did not. I do not think a const pointer adds a guarantee from the caller's perspective.

If mongoc_server_description_service_id promises not to change what description points to, a caller could not observe it.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1. And I don't think it really buys anything, either.

@kevinAlbs kevinAlbs requested review from chardan and benjirewis July 22, 2021 23:36
Copy link
Contributor

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

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

LGTM, just a nit.

@@ -31,6 +31,8 @@ Many C Driver functions report errors by returning ``false`` or -1 and filling o
+-----------------------------------------+---------------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| | ``MONGOC_ERROR_CLIENT_INVALID_ENCRYPTION_STATE`` | Failure related to Client-Side Field Level Encryption. |
+-----------------------------------------+---------------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| | ``MONGOC_ERROR_CLIENT_INVALID_LOAD_BALANCER`` | You attempted to connect to a MongoDB server behind a load balancer, but the server does not advertise load balanced support. |
Copy link
Contributor

Choose a reason for hiding this comment

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

British spelling for advertise but I assume this error message was copied from somewhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TIL I've been using the British spelling. Updated.

@kevinAlbs kevinAlbs changed the title CDRIVER-4056 Add loadBalanced to handshake and reject non-loadBalanced connections CDRIVER-4056 Send LB in handshake and reject non-LB connections Jul 26, 2021
@kevinAlbs kevinAlbs merged commit 4919d4f into mongodb:master Jul 26, 2021
@kevinAlbs kevinAlbs deleted the connection_pooling.loadbalancer branch July 26, 2021 13:45
Copy link
Contributor

@chardan chardan left a comment

Choose a reason for hiding this comment

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

+1 LGTM

@kevinAlbs kevinAlbs restored the connection_pooling.loadbalancer branch February 3, 2022 14:29
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