Skip to content

pgsqlms: improvements and fixes #235

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.

Sign up for GitHub

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

Open
oalbrigt wants to merge 1 commit into
base: master
Choose a base branch
from
Open

pgsqlms: improvements and fixes #235

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 49 additions & 25 deletions script/pgsqlms
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ sub _pg_isready {
# Add 60s to the timeout or use a 24h timeout fallback to make sure
# Pacemaker will give up before us and take decisions
my $timeout = ( _get_action_timeout() || 60*60*24 ) + 60;
my $rc = _runas( $PGISREADY, '-h', $pghost, '-p', $pgport, '-d', 'postgres', '-t', $timeout );
my $rc = _runas( $PGISREADY, '-q', '-h', $pghost, '-p', $pgport, '-d', 'postgres', '-t', $timeout );
Copy link
Member

@ioguix ioguix Oct 16, 2025

Choose a reason for hiding this comment

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

What the point of making pg_isready quiet?
I like having messages in debug level and there's very few messages from this in notice level…

Copy link
Contributor Author

@oalbrigt oalbrigt Oct 17, 2025

Choose a reason for hiding this comment

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

I dont remember, but it might have been due to errors with if()'s expecting an int instead of a string.


# Possible error codes:
# 1: ping rejected (usually when instance is in startup, in crash
Expand Down Expand Up @@ -624,14 +624,18 @@ sub _get_controldata {
and defined $controldata{'redo'}
and defined $controldata{'wal_level'};

ocf_exit_reason( 'Could not read all datas from controldata file for "%s"',
$datadir );
if ( ! ocf_is_probe() ) {
Copy link
Member

@ioguix ioguix Oct 16, 2025

Choose a reason for hiding this comment

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

Could you explain or comment in code why you want to avoid to exit with an error during probe action?

Copy link
Contributor Author

@oalbrigt oalbrigt Oct 17, 2025

Choose a reason for hiding this comment

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

Because the probe-action is run to identify whether the resource is running or not e.g. when you create a resource, and should report not running and not errors, so the cluster doesnt start fencing nodes if you run e.g. pcs resource create --disabled ... to be able to run debug-* actions or similar, or if you ban the resource from a node where the database isnt setup or present.

ocf_exit_reason( 'Could not read all datas from controldata file for "%s"',
$datadir );

ocf_log( 'debug',
"_get_controldata: controldata file: %s",
Data::Dumper->new( [ \%controldata ] )->Terse(1)->Dump, $ans );
ocf_log( 'debug',
"_get_controldata: controldata file: %s",
Data::Dumper->new( [ \%controldata ] )->Terse(1)->Dump, $ans );

exit $OCF_ERR_ARGS;
exit $OCF_ERR_ARGS;
}

return ();
}

# Pead major version from datadir/PG_VERSION and return it as numeric version
Expand All @@ -642,8 +646,12 @@ sub _get_pg_version {

# check PG_VERSION
if ( ! -s "$datadir/PG_VERSION" ) {
ocf_exit_reason( 'PG_VERSION does not exist in "%s"', $datadir );
exit $OCF_ERR_ARGS;
if ( ! ocf_is_probe() ) {
ocf_exit_reason( 'PG_VERSION does not exist in "%s"', $datadir );
exit $OCF_ERR_ARGS;
} else {
return -1;
}
}

unless ( open( $fh, '<', "$datadir/PG_VERSION" ) ) {
Expand Down Expand Up @@ -1324,22 +1332,34 @@ sub pgsql_validate_all {
}

# check notify=true
unless ( defined $ENV{'OCF_RESKEY_CRM_meta_notify'}
and lc($ENV{'OCF_RESKEY_CRM_meta_notify'}) =~ /^true$|^on$|^yes$|^y$|^1$/ ) {
unless ( $__OCF_ACTION eq 'validate-all'
or ( defined $ENV{'OCF_RESKEY_CRM_meta_notify'}
and lc($ENV{'OCF_RESKEY_CRM_meta_notify'}) =~ /^true$|^on$|^yes$|^y$|^1$/ ) ) {
ocf_exit_reason(
'You must set meta parameter notify=true for your "master" resource'
);
return $OCF_ERR_INSTALLED;
}

# check master-max=1
# check promoted_max=1/master-max=1
unless (
defined $ENV{'OCF_RESKEY_CRM_meta_master_max'}
and $ENV{'OCF_RESKEY_CRM_meta_master_max'} eq '1'
$__OCF_ACTION eq 'validate-all'
or
( defined $ENV{'OCF_RESKEY_CRM_meta_promoted_max'}
and $ENV{'OCF_RESKEY_CRM_meta_promoted_max'} eq '1' )
or
(defined $ENV{'OCF_RESKEY_CRM_meta_master_max'}
and $ENV{'OCF_RESKEY_CRM_meta_master_max'} eq '1')
) {
ocf_exit_reason(
'You must set meta parameter master-max=1 for your "master" resource'
);
if ( ocf_version_cmp( $ENV{"OCF_RESKEY_crm_feature_set"}, '3.1.0' ) =~ /^[21]$/ ) {
ocf_exit_reason(
'You must set meta parameter promoted_max=1 for your "promotable" resource'
);
} else {
ocf_exit_reason(
'You must set meta parameter master-max=1 for your "master" resource'
);
}
return $OCF_ERR_INSTALLED;
}

Expand All @@ -1366,14 +1386,14 @@ sub pgsql_validate_all {
}

$guc = qx{ $POSTGRES -C primary_conninfo -D "$pgdata" $start_opts};
unless ($guc =~ /\bapplication_name='?$nodename'?\b/) {
unless ($guc =~ /\bapplication_name='?$nodename'?\b/ or $__OCF_ACTION eq 'validate-all') {
ocf_exit_reason(
q{Parameter "primary_conninfo" MUST contain 'application_name=%s'. }.
q{It is currently set to '%s'}, $nodename, $guc );
return $OCF_ERR_ARGS;
}
}
else {
elsif ($PGVERNUM > -1 ) {
my @content;

# check recovery template
Expand Down Expand Up @@ -1428,14 +1448,14 @@ sub pgsql_validate_all {
}

# require 9.3 minimum
if ( $PGVERNUM < $PGVER_93 ) {
if ( $PGVERNUM < $PGVER_93 && $PGVERNUM > -1 ) {
ocf_exit_reason( "Require 9.3 and more" );
return $OCF_ERR_INSTALLED;
}

# check binaries
unless ( -x $PGCTL and -x $PGPSQL and -x $PGCTRLDATA and -x $PGISREADY
and ( -x $PGWALDUMP or -x "$bindir/pg_xlogdump")
unless ( ( -x $PGCTL and -x $PGPSQL and -x $PGCTRLDATA and -x $PGISREADY
and ( -x $PGWALDUMP or -x "$bindir/pg_xlogdump") ) or ocf_is_probe()
) {
ocf_exit_reason(
"Missing one or more binary. Check following path: %s, %s, %s, %s, %s or %s",
Expand All @@ -1445,7 +1465,7 @@ sub pgsql_validate_all {

# require wal_level >= hot_standby
%cdata = _get_controldata();
unless ( $cdata{'wal_level'} =~ m{hot_standby|logical|replica} ) {
unless ( (defined $cdata{'wal_level'} and $cdata{'wal_level'} =~ m{hot_standby|logical|replica}) or ocf_is_probe() ) {
ocf_exit_reason(
'wal_level must be one of "hot_standby", "logical" or "replica"' );
return $OCF_ERR_ARGS;
Expand Down Expand Up @@ -1599,6 +1619,10 @@ sub pgsql_monitor {
return _confirm_role();
}

if ( ocf_is_probe() ) {
return $OCF_NOT_RUNNING;
}

if ( $pgisready_rc == 1 ) {
# The attempt was rejected.
# This could happen in several cases:
Expand Down Expand Up @@ -2254,13 +2278,13 @@ chdir File::Spec->tmpdir();

# mandatory sanity checks
# check pgdata
if ( ! -d $pgdata ) {
if ( ! -d $pgdata and ! ocf_is_probe() ) {
ocf_exit_reason( 'PGDATA "%s" does not exist', $pgdata );
exit $OCF_ERR_ARGS;
}

# check datadir
if ( ! -d $datadir ) {
if ( ! -d $datadir and ! ocf_is_probe() ) {
ocf_exit_reason( 'data_directory "%s" does not exist', $datadir );
exit $OCF_ERR_ARGS;
}
Expand Down