Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #26017 - Skip qpidd under load #319

Closed

Conversation

upadhyeammit
Copy link
Contributor

The foreman-maintain already had logic to try taking config file backup 3 times. With this PR,

  1. We try the to collect backup 2 times with qpidd data. Here 10 seconds delay between each run.
  2. If backup fails then we skip qpidd data in third run,
WARNING: Attempt 1/3 to collect all config files failed!
Some files were modified during creation of the archive.
Waiting 10 seconds before re-try
/ Collecting config files to backup                                             
WARNING: Attempt 2/3 to collect all config files failed!
Some files were modified during creation of the archive.
Waiting 10 seconds before re-try
| Collecting config files to backup                                             
WARNING: Attempt 3/3 to collect all config files failed!
Some files were modified during creation of the archive.
- Collecting config files to backup, skipping /var/lib/qpidd                    
| Collecting config files to backup                                   [OK]      
~~

@theforeman-bot
Copy link
Member

Issues: #26017

@upadhyeammit
Copy link
Contributor Author

need to test this in two ways,

  1. Collect online backup in regular way. Its expected that we collect the qpidd directory and restore it.
  2. Collect the online backup while below command is running in extra terminal,
# while true; do  echo " " >> /var/lib/qpidd/test;done

Here we expect that qpidd directory is being skipped in backup. Also check restoration after backup to confirm there are no errors.

@vsedmik
Copy link

vsedmik commented Feb 27, 2020

Tested the scenarios above on Sat 6.7 snap 13 (+patch and pulled Amit's branch) and seem to work well (/var/lib/qpidd/* collected and restored in case 1 and skipped after three retries in case 2), health check, UI and the content was ok after restore as well.
Tomorrow I'm going to test 'full stack' approach with some real load via hosts and hammer, stay tuned.

if attempt_no == MAX_RETRIES
if attempt_no == MAX_RETRIES && @exclude_qpidd_data
tar_cmd = tar_command(configs, to_exclude.push('/var/lib/qpidd'))
with_spinner('Collecting config files to backup, skipping /var/lib/qpidd') do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As offline discussion, it would be good to have warning instead of spinner showing this information.

end

# rubocop:disable Metrics/MethodLength
def run
logger.debug("Invoking tar from #{FileUtils.pwd}")
tar_cmd = tar_command
configs, to_exclude = config_files
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing configs and to_exclude to tar_command method, I think it would be good to have options to tar_command method which will accept extra directories to exclude and append to_exclude list inside tar_command?

@upadhyeammit
Copy link
Contributor Author

I am closing this PR, shall be adding new PR for remaining work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants