Implement patching in batches to improve performance. #171
Conversation
Done code changes for Aptitude package manager and did manual testing. Code changes for Yum and Zipper is pending should be completed by next week. Adding unit tests is pending. Also, due to signature change of the method install_update_and_dependencies, there are some tests failing. I will work on those tests once we confirm the signature of the method. Using Stopwatch implementation in this PR. The implementation of Stopwatch is in the PR #161 which is going to be merged soon.
GAURAVRAMRAKHYANI
requested review from
kjohn-msft,
najams and
rane-rajasi
as code owners
Codecov Report
@@ Coverage Diff @@
## master #171 +/- ##
==========================================
+ Coverage 89.28% 89.78% +0.50%
==========================================
Files 90 90
Lines 13426 13937 +511
==========================================
+ Hits 11987 12514 +527
+ Misses 1439 1423 -16
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Did manual testing and checked that packages are getting installed and the installation status of the packages are shown correctly in the portal.
|
Will review this iteration by 2/15 PST |
…ests are succeeding now with these changes. (a) updated function name is_packages_install_time_available to is_package_install_time_available (b) Made reboot_manager mandatory in the function is_packages_install_time_available (c) Created a function install_update_and_dependencies_and_get_status
…o install packages in batch then do not stop the complete process of installing patches. The reamaining packages can be installed sequentially and there is already check for maintenance window cutoff time when installing packages sequentially so that the installation is stopped when there is not enough remaining time in maintenance window for installing package. Tested this scenario. (b) After batch patching, there is call get_remaining_packages_to_install. The time to execute get_remaining_packages_to_install should not be counted in time taken by nistalling packages in batches. Hence, stopping the stopwatch before calling get_remaining_packages_to_install and writing batch installation details in the telemetry after the call get_remaining_packages_to_install so that we also get the remaining packages to install in the telemetry. Tested this scenario.
Update the main comment/PR description with items completed vs pending and changes to implementation details, if any |
kjohn-msft
left a comment
kjohn-msft
left a comment
There was a problem hiding this comment.
Comments inline. Need to do a second pass on the PatchInstaller because of degree of changes and overall risk.
(a) Addressing PR comments (b) Adding some comments in the code.
|
@GAURAVRAMRAKHYANI please get latest, and get sign-offs on Monday from @SathishMSFT and @nikhim-um if the code is all done (if all Rajasi's comments are addressed). @rane-rajasi please plan on reviewing this daily and focus on removing regression risk. @rane-rajasi you may make any code-organization comments you are/were planning to leave out of this PR and DIY if deemed absolutely necessary to avoid turn-around loops here. |
rane-rajasi
left a comment
rane-rajasi
left a comment
There was a problem hiding this comment.
There's only one comment pending resolution, everything else looks good. Have tagged you in that open comment and also resolved all of my earlier feedbacks to make it easier to access the open one. Please take a look at it
kjohn-msft
left a comment
kjohn-msft
left a comment
There was a problem hiding this comment.
Relaxed bar to ensure PR merge. One item must be addressed prior to deployment.
Currently Azure Update Management installs one package (along with packages it depends on) at a time using a package manager command. This takes longer time in comparison to installing multiple packages (along with packages they are dependent on) using a package manager command. So, to improve performance, multiple packages can be installed in a batch which takes lesser time than installing the packages one by one sequentially.
In this PR, the code changes are done to install packages in batches. Initially batch size is kept as 3 so 3 packages are installed in a batch (along with packages they are dependent on). In case there are any failures in installing packages in batches, the failed packages are getting installed sequentially as per the existing method of installing packages. Initially batch size is kept as 3 which can be changed later based on the observations on telemetry data and customer feedback.
Code changes done:
Product and test code changes are completed, only review is pending.
Manual testings completed:
(a) Tested that the installation of all the patches is successful.
(b) Tested with total number of available patches to install is greater than batch size (patches will be installed in multiple batches) and total number of patches to install is less than batch size (patches will be installed in single batch).
(c) The portal shows correct status of the patches.
(d) Test with different classification and packages to include and exclude. Check if status of packages is shown correctly in the portal.
(e) If remaining time is not available in maintenance window to install updates in batch, then do not proceed with installing the remaining batches. The remaining packages should be attempted to install sequentially. If there is not enough time available to install packages sequentially then the installation process is stopped at that time. Also, check if status is marked correctly for non-attempted packages.
(f) Tested install updates job for all the three package managers: Apt, Yum and Zypper.
(g) Verified that after batch patching is done, the get available updates is returning the list of updates available and they are getting installed as part of sequential patching.