Skip to content

fix the range of keep data generation #218

New issue

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

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

Already on GitHub? # to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

huangfumingyue
Copy link
Contributor

If you set keep_data_generations = 0, the backup you got will be deleted immediately,
I think the setting like keep_data_generations = 0 is meaningless.
So, In this PR, the minimum value of the setting range of keep_data_generations has been changed to 1.
In addition, keep_srvlog_days, keep_srvlog_files have been modified in the same way.

@mikecaat
Copy link
Contributor

Hi, thanks for making the PR.
I have some comments.

  • Is there any reason that you didn't change other keep-* parameters (ex. keep-data-days)?
  • Is it better use parse_uint32() in parse_posi()?
  • Though I'm not confident, is it good to add parse_posi() in pguc.c because it seems that the function only returns primitive data types? Is it better to use 'f' option?

@@ -165,32 +165,54 @@ echo ''
echo '###### COMMAND OPTION TEST-0017 ######'
echo '###### invalid value in pg_rman.ini ######'
init_catalog
echo "SMOOTH_CHECKPOINT=FOO" >> ${BACKUP_PATH}/pg_rman.ini
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better not to change this line and change line 190 because the number of diff became minimum.

@mikecaat
Copy link
Contributor

mikecaat commented Feb 25, 2022

In the first place, I got not to be confidence that we must check if the values are zero because the users may want to remove the backup files.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants