Merge ~cgrabowski/maas:handle_all_connections_busy_in_getRegionClient into maas:master

Proposed by Christian Grabowski
Status: Merged
Approved by: Christian Grabowski
Approved revision: e7653e53a746f2fd8b807e5a0096508840f33e9b
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~cgrabowski/maas:handle_all_connections_busy_in_getRegionClient
Merge into: maas:master
Diff against target: 70 lines (+28/-5)
3 files modified
src/provisioningserver/rpc/__init__.py (+2/-1)
src/provisioningserver/rpc/clusterservice.py (+8/-4)
src/provisioningserver/rpc/tests/test_clusterservice.py (+18/-0)
Reviewer Review Type Date Requested Status
MAAS Lander Approve
Alberto Donato (community) Approve
Adam Collard (community) Approve
Review via email: mp+428920@code.launchpad.net

Commit message

allow getRegionClient to use a busy connection

To post a comment you must log in.
Revision history for this message
Adam Collard (adam-collard) :
review: Approve
Revision history for this message
Alberto Donato (ack) wrote :

LGTM, +1

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b handle_all_connections_busy_in_getRegionClient lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: e7653e53a746f2fd8b807e5a0096508840f33e9b

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/src/provisioningserver/rpc/__init__.py b/src/provisioningserver/rpc/__init__.py
index d5a36ed..0388356 100644
--- a/src/provisioningserver/rpc/__init__.py
+++ b/src/provisioningserver/rpc/__init__.py
@@ -25,4 +25,5 @@ def getRegionClient():
25 "Cluster services are unavailable."25 "Cluster services are unavailable."
26 )26 )
27 else:27 else:
28 return rpc_service.getClient()28 # won't scale connections if existing connections are busy, but will always return connection if one exists
29 return rpc_service.getClient(busy_ok=True)
diff --git a/src/provisioningserver/rpc/clusterservice.py b/src/provisioningserver/rpc/clusterservice.py
index a7205db..d9baf8d 100644
--- a/src/provisioningserver/rpc/clusterservice.py
+++ b/src/provisioningserver/rpc/clusterservice.py
@@ -1230,7 +1230,7 @@ class ClusterClientService(TimerService):
1230 self.time_started = self.clock.seconds()1230 self.time_started = self.clock.seconds()
1231 super().startService()1231 super().startService()
12321232
1233 def getClient(self):1233 def getClient(self, busy_ok=False):
1234 """Returns a :class:`common.Client` connected to a region.1234 """Returns a :class:`common.Client` connected to a region.
12351235
1236 The client is chosen at random.1236 The client is chosen at random.
@@ -1246,9 +1246,13 @@ class ClusterClientService(TimerService):
1246 self.connections.get_random_free_connection()1246 self.connections.get_random_free_connection()
1247 )1247 )
1248 except exceptions.AllConnectionsBusy as e:1248 except exceptions.AllConnectionsBusy as e:
1249 for endpoint_conns in self.connections.values():1249 if not busy_ok:
1250 if len(endpoint_conns) < self.connections._max_connections:1250 for endpoint_conns in self.connections.values():
1251 raise e1251 if (
1252 len(endpoint_conns)
1253 < self.connections._max_connections
1254 ):
1255 raise e
1252 # return a busy connection, assume it will free up or timeout1256 # return a busy connection, assume it will free up or timeout
1253 return common.Client(self.connections.get_random_connection())1257 return common.Client(self.connections.get_random_connection())
12541258
diff --git a/src/provisioningserver/rpc/tests/test_clusterservice.py b/src/provisioningserver/rpc/tests/test_clusterservice.py
index 6f3e4f9..04b7d66 100644
--- a/src/provisioningserver/rpc/tests/test_clusterservice.py
+++ b/src/provisioningserver/rpc/tests/test_clusterservice.py
@@ -1300,6 +1300,24 @@ class TestClusterClientService(MAASTestCase):
1300 service.connections.connections = {}1300 service.connections.connections = {}
1301 self.assertRaises(exceptions.NoConnectionsAvailable, service.getClient)1301 self.assertRaises(exceptions.NoConnectionsAvailable, service.getClient)
13021302
1303 def test_getClient_returns_a_busy_connection_when_busy_ok_is_True(self):
1304 service = ClusterClientService(Clock(), max_conns=1)
1305 service.connections.connections = {
1306 sentinel.eventloop01: [FakeBusyConnectionToRegion()],
1307 sentinel.eventloop02: [FakeBusyConnectionToRegion()],
1308 sentinel.eventloop03: [FakeBusyConnectionToRegion()],
1309 }
1310 client = service.getClient(busy_ok=True)
1311 self.assertTrue(client._conn.in_use)
1312 self.assertIn(
1313 client,
1314 {
1315 common.Client(conn)
1316 for conns in service.connections.values()
1317 for conn in conns
1318 },
1319 )
1320
1303 @inlineCallbacks1321 @inlineCallbacks
1304 def test_getClientNow_scales_connections_when_busy(self):1322 def test_getClientNow_scales_connections_when_busy(self):
1305 service = ClusterClientService(Clock(), max_conns=2)1323 service = ClusterClientService(Clock(), max_conns=2)

Subscribers

People subscribed via source and target branches