-
Notifications
You must be signed in to change notification settings - Fork 603
pgsql: Fixed an issue where tmpdir was not created when rep_mode was set to slave. #2071
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
base: main
Are you sure you want to change the base?
Conversation
Can one of the admins check and authorise this run please: https://ci.kronosnet.org/job/resource-agents/job/resource-agents-pipeline/job/PR-2071/1/input |
heartbeat/pgsql
Outdated
rc=$? | ||
if [ $rc -eq 1 ]||[ $rc -eq 2 ]; then # PosrgreSQL 12 or later. | ||
if ! mkdir -p $OCF_RESKEY_tmpdir || ! chown $OCF_RESKEY_pgdba $OCF_RESKEY_tmpdir || ! chmod 700 $OCF_RESKEY_tmpdir; then | ||
ocf_exit_reason "Can't create directory $OCF_RESKEY_tmpdir or it is not readable by $OCF_RESKEY_pgdba" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should say "writable", and should only be run during the start-action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your comment.
I modified the code based on the following reasoning:
- The existing code already included a similar process for creating tmpdir within validate_ocf_check_level_10(), so I aligned with that approach.
- As you mentioned, if it should only run during the start action, should I move the above code to the start process as well?
- Additionally, I slightly modified the code so it only runs when rep_mode="slave" is set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should move it to the start-action, as we dont need it for regular actions (if it's needed for monitor or similar we might want to change the logic in those actions).
You'll also have to cover "promoted" or what term they are using in the latest PostgreSQL releases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your comment.
As requested, I've moved the tmpdir creation process to the start action.
cd8e58f
Based on my research, all write operations to tmpdir within the start action were consolidated in make_recovery_conf(), so I've grouped the processing there.
How does this approach look?
c86b25d
to
0c3f2cf
Compare
Can one of the admins check and authorise this run please: https://ci.kronosnet.org/job/resource-agents/job/resource-agents-pipeline/job/PR-2071/2/input |
Can one of the admins check and authorise this run please: https://ci.kronosnet.org/job/resource-agents/job/resource-agents-pipeline/job/PR-2071/3/input |
9829572
to
cd8e58f
Compare
Can one of the admins check and authorise this run please: https://ci.kronosnet.org/job/resource-agents/job/resource-agents-pipeline/job/PR-2071/4/input |
if [ $rc -eq 1 ]||[ $rc -eq 2 ]; then # PosrgreSQL 12 or later. | ||
if ! mkdir -p $OCF_RESKEY_tmpdir || ! chown $OCF_RESKEY_pgdba $OCF_RESKEY_tmpdir || ! chmod 700 $OCF_RESKEY_tmpdir; then | ||
ocf_exit_reason "Can't create directory $OCF_RESKEY_tmpdir or it is not readable by $OCF_RESKEY_pgdba" | ||
return $OCF_ERR_PERM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove the outer if[]
here, and and use [ "$OCF_RESKEY_rep_mode" = "slave" ] && return $OCF_ERR_PERM || return $OCF_ERR_GENERIC
here instead.
Problem Details
When starting a pgsql resource with rep_mode="slave", the following error occurred, causing the start operation to fail.
The error message is as follows:
Environment
Solution
In the current code, the tmp directory is not created when rep_mode="slave".
The existing code creates the tmp directory only if the result of is_replication() is true within the validate_ocf_check_level_10 function.
To resolve this issue, I modified the validate_ocf_check_level_10 function to create the tmp directory even when rep_mode="slave" is set.