Skip to content

Commit

Permalink
fix regression using SINGLE_NODE
Browse files Browse the repository at this point in the history
  • Loading branch information
julien6387 committed May 19, 2024
1 parent 56f0103 commit 8a52296
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 20 deletions.
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## 0.19 (2024-xx-xx)

* Fix regression using the `SINGLE_NODE` distribution rule.

* Add disk usage and read/write statistics to the **Supvisors** Web UI.
Disk statistics are published to the event interface and the JAVA client has been updated accordingly.

Expand Down
3 changes: 2 additions & 1 deletion supvisors/commander.py
Original file line number Diff line number Diff line change
Expand Up @@ -743,8 +743,9 @@ def distribute_to_single_node(self) -> None:
# find the applicable Supvisors instances iaw strategy
application_load = self.application.get_start_sequence_expected_load()
identifiers = self.application.possible_node_identifiers()
load_request_map = self.get_load_requests()
# choose the node that can support the application load
node_name = get_node(self.supvisors, self.starting_strategy, identifiers, application_load)
node_name = get_node(self.supvisors, self.starting_strategy, identifiers, application_load, load_request_map)
# intersect the identifiers running on the node and the application possible identifiers
# comprehension based on iteration over application possible identifiers to keep the CONFIG order
node_identifiers = list(self.supvisors.mapper.nodes.get(node_name, []))
Expand Down
33 changes: 18 additions & 15 deletions supvisors/strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,11 +294,12 @@ def get_supvisors_instance(supvisors: Any, strategy: StartingStrategies, identif
expected_load: int, load_request_map: LoadMap) -> Optional[str]:
""" Creates a strategy and let it find a Supvisors instance to start a process having a defined load.
:param supvisors: the global Supvisors structure
:param strategy: the strategy used to choose a Supvisors instance
:param identifiers: the identifiers of the candidate Supvisors instances (from configuration perspective)
:param expected_load: the load of the program to be started
:return: the identifier of the Supvisors instance that can support the additional load
:param supvisors: the global Supvisors structure.
:param strategy: the strategy used to choose a Supvisors instance.
:param identifiers: the identifiers of the candidate Supvisors instances (from configuration perspective).
:param expected_load: the load of the program to be started.
:param load_request_map: the load of the requests in progress.
:return: the identifier of the Supvisors instance that can support the additional load.
"""
# restrict the candidate Supvisors instances to those that are actually running
running_identifiers = supvisors.context.running_identifiers()
Expand All @@ -318,19 +319,21 @@ def get_supvisors_instance(supvisors: Any, strategy: StartingStrategies, identif
(load_request_map, node_load_request_map, node_load_map))


def get_node(supvisors: Any, strategy: StartingStrategies, identifiers: NameList, expected_load: int) -> Optional[str]:
def get_node(supvisors: Any, strategy: StartingStrategies, identifiers: NameList,
expected_load: int, load_request_map: LoadMap) -> Optional[str]:
""" Creates a strategy and let it find the node to start a process having a defined load.
:param supvisors: the global Supvisors structure
:param strategy: the strategy used to choose a Supvisors instance
:param identifiers: the identifiers of the candidate Supvisors instances (from configuration perspective)
:param expected_load: the load of the program to be started
:return: the IP address of the node that can support the additional load
:param supvisors: the global Supvisors structure.
:param strategy: the strategy used to choose a Supvisors instance.
:param identifiers: the identifiers of the candidate Supvisors instances (from configuration perspective).
:param expected_load: the load of the program to be started.
:param load_request_map: the load of the requests in progress.
:return: the IP address of the node that can support the additional load.
"""
# Note: the node load is the sum of the load of its instances
# the selection of an instance is conditioned to the fact that the node can support the additional load
# if the node can support, there must be one of its instance that can support
identifier = get_supvisors_instance(supvisors, strategy, identifiers, expected_load)
# Note: The node load is the sum of the load of its instances.
# The selection of an instance is conditioned to the fact that the node can support the additional load.
# If the node can support, there must be one of its instances that can support too.
identifier = get_supvisors_instance(supvisors, strategy, identifiers, expected_load, load_request_map)
# get the corresponding IP address
if identifier:
return supvisors.mapper.instances[identifier].ip_address
Expand Down
6 changes: 4 additions & 2 deletions supvisors/tests/test_commander.py
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,8 @@ def test_application_start_job_distribute_to_single_node(mocker, supvisors, appl
mocked_get_node.return_value = None
application_start_job_1.distribute_to_single_node()
assert application_start_job_1.identifiers == []
assert mocked_get_node.call_args_list == [call(supvisors, StartingStrategies.LESS_LOADED, possible_identifiers, 27)]
assert mocked_get_node.call_args_list == [call(supvisors, StartingStrategies.LESS_LOADED,
possible_identifiers, 27, {})]
assert not mocked_get_instance.called
# check commands
assert all(command.identifier is None
Expand All @@ -782,7 +783,8 @@ def test_application_start_job_distribute_to_single_node(mocker, supvisors, appl
application_start_job_1.distribute_to_single_node()
expected_identifiers = [supvisors.mapper.local_identifier, test_identifier]
assert application_start_job_1.identifiers == expected_identifiers
assert mocked_get_node.call_args_list == [call(supvisors, StartingStrategies.LESS_LOADED, possible_identifiers, 27)]
assert mocked_get_node.call_args_list == [call(supvisors, StartingStrategies.LESS_LOADED,
possible_identifiers, 27, {})]
expected = [call(supvisors, StartingStrategies.LESS_LOADED, expected_identifiers, 10, {}),
call(supvisors, StartingStrategies.LESS_LOADED, expected_identifiers, 20, {}),
call(supvisors, StartingStrategies.LESS_LOADED, expected_identifiers, 30, {})]
Expand Down
4 changes: 2 additions & 2 deletions supvisors/tests/test_strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,12 @@ def test_get_node(mocker, filled_instances):
instances = [local_identifier, '10.0.0.3:25000', '10.0.0.5:25000', test_identifier]
# test with no instance found
strategy = StartingStrategies.CONFIG
assert get_node(filled_instances, strategy, instances, 27) is None
assert get_node(filled_instances, strategy, instances, 27, {}) is None
# test with no instance found
mocked_get_instance.return_value = test_identifier
strategy = StartingStrategies.CONFIG
local_instance = filled_instances.mapper.instances[local_identifier]
assert get_node(filled_instances, strategy, instances, 27) == local_instance.host_id
assert get_node(filled_instances, strategy, instances, 27, {}) == local_instance.host_id


def create_process_status(name, timed_identifiers):
Expand Down

0 comments on commit 8a52296

Please sign in to comment.