Skip to content

Commit

Permalink
Bug/837 No need to checkpoint and restart if pg is not running (#839)
Browse files Browse the repository at this point in the history
* No need to checkpoint and restart if pg is not running

When updating replication settings, no need to attempt a checkpoint and
restart when postgres keeper is not running. Just start the service.

Fixes #837

* Small test fix: re.match only checks a single line

re.search will not give us false positives in this test
  • Loading branch information
Rachel Heaton committed Nov 25, 2021
1 parent 37a2496 commit ec055df
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 16 deletions.
28 changes: 20 additions & 8 deletions src/bin/pg_autoctl/keeper.c
Original file line number Diff line number Diff line change
Expand Up @@ -973,17 +973,29 @@ keeper_ensure_configuration(Keeper *keeper, bool postgresNotRunningIsOk)
log_info("Replication settings at \"%s\" have changed, "
"restarting Postgres", upstreamConfPath);

if (!pgsql_checkpoint(&(postgres->sqlClient)))
if (pg_setup_is_running(pgSetup))
{
log_warn("Failed to CHECKPOINT before restart, "
"see above for details");
}
if (!pgsql_checkpoint(&(postgres->sqlClient)))
{
log_warn("Failed to CHECKPOINT before restart, "
"see above for details");
}

if (!keeper_restart_postgres(keeper))
if (!keeper_restart_postgres(keeper))
{
log_error("Failed to restart Postgres to enable new "
"replication settings, see above for details");
return false;
}
}
else
{
log_error("Failed to restart Postgres to enable new "
"replication settings, see above for details");
return false;
if (!ensure_postgres_service_is_running(postgres))
{
log_error("Failed to start Postgres with new "
"replication settings, see above for details");
return false;
}
}
}
}
Expand Down
18 changes: 11 additions & 7 deletions tests/pgautofailover_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -655,16 +655,20 @@ def show_uri(self, json=False):
out, err, ret = command.execute("show uri", "show", "uri")
return out

def logs(self):
def logs(self, log_type=""):
log_string = ""
if self.running():
out, err, ret = self.stop_pg_autoctl()
log_string += f"STDOUT OF PG_AUTOCTL FOR {self.datadir}:\n"
log_string += f"{self.pg_autoctl.cmd}\n{out}\n"
log_string += f"STDERR OF PG_AUTOCTL FOR {self.datadir}:\n{err}\n"

pglogs = self.get_postgres_logs()
log_string += f"POSTGRES LOGS FOR {self.datadir}:\n{pglogs}\n"
if not log_type or (log_type == "STDOUT"):
log_string += f"STDOUT OF PG_AUTOCTL FOR {self.datadir}:\n"
log_string += f"{self.pg_autoctl.cmd}\n{out}\n"
if not log_type or (log_type == "STDERR"):
log_string += (
f"STDERR OF PG_AUTOCTL FOR {self.datadir}:\n{err}\n"
)
if not log_type or (log_type == "POSTGRES"):
pglogs = self.get_postgres_logs()
log_string += f"POSTGRES LOGS FOR {self.datadir}:\n{pglogs}\n"
return log_string

def get_events_str(self):
Expand Down
10 changes: 9 additions & 1 deletion tests/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ def test_003_init_secondary():
node1.get_synchronous_standby_names_local(),
"ANY 1 (pgautofailover_standby_2)",
)
logs = node2.logs("STDERR")
match = re.search(
"Failed to connect to .*, retrying until the server is ready", logs
)
if match:
print("Found connection failure in logs: ", match[0])
assert not match
node2.run()


def test_004_failover():
Expand All @@ -89,7 +97,7 @@ def test_005_logging_of_passwords():
assert "password=****" in logs
# We are still logging passwords when the pguri is incomplete and when printing settings,
# so assert that it's not there in other cases:
assert not re.match(
assert not re.search(
"^(?!primary_conninfo|Failed to find).*%s.*$" % replication_password,
logs,
)

0 comments on commit ec055df

Please sign in to comment.