[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: |
|
||||||||
| 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: | |||||||||||
| Comment by Ramon Fernandez Marina [ 18/Sep/17 ] | |||||||||||
|
Author: {'username': u'bjori', 'name': u'Hannes Magnusson', 'email': u'bjori@php.net'}Message:Merge branch '
| |||||||||||
| Comment by Githook User [ 18/Sep/17 ] | |||||||||||
|
Author: {'username': 'bjori', 'name': 'Hannes Magnusson', 'email': 'bjori@php.net'}Message: | |||||||||||
| Comment by Githook User [ 18/Sep/17 ] | |||||||||||
|
Author: {'username': 'bjori', 'name': 'Hannes Magnusson', 'email': 'bjori@php.net'}Message: | |||||||||||
| 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: | |||||||||||
| Comment by Ramon Fernandez Marina [ 13/Sep/17 ] | |||||||||||
|
Author: {'username': u'ajdavis', 'name': u'A. Jesse Jiryu Davis', 'email': u'jesse@mongodb.com'}Message: | |||||||||||
| Comment by Ramon Fernandez Marina [ 12/Sep/17 ] | |||||||||||
|
Author: {'username': u'ajdavis', 'name': u'A. Jesse Jiryu Davis', 'email': u'jesse@mongodb.com'}Message: | |||||||||||
| Comment by Ramon Fernandez Marina [ 12/Sep/17 ] | |||||||||||
|
Author: {'username': u'ajdavis', 'name': u'A. Jesse Jiryu Davis', 'email': u'jesse@mongodb.com'}Message: | |||||||||||
| 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. | |||||||||||
| 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
is sufficient and seems correct. I'm still going through a ton of mockserver tests though |