[SERVER-57521] FCV change unconditionally closes outgoing connections that have not finished their hello handshake Created: 08/Jun/21 Updated: 29/Oct/23 Resolved: 29/Jun/21 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 5.0.2, 5.1.0-rc0 |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Jordi Serra Torrens | Assignee: | Janna Golden |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||||||
| Issue Links: |
|
||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||
| Backport Requested: |
v5.0
|
||||||||||||||||||||
| Sprint: | Sharding 2021-06-28 | ||||||||||||||||||||
| Participants: | |||||||||||||||||||||
| Linked BF Score: | 50 | ||||||||||||||||||||
| Description |
|
Upon changing the FCV value, outgoing connections to servers with a lower binary version are closed. This is done by closing all the connections except those tagged with transport::Session::kKeepOpen. ConnectionPool::dropConnections is not considering the kPending flag. |
| Comments |
| Comment by Vivian Ge (Inactive) [ 06/Oct/21 ] |
|
Updating the fixversion since branching activities occurred yesterday. This ticket will be in rc0 when it’s been triggered. For more active release information, please keep an eye on #server-release. Thank you! |
| Comment by Githook User [ 21/Jul/21 ] |
|
Author: {'name': 'jannaerin', 'email': 'golden.janna@gmail.com', 'username': 'jannaerin'}Message: (cherry picked from commit 39505432404a35ebd35e05dffec0ad29d7f79262) |
| Comment by Githook User [ 29/Jun/21 ] |
|
Author: {'name': 'jannaerin', 'email': 'golden.janna@gmail.com', 'username': 'jannaerin'}Message: |
| Comment by Jordi Serra Torrens [ 11/Jun/21 ] |
|
AsyncDBClient::_parseIsMasterResponse does check the wire version prior to setting the kKeepOpen flag, but this can race. In fact, I think a race condition already exists where: So either this has to be safe, or I think we already have a bug there. Someone from repl or service arch may be more familiar with this code to confirm this. |
| Comment by Matthew Saltz (Inactive) [ 10/Jun/21 ] |
|
Is it a problem if we don’t drop a pending connection and then it turns out to be a connection that’s not marked “kKeepOpen” and is also to an older binary version? |
| Comment by Janna Golden [ 10/Jun/21 ] |
|
I have a repro of this issue that I've attached to this ticket. Adding a check for `kPending` in ConnectionPool::dropConnections fixes the issue. I can take this ticket and put up a CR if we agree that adding a check for `kPending` in `ConnectionPool:dropConnections` is the right fix. |