repmgrd: clean up PQExpBuffer handling

Unless the PQExpBuffer is required for the duration of the function,
ensure it's always a variable local to the relevant code block. This
mitigates the risk of accidentally accessing a generically named
PQExpBuffer which hasn't been initialised or was previously terminated.
This commit is contained in:
Ian Barwick
2019-03-26 13:15:25 +09:00
parent 801ed2b0c8
commit 9164d3931b
2 changed files with 302 additions and 245 deletions

View File

@@ -68,7 +68,6 @@ monitor_bdr(void)
t_bdr_node_info bdr_node_info = T_BDR_NODE_INFO_INITIALIZER;
RecordStatus record_status;
NodeInfoListCell *cell;
PQExpBufferData event_details;
instr_time log_status_interval_start;
/* sanity check local database */
@@ -229,6 +228,7 @@ monitor_bdr(void)
if (cell->node_info->node_status == NODE_STATUS_UP)
{
int node_unreachable_elapsed = calculate_elapsed(node_unreachable_start);
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
@@ -366,7 +366,6 @@ do_bdr_failover(NodeInfoList *nodes, t_node_info *monitored_node)
{
PGconn *next_node_conn = NULL;
NodeInfoListCell *cell;
PQExpBufferData event_details;
t_event_info event_info = T_EVENT_INFO_INITIALIZER;
t_node_info target_node = T_NODE_INFO_INITIALIZER;
t_node_info failed_node = T_NODE_INFO_INITIALIZER;
@@ -460,6 +459,9 @@ do_bdr_failover(NodeInfoList *nodes, t_node_info *monitored_node)
log_debug("this node is the failover handler");
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
event_info.conninfo_str = target_node.conninfo;
@@ -499,6 +501,7 @@ do_bdr_failover(NodeInfoList *nodes, t_node_info *monitored_node)
log_info("%s", event_details.data);
termPQExpBuffer(&event_details);
}
unset_bdr_failover_handler(next_node_conn);
@@ -513,7 +516,6 @@ do_bdr_recovery(NodeInfoList *nodes, t_node_info *monitored_node)
{
PGconn *recovered_node_conn;
PQExpBufferData event_details;
t_event_info event_info = T_EVENT_INFO_INITIALIZER;
int i;
bool slot_reactivated = false;
@@ -543,6 +545,8 @@ do_bdr_recovery(NodeInfoList *nodes, t_node_info *monitored_node)
*/
if (PQstatus(local_conn) != CONNECTION_OK)
{
PQExpBufferData event_details;
local_conn = NULL;
log_warning(_("unable to reconnect to local node"));
@@ -613,6 +617,8 @@ do_bdr_recovery(NodeInfoList *nodes, t_node_info *monitored_node)
node_recovery_elapsed = calculate_elapsed(degraded_monitoring_start);
monitored_node->monitoring_state = MS_NORMAL;
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
@@ -641,8 +647,7 @@ do_bdr_recovery(NodeInfoList *nodes, t_node_info *monitored_node)
event_info.conninfo_str = monitored_node->conninfo;
event_info.node_name = monitored_node->node_name;
create_event_notification_extended(
local_conn,
create_event_notification_extended(local_conn,
&config_file_options,
config_file_options.node_id,
"bdr_recovery",
@@ -651,11 +656,11 @@ do_bdr_recovery(NodeInfoList *nodes, t_node_info *monitored_node)
&event_info);
}
termPQExpBuffer(&event_details);
}
update_node_record_set_active(local_conn, monitored_node->node_id, true);
termPQExpBuffer(&event_details);
PQfinish(recovered_node_conn);
return;

View File

@@ -229,10 +229,11 @@ void
monitor_streaming_primary(void)
{
instr_time log_status_interval_start;
PQExpBufferData event_details;
reset_node_voting_status();
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
appendPQExpBuffer(&event_details,
@@ -240,7 +241,6 @@ monitor_streaming_primary(void)
local_node_info.node_name,
local_node_info.node_id);
/* Log startup event */
if (startup_event_logged == false)
{
@@ -266,6 +266,7 @@ monitor_streaming_primary(void)
log_notice("%s", event_details.data);
termPQExpBuffer(&event_details);
}
INSTR_TIME_SET_CURRENT(log_status_interval_start);
local_node_info.node_status = NODE_STATUS_UP;
@@ -287,11 +288,13 @@ monitor_streaming_primary(void)
/* local node is down, we were expecting it to be up */
if (local_node_info.node_status == NODE_STATUS_UP)
{
PQExpBufferData event_details;
instr_time local_node_unreachable_start;
INSTR_TIME_SET_CURRENT(local_node_unreachable_start);
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
appendPQExpBufferStr(&event_details,
@@ -299,7 +302,6 @@ monitor_streaming_primary(void)
log_warning("%s", event_details.data);
local_node_info.node_status = NODE_STATUS_UNKNOWN;
/*
* as we're monitoring the primary, no point in trying to
@@ -315,6 +317,9 @@ monitor_streaming_primary(void)
event_details.data);
termPQExpBuffer(&event_details);
}
local_node_info.node_status = NODE_STATUS_UNKNOWN;
try_reconnect(&local_conn, &local_node_info);
@@ -322,6 +327,7 @@ monitor_streaming_primary(void)
{
int local_node_unreachable_elapsed = calculate_elapsed(local_node_unreachable_start);
int stored_local_node_id = UNKNOWN_NODE_ID;
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
@@ -375,6 +381,8 @@ monitor_streaming_primary(void)
if (config_file_options.degraded_monitoring_timeout > 0
&& degraded_monitoring_elapsed > config_file_options.degraded_monitoring_timeout)
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
appendPQExpBuffer(&event_details,
@@ -476,7 +484,6 @@ loop:
bool
check_primary_status(int degraded_monitoring_elapsed)
{
PQExpBufferData event_details;
PGconn *new_primary_conn;
RecordStatus record_status;
bool resume_monitoring = true;
@@ -494,6 +501,8 @@ check_primary_status(int degraded_monitoring_elapsed)
{
if (degraded_monitoring_elapsed != NO_DEGRADED_MONITORING_ELAPSED)
{
PQExpBufferData event_details;
monitoring_state = MS_NORMAL;
initPQExpBuffer(&event_details);
@@ -517,6 +526,8 @@ check_primary_status(int degraded_monitoring_elapsed)
/* the node is now a standby */
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
if (degraded_monitoring_elapsed != NO_DEGRADED_MONITORING_ELAPSED)
@@ -533,6 +544,7 @@ check_primary_status(int degraded_monitoring_elapsed)
log_notice("%s", event_details.data);
termPQExpBuffer(&event_details);
}
primary_node_id = UNKNOWN_NODE_ID;
@@ -567,6 +579,8 @@ check_primary_status(int degraded_monitoring_elapsed)
*/
if (record_status == RECORD_NOT_FOUND)
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
appendPQExpBuffer(&event_details,
@@ -622,6 +636,7 @@ check_primary_status(int degraded_monitoring_elapsed)
if (resume_monitoring == true)
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
if (degraded_monitoring_elapsed != NO_DEGRADED_MONITORING_ELAPSED)
@@ -671,7 +686,6 @@ monitor_streaming_standby(void)
{
RecordStatus record_status;
instr_time log_status_interval_start;
PQExpBufferData event_details;
MonitoringState local_monitoring_state = MS_NORMAL;
instr_time local_degraded_monitoring_start;
@@ -875,10 +889,13 @@ monitor_streaming_standby(void)
INSTR_TIME_SET_CURRENT(upstream_node_unreachable_start);
initPQExpBuffer(&event_details);
upstream_node_info.node_status = NODE_STATUS_UNKNOWN;
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
appendPQExpBuffer(&event_details,
_("unable to connect to upstream node \"%s\" (node ID: %i)"),
upstream_node_info.node_name, upstream_node_info.node_id);
@@ -906,6 +923,7 @@ monitor_streaming_standby(void)
log_warning("%s", event_details.data);
termPQExpBuffer(&event_details);
}
/*
* if local node is unreachable, make a last-minute attempt to reconnect
@@ -923,6 +941,7 @@ monitor_streaming_standby(void)
if (upstream_node_info.node_status == NODE_STATUS_UP)
{
int upstream_node_unreachable_elapsed = calculate_elapsed(upstream_node_unreachable_start);
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
@@ -939,6 +958,10 @@ monitor_streaming_standby(void)
{
ExecStatusType ping_result;
/*
* we're returning at the end of this block and no longer require the
* event details buffer
*/
termPQExpBuffer(&event_details);
log_notice(_("current upstream node \"%s\" (node ID: %i) is not primary, restarting monitoring"),
@@ -1038,6 +1061,8 @@ monitor_streaming_standby(void)
if (config_file_options.degraded_monitoring_timeout > 0
&& degraded_monitoring_elapsed > config_file_options.degraded_monitoring_timeout)
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
appendPQExpBuffer(&event_details,
@@ -1068,6 +1093,7 @@ monitor_streaming_standby(void)
if (PQstatus(upstream_conn) == CONNECTION_OK)
{
PQExpBufferData event_details;
log_debug("upstream node %i has recovered",
upstream_node_info.node_id);
@@ -1140,6 +1166,7 @@ monitor_streaming_standby(void)
int degraded_monitoring_elapsed;
int former_upstream_node_id = local_node_info.upstream_node_id;
NodeInfoList sibling_nodes = T_NODE_INFO_LIST_INITIALIZER;
PQExpBufferData event_details;
update_node_record_set_primary(local_conn, local_node_info.node_id);
record_status = get_node_record(local_conn, local_node_info.node_id, &local_node_info);
@@ -1286,6 +1313,7 @@ loop:
log_info("%s", monitoring_summary.data);
termPQExpBuffer(&monitoring_summary);
if (monitoring_state == MS_DEGRADED && config_file_options.failover == FAILOVER_AUTOMATIC)
{
if (PQstatus(local_conn) == CONNECTION_OK && repmgrd_is_paused(local_conn))
@@ -1527,7 +1555,6 @@ monitor_streaming_witness(void)
instr_time log_status_interval_start;
instr_time witness_sync_interval_start;
PQExpBufferData event_details;
RecordStatus record_status;
int primary_node_id = UNKNOWN_NODE_ID;
@@ -1654,10 +1681,13 @@ monitor_streaming_witness(void)
INSTR_TIME_SET_CURRENT(upstream_node_unreachable_start);
initPQExpBuffer(&event_details);
upstream_node_info.node_status = NODE_STATUS_UNKNOWN;
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
appendPQExpBuffer(&event_details,
_("unable to connect to primary node \"%s\" (node ID: %i)"),
upstream_node_info.node_name, upstream_node_info.node_id);
@@ -1668,6 +1698,8 @@ monitor_streaming_witness(void)
"repmgrd_upstream_disconnect",
true,
event_details.data);
termPQExpBuffer(&event_details);
}
try_reconnect(&primary_conn, &upstream_node_info);
@@ -1675,6 +1707,7 @@ monitor_streaming_witness(void)
if (upstream_node_info.node_status == NODE_STATUS_UP)
{
int upstream_node_unreachable_elapsed = calculate_elapsed(upstream_node_unreachable_start);
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
@@ -1742,6 +1775,8 @@ monitor_streaming_witness(void)
if (PQstatus(primary_conn) == CONNECTION_OK)
{
PQExpBufferData event_details;
upstream_node_info.node_status = NODE_STATUS_UP;
monitoring_state = MS_NORMAL;
@@ -2517,7 +2552,6 @@ update_monitoring_history(void)
static bool
do_upstream_standby_failover(void)
{
PQExpBufferData event_details;
t_node_info primary_node_info = T_NODE_INFO_INITIALIZER;
RecordStatus record_status = RECORD_NOT_FOUND;
RecoveryType primary_type = RECTYPE_UNKNOWN;
@@ -2599,6 +2633,8 @@ do_upstream_standby_failover(void)
if (standby_follow_result != 0)
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
appendPQExpBuffer(&event_details,
@@ -2676,6 +2712,8 @@ do_upstream_standby_failover(void)
local_node_info.node_id,
primary_node_info.node_id) == false)
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
appendPQExpBuffer(&event_details,
_("unable to set node %i's new upstream ID to %i"),
@@ -2710,6 +2748,9 @@ do_upstream_standby_failover(void)
local_node_info.upstream_node_id = primary_node_info.node_id;
}
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
appendPQExpBuffer(&event_details,
@@ -2727,6 +2768,7 @@ do_upstream_standby_failover(void)
event_details.data);
termPQExpBuffer(&event_details);
}
/* keep the primary connection open */
@@ -2737,7 +2779,6 @@ do_upstream_standby_failover(void)
static FailoverState
promote_self(void)
{
PQExpBufferData event_details;
char *promote_command;
int r;
@@ -2802,10 +2843,13 @@ promote_self(void)
int primary_node_id;
upstream_conn = get_primary_connection(local_conn,
&primary_node_id, NULL);
&primary_node_id,
NULL);
if (PQstatus(upstream_conn) == CONNECTION_OK && primary_node_id == failed_primary.node_id)
{
PQExpBufferData event_details;
log_notice(_("original primary (id: %i) reappeared before this standby was promoted - no action taken"),
failed_primary.node_id);
@@ -2833,16 +2877,13 @@ promote_self(void)
log_error(_("promote command failed"));
initPQExpBuffer(&event_details);
create_event_notification(
NULL,
create_event_notification(NULL,
&config_file_options,
local_node_info.node_id,
"repmgrd_promote_error",
true,
event_details.data);
termPQExpBuffer(&event_details);
"");
return FAILOVER_STATE_PROMOTION_FAILED;
}
@@ -2850,7 +2891,8 @@ promote_self(void)
/* bump the electoral term */
increment_current_term(local_conn);
initPQExpBuffer(&event_details);
{
PQExpBufferData event_details;
/* update own internal node record */
record_status = get_node_record(local_conn, local_node_info.node_id, &local_node_info);
@@ -2858,6 +2900,8 @@ promote_self(void)
/*
* XXX here we're assuming the promote command updated metadata
*/
initPQExpBuffer(&event_details);
appendPQExpBuffer(&event_details,
_("node %i promoted to primary; old primary %i marked as failed"),
local_node_info.node_id,
@@ -2872,6 +2916,7 @@ promote_self(void)
event_details.data);
termPQExpBuffer(&event_details);
}
return FAILOVER_STATE_PROMOTED;
}
@@ -2968,8 +3013,6 @@ static FailoverState
follow_new_primary(int new_primary_id)
{
char parsed_follow_command[MAXPGPATH] = "";
PQExpBufferData event_details;
int i, r;
/* Store details of the failed node here */
@@ -3069,6 +3112,8 @@ follow_new_primary(int new_primary_id)
if (upstream_recovery_type == RECTYPE_PRIMARY)
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
appendPQExpBufferStr(&event_details,
_("original primary reappeared - no action taken"));
@@ -3148,6 +3193,9 @@ follow_new_primary(int new_primary_id)
repmgrd_set_local_node_id(local_conn, config_file_options.node_id);
repmgrd_set_pid(local_conn, getpid(), pid_file);
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
appendPQExpBuffer(&event_details,
_("node %i now following new upstream node %i"),
@@ -3164,6 +3212,7 @@ follow_new_primary(int new_primary_id)
event_details.data);
termPQExpBuffer(&event_details);
}
return FAILOVER_STATE_FOLLOWED_NEW_PRIMARY;
}
@@ -3172,8 +3221,6 @@ follow_new_primary(int new_primary_id)
static FailoverState
witness_follow_new_primary(int new_primary_id)
{
PQExpBufferData event_details;
t_node_info new_primary = T_NODE_INFO_INITIALIZER;
RecordStatus record_status = RECORD_NOT_FOUND;
bool new_primary_ok = false;
@@ -3250,6 +3297,9 @@ witness_follow_new_primary(int new_primary_id)
return FAILOVER_STATE_FOLLOW_FAIL;
}
{
PQExpBufferData event_details;
initPQExpBuffer(&event_details);
appendPQExpBuffer(&event_details,
_("witness node %i now following new primary node %i"),
@@ -3258,8 +3308,7 @@ witness_follow_new_primary(int new_primary_id)
log_notice("%s", event_details.data);
create_event_notification(
upstream_conn,
create_event_notification(upstream_conn,
&config_file_options,
local_node_info.node_id,
"repmgrd_failover_follow",
@@ -3267,6 +3316,7 @@ witness_follow_new_primary(int new_primary_id)
event_details.data);
termPQExpBuffer(&event_details);
}
return FAILOVER_STATE_FOLLOWED_NEW_PRIMARY;
}
@@ -3696,6 +3746,8 @@ do_election(NodeInfoList *sibling_nodes, int *new_primary_id)
reset_node_voting_status();
termPQExpBuffer(&nodes_with_primary_visible);
return ELECTION_CANCELLED;
}