Repository: tarantool/ansible-cartridge
Issue category: Performance Issue

[2pt] Too long "Install package" task
I have 100 Tarantool instances in a cluster. When I deploy them via ansible-cartridge, it takes a very long time to complete the "Install package" task, since it is performed for each instance. Moreover, this task is performed even despite the fact that it is written in the logs that task is skipped for all instances.

Let's define the host list once instead of a hundred times.
[2pt] Too long "Install package" task
I have 100 Tarantool instances in a cluster. When I deploy them via ansible-cartridge, it takes a very long time to complete the "Install package" task, since it is performed for each instance. Moreover, this task is performed even despite the fact that it is written in the logs that task is skipped for all instances.

Let's define the host list once instead of a hundred times.


Functions to edit:
filter_plugins/filter_hosts.py:get_machine_identifier
filter_plugins/filter_hosts.py:get_one_not_expelled_instance_for_machine
Functions added: filter_plugins/filter_hosts.py:get_machine_hostname

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 978b7e7a..a6396c0b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,6 +26,9 @@ to use the newest tag with new release
   to expel. It can be called second time to set up failover priority for
   replicasets where new instances were joined.
   As a result, `replicaset_healthy_timeout` is removed as unused.
+- Now list of instances for installing a package is selected once for all.
+  Before this patch, the complexity of calculating the list of instances was O(N^2),
+  now it is O(N). For 100 instances, it gives a 10x time reduction (60s -> 5s).
 
 ## [1.7.0] - 2020-11-24
 
diff --git a/filter_plugins/filter_hosts.py b/filter_plugins/filter_hosts.py
index aa103a74..099c41b6 100644
--- a/filter_plugins/filter_hosts.py
+++ b/filter_plugins/filter_hosts.py
@@ -1,5 +1,8 @@
-def get_machine_identifier(v):
-    return v['ansible_host'] if 'ansible_host' in v else v['inventory_hostname']
+def get_machine_hostname(v):
+    if 'ansible_host' in v:
+        return v['ansible_host']
+
+    raise Exception(f'Instance {v["inventory_hostname"]} has not "ansible_host" option!')
 
 
 def is_expelled(instance_vars):
@@ -18,19 +21,19 @@ def get_instance_fullname(app_name, inventory_hostname, stateboard):
 
 
 def get_one_not_expelled_instance_for_machine(hostvars, play_hosts):
-    res = []
-    added_machines = {}
+    machine_hostnames = set()
+    instance_names = []
 
-    for i in play_hosts:
-        if is_expelled(hostvars[i]):
+    for instance_name in play_hosts:
+        if is_expelled(hostvars[instance_name]):
             continue
 
-        machine_id = get_machine_identifier(hostvars[i])
-        if machine_id not in added_machines:
-            added_machines[machine_id] = True
-            res.append(i)
+        machine_hostname = get_machine_hostname(hostvars[instance_name])
+        if machine_hostname not in machine_hostnames:
+            machine_hostnames.add(machine_hostname)
+            instance_names.append(instance_name)
 
-    return res
+    return instance_names
 
 
 def get_one_not_expelled_instance(hostvars, play_hosts):
diff --git a/tasks/install_deb.yml b/tasks/install_deb.yml
index c33b5d59..144e87e8 100644
--- a/tasks/install_deb.yml
+++ b/tasks/install_deb.yml
@@ -52,7 +52,7 @@
     version: '{{ tnt_version[0] }}'
   when: '"tarantool" in deplist and cartridge_enable_tarantool_repo'
 
-- name: Install DEB
+- name: Install DEB package
   apt:
     deb: /tmp/{{ deb_filename }}
     update_cache: true
diff --git a/tasks/install_rpm.yml b/tasks/install_rpm.yml
index 9237094d..817e093c 100644
--- a/tasks/install_rpm.yml
+++ b/tasks/install_rpm.yml
@@ -57,7 +57,7 @@
     version: '{{ tnt_version[0] }}'
   when: 'cartridge_enable_tarantool_repo and "tarantool" in deplist.stdout'
 
-- name: Install RPM
+- name: Install RPM package
   yum:
     name: '/tmp/{{ rpm_filename }}'
     state: present
diff --git a/tasks/main.yml b/tasks/main.yml
index 0320c36d..f9fb11c6 100644
--- a/tasks/main.yml
+++ b/tasks/main.yml
@@ -50,12 +50,18 @@
     - cartridge-replicasets
     - cartridge-config
 
+- name: Get one instance from each physical machine
+  set_fact:
+    single_instances_for_each_machine: '{{ hostvars | get_one_not_expelled_instance_for_machine(play_hosts) }}'
+  run_once: true
+  tags: cartridge-instances
+
 # Rewrites cartridge_app_name from package info
 - name: Install package
   include_tasks: install_package.yml
   when: >-
     cartridge_package_path is defined and
-    inventory_hostname in hostvars | get_one_not_expelled_instance_for_machine(play_hosts)
+    inventory_hostname in single_instances_for_each_machine
   tags: cartridge-instances
 
 - name: Start instance

Test patch: diff --git a/unit/test_tags.py b/unit/test_tags.py
index 01e1f68e..74281892 100644
--- a/unit/test_tags.py
+++ b/unit/test_tags.py
@@ -48,6 +48,7 @@ def test_without_tags(self):
             'Set remote_user for delegated tasks',
             'Validate config',
             'Set instance facts',
+            'Get one instance from each physical machine',
             'Install package',
             'Start instance',
             'Restart instances and reload systemd-daemon',
@@ -74,6 +75,7 @@ def test_tag_cartridge_instances(self):
             'Set remote_user for delegated tasks',
             'Validate config',
             'Set instance facts',
+            'Get one instance from each physical machine',
             'Install package',
             'Start instance',
             'Restart instances and reload systemd-daemon',