[CDRIVER-2270] _mongoc_topology_description_check_compatible doesn't allow for unknown states Created: 08/Sep/17  Updated: 28/Oct/23  Resolved: 12/Sep/17

Status: Closed
Project: C Driver
Component/s: None
Affects Version/s: 1.8.0
Fix Version/s: 1.8.0

Type: Bug Priority: Major - P3
Reporter: Hannes Magnusson Assignee: A. Jesse Jiryu Davis
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
related to CDRIVER-689 Detect incompatible wire versions Closed

 Description   

_mongoc_topology_description_check_compatible determines the topology to be incompatible if any server is outside of the compatible range.

Since we supported wire version 0, and that was the default value for a reset/disconnected server_description nothing was ever outside of the range.

Now that minimum wire version is 2, a unknown state server_description will have wire_version=0, yet still part of the topology (just not selectable).

However, now the compatible check will fail because it found a server outside of the range.

I think we should probably maybe initialize the server_descriptions with wire version -1 or something, or otherwise skip checks for servers in unknown state.



 Comments   
Comment by Githook User [ 19/Sep/17 ]

Author:

{'username': 'bjori', 'name': 'Hannes Magnusson', 'email': 'bjori@php.net'}

Message: CDRIVER-2270 Remove duplicate fix
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/f48a5d97b1a2ab3df24a6b360ffede83ac3e4ed6

Comment by Ramon Fernandez Marina [ 18/Sep/17 ]

Author:

{'username': u'bjori', 'name': u'Hannes Magnusson', 'email': u'bjori@php.net'}

Message:Merge branch 'CDRIVER-2243-drop-2.4-support'

Comment by Githook User [ 18/Sep/17 ]

Author:

{'username': 'bjori', 'name': 'Hannes Magnusson', 'email': 'bjori@php.net'}

Message: CDRIVER-2270 Fix possible primary
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/52eae75dbf98d65eb2b7113f68de36c2f1ffdfef

Comment by Githook User [ 18/Sep/17 ]

Author:

{'username': 'bjori', 'name': 'Hannes Magnusson', 'email': 'bjori@php.net'}

Message: CDRIVER-2270 _mongoc_topology_description_check_compatible doesn't allow for unknown states
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/832bba96dd9411c38fb68439384b7f884affb0fa

Comment by Hannes Magnusson [ 13/Sep/17 ]

There is still a bug, where a segfault will occur if the topology is incompatible and the optional error was not provided.

See also: https://mongodbcr.appspot.com/162150001/diff/80001/src/mongoc/mongoc-topology.c#newcode433

Comment by Ramon Fernandez Marina [ 13/Sep/17 ]

Author:

{'username': u'ajdavis', 'name': u'A. Jesse Jiryu Davis', 'email': u'jesse@mongodb.com'}

Message:CDRIVER-2270 compatibility logic fix, part 2
Branch:master
https://github.com/mongodb/mongo-c-driver/commit/382334a61e756e5209566612360a2ca2e253eb1c

Comment by Ramon Fernandez Marina [ 13/Sep/17 ]

Author:

{'username': u'ajdavis', 'name': u'A. Jesse Jiryu Davis', 'email': u'jesse@mongodb.com'}

Message:CDRIVER-2270 compatibility logic fix, part 2
Branch:r1.8
https://github.com/mongodb/mongo-c-driver/commit/4746b65b978177b9de44728914b96ac8163dce4a

Comment by Ramon Fernandez Marina [ 12/Sep/17 ]

Author:

{'username': u'ajdavis', 'name': u'A. Jesse Jiryu Davis', 'email': u'jesse@mongodb.com'}

Message:CDRIVER-2270 compatibility logic fix
Branch:master
https://github.com/mongodb/mongo-c-driver/commit/590cdfe1fa7d5c299db1d1ca25f8c86f7b31bde2

Comment by Ramon Fernandez Marina [ 12/Sep/17 ]

Author:

{'username': u'ajdavis', 'name': u'A. Jesse Jiryu Davis', 'email': u'jesse@mongodb.com'}

Message:CDRIVER-2270 compatibility logic fix
Branch:r1.8
https://github.com/mongodb/mongo-c-driver/commit/7d761a0fb4d00492e23f30441c3d0cc1c547eae4

Comment by A. Jesse Jiryu Davis [ 12/Sep/17 ]

Ooh nice catch.

Comment by Hannes Magnusson [ 12/Sep/17 ]

I think this is still Open.

We aggressively call _mongoc_topology_description_check_compatible, every time we get a ismaster response in fact.

In replicaset mode, when seeding with just a secondary, we get that result back, iterate over the hosts add them to the topology and mark the primary (as reported by this node) as MONGOC_SERVER_POSSIBLE_PRIMARY.
We then continue to check the compatibility of the topology, before ever actually having recieved ismaster from that node.
Other secondaries we discovered are in the state of MONGOC_SERVER_UNKNOWN so we skip them now, but the likely primary is MONGOC_SERVER_POSSIBLE_PRIMARY so we deem the topology as a whole as incompatible

Comment by A. Jesse Jiryu Davis [ 11/Sep/17 ]

master: https://github.com/mongodb/mongo-c-driver/commit/590cdfe1fa7d5c299db1d1ca25f8c86f7b31bde2

1.8: https://github.com/mongodb/mongo-c-driver/commit/7d761a0fb4d00492e23f30441c3d0cc1c547eae4

Comment by A. Jesse Jiryu Davis [ 11/Sep/17 ]

Oh, I was confused. 1.8.0 works despite the logic bug because 1.8.0 is still compatible with servers that have maxWireVersion 0. Let's fix it in 1.8.0 anyway since that's got the latest update to this compatibility logic. It wouldn't actually affect behavior until we bump WIRE_VERSION_MIN in 1.9.0.

Comment by Hannes Magnusson [ 08/Sep/17 ]

I think so, I haven't dug way to deep into this yet, just applied the above patch to fix this for the mock server tests

Comment by A. Jesse Jiryu Davis [ 08/Sep/17 ]

This needs to be fixed in 1.8, right? Otherwise 1.8 will fail with an "incompatible" error when connected to a replica set with some unavailable members.

Comment by Hannes Magnusson [ 08/Sep/17 ]

It looks like

diff --git a/src/mongoc/mongoc-topology-description.c b/src/mongoc/mongoc-topology-description.c
index 94b260ffe..a87a98835 100644
--- a/src/mongoc/mongoc-topology-description.c
+++ b/src/mongoc/mongoc-topology-description.c
@@ -1704,6 +1704,9 @@ _mongoc_topology_description_check_compatible (
    for (i = 0; i < td->servers->items_len; i++) {
       sd = (mongoc_server_description_t *) mongoc_set_get_item (td->servers,
                                                                 (int) i);
+      if (sd->type == MONGOC_SERVER_UNKNOWN) {
+         continue;
+      }

is sufficient and seems correct.

I'm still going through a ton of mockserver tests though

Generated at Wed Feb 07 21:14:43 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.