[SERVER-42930] ConnectionPool controller updates must batch across hosts Created: 20/Aug/19  Updated: 29/Oct/23  Resolved: 02/Mar/20

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: 4.2.0
Fix Version/s: 4.2.4, 4.3.4

Type: Bug Priority: Major - P3
Reporter: Benjamin Caimano (Inactive) Assignee: Benjamin Caimano (Inactive)
Resolution: Fixed Votes: 1
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Backports
Depends
Duplicate
is duplicated by SERVER-42871 ConnectionPool::SpecificPool is able ... Closed
Problem/Incident
Related
Backwards Compatibility: Fully Compatible
Operating System: ALL
Backport Requested:
v4.2
Sprint: Service Arch 2019-08-26, Service Arch 2019-09-09, Service Arch 2019-09-23, Service Arch 2019-10-21, Service Arch 2019-11-04, Service Arch 2019-11-18, Service Arch 2019-12-02, Service Arch 2019-12-16, Service Arch 2019-12-30, Service Arch 2020-01-13
Participants:
Linked BF Score: 17

 Description   

Each ConnectionPool::ControllerInterface implementation has its own state. As a consequence of this, its state is only brought in line with each SpecificPool during updateHost(). This can lead to the following race when there are two related pools to HostA and HostB:

  • Pool to HostA is expired initially. It has no outstanding call to updateHost(), i.e. it has already happened.
  • Pool to HostB is now expired. It schedules a call to updateHost(). Both hosts are now expired, thus both can shut down.
  • Pool to HostA is no longer expired. It schedules a call to updateHost(). We should not be able to shutdown anymore, but the controller doesn't know that until updateHost() is called.
  • The updateHost() for the pool to HostB runs. It updates the controller's understanding of HostB and the controller believes both pools can shutdown. Which is wrong, but HostA's updateHost() is queued behind the updateHost() for HostB.

The solution is to run all in flight updateHost() calls before we do anything on the ConnectionPool side. This probably involves moving all controller logic to ConnectionPool instead of SpecificPool. Hopefully, this shouldn't ruin the perf, but in the worst case we can maybe shift the spawnConnection() to be scheduled alone and call updateHost() inline.



 Comments   
Comment by Githook User [ 02/Mar/20 ]

Author:

{'name': 'Ben Caimano', 'email': 'ben.caimano@10gen.com'}

Message: SERVER-42930 Pools for hosts can outlive their groups

Previously, SpecificPools always were shutdown if their Controller told
them they could shutdown. This commit changes the logic to allow
SpecificPools to reject their Controller's suggestions if the
SpecificPool no longer thinks it is "expired".

(cherry picked from commit 2d8f351c2640f402027a2fc0c51628db8d4f85b1)
Branch: v4.2
https://github.com/mongodb/mongo/commit/15d042e5ce044f136c6d2c6a79d09b23ae838d73

Comment by Benjamin Caimano (Inactive) [ 02/Mar/20 ]

I've landed a patch that should ameliorate the invariant failures and make the behavior slightly more reasonable. This isn't a perfect fix, we'll still do a bit more work than we need to in the connection pooling. That said, there were odd side effects when I attempted to linearize our pooling control patterns.

Comment by Githook User [ 28/Feb/20 ]

Author:

{'name': 'Ben Caimano', 'email': 'ben.caimano@10gen.com'}

Message: SERVER-42930 Pools for hosts can outlive their groups

Previously, SpecificPools always were shutdown if their Controller told
them they could shutdown. This commit changes the logic to allow
SpecificPools to reject their Controller's suggestions if the
SpecificPool no longer thinks it is "expired".
Branch: master
https://github.com/mongodb/mongo/commit/2d8f351c2640f402027a2fc0c51628db8d4f85b1

Comment by Githook User [ 23/Sep/19 ]

Author:

{'username': 'pvselvan', 'email': 'pavithra.vetriselvan@mongodb.com', 'name': 'Pavithra Vetriselvan'}

Message: Revert "SERVER-42790 SERVER-42930 ConnectionPool controller updates must batch across hosts"

This reverts commit e9f5360ba5fba5598e2556816a9d7d818ab586c7.
Branch: v4.2
https://github.com/mongodb/mongo/commit/28a50ae351071acdf18745c0f3bb54c84485d3ba

Comment by Githook User [ 23/Sep/19 ]

Author:

{'username': 'pvselvan', 'email': 'pavithra.vetriselvan@mongodb.com', 'name': 'Pavithra Vetriselvan'}

Message: Revert "SERVER-42930 ConnectionPool controller updates must batch across hosts"

This reverts commit 872a46afa66260b8ea59772aa20acb374939eef5.
Branch: master
https://github.com/mongodb/mongo/commit/d1a457ba34733bec62a201d0d218677494f91b33

Comment by Githook User [ 19/Sep/19 ]

Author:

{'name': 'Ben Caimano', 'username': 'bcaimano', 'email': 'ben.caimano@mongodb.com'}

Message: SERVER-42790 SERVER-42930 ConnectionPool controller updates must batch across hosts
Branch: v4.2
https://github.com/mongodb/mongo/commit/e9f5360ba5fba5598e2556816a9d7d818ab586c7

Comment by Githook User [ 19/Sep/19 ]

Author:

{'username': 'bcaimano', 'email': 'ben.caimano@mongodb.com', 'name': 'Ben Caimano'}

Message: SERVER-42930 ConnectionPool controller updates must batch across hosts
Branch: master
https://github.com/mongodb/mongo/commit/872a46afa66260b8ea59772aa20acb374939eef5

Generated at Thu Feb 08 05:01:48 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.