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

fix the range of keep data generation #218

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.

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

Successfully merging this pull request may close these issues.

2 participants