Increasing batch size from 3 to 6 to improve performance of batch pat…

…ching (#227)

* Increasing batch size from 3 to 6

* Updated unit tests for increasing batch size from 3 to 6

* Moved NUMBER_OF_PACKAGES_IN_BATCH_COULD_TAKE_MAX_TIME_TO_INSTALL to Constants file

* Add inline comments for PACKAGE_INSTALL_EXPECTED_AVG_TIME_IN_MINUTES

src/core/src/bootstrap/Constants.py

@@ -208,7 +208,8 @@ class AutoAssessmentStates(EnumBackport):
     MAX_INSTALLATION_RETRY_COUNT = 3
     MAX_IMDS_CONNECTION_RETRY_COUNT = 5
     MAX_ZYPPER_REPO_REFRESH_RETRY_COUNT = 5
-    MAX_BATCH_SIZE_FOR_PACKAGES = 3
+    MAX_BATCH_SIZE_FOR_PACKAGES = 6
+    NUMBER_OF_PACKAGES_IN_BATCH_COULD_TAKE_MAX_TIME_TO_INSTALL = 3
     MAX_COMPLETE_STATUS_FILES_TO_RETAIN = 10
 
     class PackageClassification(EnumBackport):
@@ -257,6 +258,19 @@ class PatchAssessmentSummaryStartedBy(EnumBackport):
 
     # Maintenance Window
     PACKAGE_INSTALL_EXPECTED_MAX_TIME_IN_MINUTES = 5
+
+    # As per telemetry data, when batch size is 3, the average time taken per package installation for different package managers is as follow:
+    # apt: 43 seconds
+    # yum: 71 seconds
+    # zypper: 142 seconds
+    # Overall, including all package managers, the average time is 51 seconds.
+    # The average time taken per package installation is average of (total time taken to install the packages) / (total number of packages installed).
+    # The expected average time should be kept as max of average time taken by different packages managers. The max of average time is taken in zypper i.e. 142 seconds when batch size is 3.
+    # But as the batch size increases, the time taken to install package will decrease. Also, the average time is taken in consideration in calculating maintenance window cutoff only if the 
+    # batch size is greater than or equal to 4. So, keeping the expected average time as 2 minutes i.e. 120 seconds. It should be fine to keep expected time little lower than actual average time 
+    # observed in telemetry because there is PACKAGE_INSTALL_EXPECTED_MAX_TIME_IN_MINUTES also in the calucalation of maintenance window cut off which will make the overall cutoff time enough to 
+    # install the batch of packages.
+    PACKAGE_INSTALL_EXPECTED_AVG_TIME_IN_MINUTES = 2
 
     # Package Manager Setting
     PACKAGE_MGR_SETTING_REPEAT_PATCH_OPERATION = "RepeatUpdateRun"

src/core/src/core_logic/MaintenanceWindow.py

@@ -61,7 +61,30 @@ def get_remaining_time_in_minutes(self, current_time=None, log_to_stdout=False):
 
     def is_package_install_time_available(self, remaining_time_in_minutes=None, number_of_packages_in_batch=1):
         """Check if time still available for package installation"""
-        cutoff_time_in_minutes = Constants.PACKAGE_INSTALL_EXPECTED_MAX_TIME_IN_MINUTES * number_of_packages_in_batch
+        cutoff_time_in_minutes = 0
+
+        # In the extreme case, all the package installations in the batch might take the maximum time. 
+        # But calculating cutoff time based on max time to install packages for all the packages will make cutoff time very huge 
+        # as it is very unlikely that all the package installations take maximum time.
+        # Also, as the batch size increases, the expected time taken per package installation decreases due to batch patching.
+        # So, for batch size less than or equal to 3, calculating cutoff time expecting all packages might take max time to install.
+        # For larger batch size i.e. 4 or higher, expecting 3 package installations can take max time and rest can take average time to install.
+        # It is safe assumption that only 3 packages will take max time to install. Even if more number of packages take max time to install then
+        # due to batch patching in large batch size, the time taken to install a package decreases substantially and hence cutoff time should be enough
+        # to install the batch of packages.
+        # PACKAGE_INSTALL_EXPECTED_MAX_TIME is 5 minutes, PACKAGE_INSTALL_EXPECTED_AVG_TIME is 2 minutes
+        # For different batch size, following would be cutoff:
+        # Batch Size = 1, Cutoff = 5 * 1 = 5 minutes
+        # Batch Size = 2, Cutoff = 5 * 2 = 10 minutes
+        # Batch Size = 3, Cutoff = 5 * 3 = 15 minutes
+        # Batch Size = 4, Cutoff = (5 * 3) + (2 * 1) = 17 minutes
+        # Batch Size = 5, Cutoff = (5 * 3) + (2 * 2) = 19 minutes
+        # Batch Size = 6, Cutoff = (5 * 3) + (2 * 3) = 21 minutes
+        if number_of_packages_in_batch <= Constants.NUMBER_OF_PACKAGES_IN_BATCH_COULD_TAKE_MAX_TIME_TO_INSTALL:
+            cutoff_time_in_minutes = Constants.PACKAGE_INSTALL_EXPECTED_MAX_TIME_IN_MINUTES * number_of_packages_in_batch
+        else:
+            cutoff_time_in_minutes = Constants.PACKAGE_INSTALL_EXPECTED_MAX_TIME_IN_MINUTES * Constants.NUMBER_OF_PACKAGES_IN_BATCH_COULD_TAKE_MAX_TIME_TO_INSTALL
+            cutoff_time_in_minutes += Constants.PACKAGE_INSTALL_EXPECTED_AVG_TIME_IN_MINUTES * (number_of_packages_in_batch - Constants.NUMBER_OF_PACKAGES_IN_BATCH_COULD_TAKE_MAX_TIME_TO_INSTALL)
 
         if Constants.REBOOT_SETTINGS[self.execution_config.reboot_setting] != Constants.REBOOT_NEVER:
             cutoff_time_in_minutes = cutoff_time_in_minutes + Constants.REBOOT_BUFFER_IN_MINUTES

src/core/tests/Test_PatchInstaller.py

@@ -337,12 +337,15 @@ def test_dependency_installed_successfully(self):
         runtime.set_legacy_test_type('DependencyInstallSuccessfully')
         # As all the packages should get installed using batch patching, get_remaining_packages_to_install should return 0 packages
         installed_update_count, update_run_successful, maintenance_window_exceeded = runtime.patch_installer.install_updates(runtime.maintenance_window, runtime.package_manager, simulate=True)
-        self.assertEqual(4, installed_update_count)
+        self.assertEqual(7, installed_update_count)
         self.assertTrue(update_run_successful)
         self.assertFalse(maintenance_window_exceeded)
         runtime.stop()
 
     def test_dependency_install_failed(self):
+        # exclusion list contains grub-efi-amd64-bin
+        # grub-efi-amd64-signed is dependent on grub-efi-amd64-bin, so grub-efi-amd64-signed should also get excluded
+        # so, out of 7 packages, only 5 packages are installed and 2 are excluded
         current_time = datetime.datetime.utcnow()
         td = datetime.timedelta(hours=0, minutes=20)
         job_start_time = (current_time - td).strftime("%Y-%m-%dT%H:%M:%S.9999Z")
@@ -353,13 +356,13 @@ def test_dependency_install_failed(self):
         # Path change
         runtime.set_legacy_test_type('DependencyInstallFailed')
         installed_update_count, update_run_successful, maintenance_window_exceeded = runtime.patch_installer.install_updates(runtime.maintenance_window, runtime.package_manager, simulate=True)
-        self.assertEqual(2, installed_update_count)
+        self.assertEqual(5, installed_update_count)
         self.assertFalse(update_run_successful)
         self.assertFalse(maintenance_window_exceeded)
         runtime.stop()
 
     def test_not_enough_time_for_batch_patching_dependency_installed_successfully(self):
-        # total packages to install is 3, reboot_setting is 'Never', so cutoff time for batch = 3*5 = 15
+        # total packages to install is 7, reboot_setting is 'Never', cutoff time for batch of 6 packages = (5*3) + (2*3) = 21
         # window size is 60 minutes, let time remain = 14 minutes so that not enough time to install in batch
         # So td = 60-14 = 46
         current_time = datetime.datetime.utcnow()
@@ -373,13 +376,13 @@ def test_not_enough_time_for_batch_patching_dependency_installed_successfully(se
         # Path change
         runtime.set_legacy_test_type('DependencyInstallSuccessfully')
         installed_update_count, update_run_successful, maintenance_window_exceeded = runtime.patch_installer.install_updates(runtime.maintenance_window, runtime.package_manager, simulate=True)
-        self.assertEqual(4, installed_update_count)
+        self.assertEqual(7, installed_update_count)
         self.assertTrue(update_run_successful)
         self.assertFalse(maintenance_window_exceeded)
         runtime.stop()
 
     def test_not_enough_time_for_batch_patching_dependency_install_failed(self):
-        # total packages to install is 3, reboot_setting is 'Never', so cutoff time for batch = 3*5 = 15
+        # total packages to install is 7, reboot_setting is 'Never', so cutoff time for batch = (5*3) + (2*3) = 21
         # window size is 60 minutes, let time remain = 14 minutes so that not enough time to install in batch
         # So td = 60-14 = 46
         current_time = datetime.datetime.utcnow()
@@ -393,7 +396,7 @@ def test_not_enough_time_for_batch_patching_dependency_install_failed(self):
         # Path change
         runtime.set_legacy_test_type('DependencyInstallFailed')
         installed_update_count, update_run_successful, maintenance_window_exceeded = runtime.patch_installer.install_updates(runtime.maintenance_window, runtime.package_manager, simulate=True)
-        self.assertEqual(2, installed_update_count)
+        self.assertEqual(5, installed_update_count)
         self.assertFalse(update_run_successful)
         self.assertFalse(maintenance_window_exceeded)
         runtime.stop()
@@ -496,7 +499,7 @@ def test_skip_package_version_UA_ESM_REQUIRED(self):
     def test_dependent_package_excluded(self):
         # exclusion list contains grub-efi-amd64-bin
         # grub-efi-amd64-signed is dependent on grub-efi-amd64-bin, so grub-efi-amd64-signed should also get excluded
-        # so, out of 4 packages, only 2 packages are installed and 2 are excluded
+        # so, out of 7 packages, only 5 packages are installed and 2 are excluded
         current_time = datetime.datetime.utcnow()
         td = datetime.timedelta(hours=0, minutes=20)
         job_start_time = (current_time - td).strftime("%Y-%m-%dT%H:%M:%S.9999Z")
@@ -509,20 +512,20 @@ def test_dependent_package_excluded(self):
         runtime.set_legacy_test_type('DependencyInstallSuccessfully')
         # As all the packages should get installed using batch patching, get_remaining_packages_to_install should return 0 packages
         installed_update_count, update_run_successful, maintenance_window_exceeded = runtime.patch_installer.install_updates(runtime.maintenance_window, runtime.package_manager, simulate=True)
-        self.assertEqual(2, installed_update_count)
+        self.assertEqual(5, installed_update_count)
         self.assertTrue(update_run_successful)
         self.assertFalse(maintenance_window_exceeded)
         runtime.stop()
 
     def test_dependent_package_excluded_and_not_enough_time_for_batch_patching(self):
         # exclusion list contains grub-efi-amd64-bin
         # grub-efi-amd64-signed is dependent on grub-efi-amd64-bin, so grub-efi-amd64-signed should also get excluded
-        # so, out of 4 packages, only 2 packages are installed and 2 are excluded.
-        # total packages to install is 2, reboot_setting is 'Never', so cutoff time for batch = 2*5 = 10
-        # window size is 60 minutes, let time remain = 9 minutes so that not enough time to install in batch
-        # So td = 60-9 = 51
+        # so, out of 7 packages, only 5 packages are installed and 2 are excluded.
+        # total packages to install is 7, reboot_setting is 'Never', so cutoff time for batch of 6 packages= (5*3) + (2*3) = 21
+        # window size is 60 minutes, let time remain = 16 minutes so that not enough time to install in batch
+        # So td = 60-16 = 44
         current_time = datetime.datetime.utcnow()
-        td = datetime.timedelta(hours=0, minutes=51)
+        td = datetime.timedelta(hours=0, minutes=44)
         job_start_time = (current_time - td).strftime("%Y-%m-%dT%H:%M:%S.9999Z")
         argument_composer = ArgumentComposer()
         argument_composer.patches_to_exclude = ["grub-efi-amd64-bin"]
@@ -532,7 +535,7 @@ def test_dependent_package_excluded_and_not_enough_time_for_batch_patching(self)
         # Path change
         runtime.set_legacy_test_type('DependencyInstallSuccessfully')
         installed_update_count, update_run_successful, maintenance_window_exceeded = runtime.patch_installer.install_updates(runtime.maintenance_window, runtime.package_manager, simulate=True)
-        self.assertEqual(2, installed_update_count)
+        self.assertEqual(5, installed_update_count)
         self.assertTrue(update_run_successful)
         self.assertFalse(maintenance_window_exceeded)
         runtime.stop()
@@ -549,7 +552,7 @@ def test_arch_dependency_install_success(self):
         runtime.set_legacy_test_type('ArchDependency')
         # As all the packages should get installed using batch patching, get_remaining_packages_to_install should return 0 packages
         installed_update_count, update_run_successful, maintenance_window_exceeded = runtime.patch_installer.install_updates(runtime.maintenance_window, runtime.package_manager, simulate=True)
-        self.assertEqual(4, installed_update_count)
+        self.assertEqual(7, installed_update_count)
         self.assertTrue(update_run_successful)
         self.assertFalse(maintenance_window_exceeded)
         runtime.stop()