[SERVER-45039] Awaitable isMaster returns an error on reconfigs that change the SplitHorizon Created: 10/Dec/19 Updated: 08/Jan/24 Resolved: 10/Feb/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Replication |
| Affects Version/s: | 4.3 Required |
| Fix Version/s: | 4.3.4 |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Jason Chan | Assignee: | Jason Chan |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Backwards Compatibility: | Fully Compatible |
| Sprint: | Repl 2020-01-27, Repl 2020-02-10 |
| Participants: |
| Description |
|
Consider the scenario: We will respond with an error to all waiting isMasters for reconfigs that change the horizon. This terminates the exhaust stream, when exhaust is used. |
| Comments |
| Comment by Githook User [ 07/Feb/20 ] |
|
Author: {'name': 'Jason Chan', 'username': 'jasonjhchan', 'email': 'jason.chan@mongodb.com'}Message: |
| Comment by Tess Avitabile (Inactive) [ 28/Jan/20 ] |
|
I believe the next isMaster will fail with a network error. |
| Comment by Shane Harvey [ 27/Jan/20 ] |
I'm trying to decide if it would make more sense for the client to simply close the connection after isMaster fails. Is the assumption that the next isMaster on that same connection will fail with a command error or fail with a network error? |
| Comment by Tess Avitabile (Inactive) [ 15/Jan/20 ] |
|
Per in-person discussion, since the server will respond with an error message, which terminates the exhaust stream, the client would need to send a new isMaster request, at which point they would see the horizon is no longer valid. So it's not essential to close the connection. Any driver that is behavior correctly should eventually stop using the connection. mira.carey@mongodb.com will follow up about whether it's desirable in general to close connections over a particular horizon when that horizon becomes invalid. |
| Comment by Mira Carey [ 15/Jan/20 ] |
|
Per whether to close sockets on reconfig that changes horizon: My thought had been that in a non-exhaust mode, a reconfig that changed horizon such that you as a client could no longer reach the replica set would mean that you would no longer receive events for that host (because there would be no way to do a name lookup based on an ismaster response that would give you a valid socket). With exhaust mode, you could receive that message and would have to yourself realize that the socket you have in hand isn't valid to communicate over. Rather than put the onus on the driver to figure that out, erring on the side of closing monitoring sockets felt like the least mental overhead design-wise. In terms of why this would be relevant for this project (and not homed in the split horizon project):
If you'd rather tackle this by closing sockets when we perform horizon altering reconfigs, I can definitely see that angle. But either way I think I'd lean towards closing the socket (just to make it easier to reason about possible state transitions) |
| Comment by Tess Avitabile (Inactive) [ 14/Jan/20 ] |
|
mira.carey@mongodb.com, I just wanted to remind you about the above question. |
| Comment by Tess Avitabile (Inactive) [ 09/Jan/20 ] |
|
mira.carey@mongodb.com, I have a design question for you. We are planning to return an error on all waiting isMaster requests when a reconfig changes the horizon mapping. This will terminate exhaust streams for exhaust isMaster requests. We are wondering whether we also need to close the connection. jason.chan described that if the client connected over a horizon that has been removed, then it may be unsafe to allow the connection to remain open. However, this seems unrelated to awaitable/exhaust isMaster and it should have been part of the original Split Horizons project. We would also want to close the connection regardless of whether it has a currently running isMaster request. Do you know if it's a requirement that we close connections over removed horizons? If so, was this covered by the Split Horizons project? If not, I would like to handle this as a bug separate from this project. |
| Comment by Tess Avitabile (Inactive) [ 17/Dec/19 ] |
|
As part of this work, we will add tests for streamable isMaster with split horizon:
|