Skip to content

Commit c2234f1

Browse files
committed
Use pg_ctl for cross-platform shutdown
Polite `taskkill /PID` on Windows does not actually trigger PostgreSQL's graceful shutdown, so the wait_for_shutdown loop hit the 60s timeout on every stop call in the Windows SDK test suite (~20 min vs 6 min baseline). Delegate to `pg_ctl stop -m fast -w -t <timeout>` instead: pg_ctl knows the correct per-platform shutdown signal and waits for the postmaster to exit itself. Fall back to SIGKILL + error on timeout, same as before. Also fix find_pg_ctl_binary to match find_psql_binary's layout — instance metadata stores the parent installation_dir, with `<version>/bin/` under it.
1 parent a771508 commit c2234f1

1 file changed

Lines changed: 73 additions & 24 deletions

File tree

src/main.rs

Lines changed: 73 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -961,23 +961,6 @@ fn start(
961961
Ok(())
962962
}
963963

964-
fn send_term_signal(pid: u32) {
965-
#[cfg(unix)]
966-
{
967-
use std::process::Command;
968-
let _ = Command::new("kill")
969-
.args(["-TERM", &pid.to_string()])
970-
.output();
971-
}
972-
#[cfg(windows)]
973-
{
974-
use std::process::Command;
975-
let _ = Command::new("taskkill")
976-
.args(["/PID", &pid.to_string()])
977-
.output();
978-
}
979-
}
980-
981964
fn send_kill_signal(pid: u32) {
982965
#[cfg(unix)]
983966
{
@@ -1016,6 +999,40 @@ fn wait_for_shutdown(pid: u32, data_dir: &PathBuf, timeout: std::time::Duration)
1016999
}
10171000
}
10181001

1002+
fn find_pg_ctl_binary(installation_dir: &PathBuf) -> Result<PathBuf, CliError> {
1003+
let pg_ctl_name = if cfg!(windows) { "pg_ctl.exe" } else { "pg_ctl" };
1004+
1005+
// Same layout as find_psql_binary: installation_dir/<version>/bin/pg_ctl
1006+
if let Ok(entries) = fs::read_dir(installation_dir) {
1007+
for entry in entries.flatten() {
1008+
let candidate = entry.path().join("bin").join(pg_ctl_name);
1009+
if candidate.exists() {
1010+
return Ok(candidate);
1011+
}
1012+
}
1013+
}
1014+
1015+
let direct = installation_dir.join("bin").join(pg_ctl_name);
1016+
if direct.exists() {
1017+
return Ok(direct);
1018+
}
1019+
1020+
Err(CliError::Io(std::io::Error::new(
1021+
std::io::ErrorKind::NotFound,
1022+
format!(
1023+
"{} not found in {}",
1024+
pg_ctl_name,
1025+
installation_dir.display()
1026+
),
1027+
)))
1028+
}
1029+
1030+
/// Stop the postmaster cleanly. Delegates to `pg_ctl stop -m fast -w` so the
1031+
/// shutdown uses the correct per-platform signal (especially on Windows, where
1032+
/// plain `taskkill` does not trigger graceful PostgreSQL shutdown) and the
1033+
/// command does not return until the postmaster has exited and postmaster.pid
1034+
/// is gone. On timeout, falls back to SIGKILL and returns an error so callers
1035+
/// know the shutdown wasn't clean. See vectorize-io/pg0#17.
10191036
fn stop(name: String, timeout_secs: u64) -> Result<(), CliError> {
10201037
let info = load_instance(&name)?.ok_or(CliError::NoInstance)?;
10211038

@@ -1026,16 +1043,32 @@ fn stop(name: String, timeout_secs: u64) -> Result<(), CliError> {
10261043

10271044
println!("Stopping PostgreSQL instance '{}' (pid: {})...", name, info.pid);
10281045

1029-
send_term_signal(info.pid);
1046+
let pg_ctl = find_pg_ctl_binary(&info.installation_dir)?;
1047+
let pg_ctl_status = std::process::Command::new(&pg_ctl)
1048+
.arg("stop")
1049+
.arg("-D")
1050+
.arg(&info.data_dir)
1051+
.arg("-m")
1052+
.arg("fast")
1053+
.arg("-w")
1054+
.arg("-t")
1055+
.arg(timeout_secs.to_string())
1056+
.status()?;
10301057

1031-
let timeout = std::time::Duration::from_secs(timeout_secs);
1032-
if wait_for_shutdown(info.pid, &info.data_dir, timeout) {
1058+
// Belt-and-suspenders: even after pg_ctl reports success, give the OS a
1059+
// brief moment to reap the process and remove postmaster.pid before any
1060+
// subsequent start runs.
1061+
if pg_ctl_status.success()
1062+
&& wait_for_shutdown(
1063+
info.pid,
1064+
&info.data_dir,
1065+
std::time::Duration::from_secs(5),
1066+
)
1067+
{
10331068
println!("PostgreSQL instance '{}' stopped.", name);
10341069
return Ok(());
10351070
}
10361071

1037-
// Timed out — force-kill as a safety net, but surface an error so the
1038-
// caller knows the shutdown wasn't clean.
10391072
eprintln!(
10401073
"PostgreSQL did not shut down within {}s, sending SIGKILL...",
10411074
timeout_secs
@@ -1078,8 +1111,24 @@ fn drop_instance(name: String, force: bool) -> Result<(), CliError> {
10781111
// an in-progress shutdown.
10791112
if is_process_running(info.pid) {
10801113
println!("Stopping PostgreSQL instance '{}' (pid: {})...", name, info.pid);
1081-
send_term_signal(info.pid);
1082-
if !wait_for_shutdown(info.pid, &info.data_dir, std::time::Duration::from_secs(60)) {
1114+
let stopped = match find_pg_ctl_binary(&info.installation_dir) {
1115+
Ok(pg_ctl) => std::process::Command::new(&pg_ctl)
1116+
.arg("stop")
1117+
.arg("-D")
1118+
.arg(&info.data_dir)
1119+
.arg("-m")
1120+
.arg("fast")
1121+
.arg("-w")
1122+
.arg("-t")
1123+
.arg("60")
1124+
.status()
1125+
.map(|s| s.success())
1126+
.unwrap_or(false),
1127+
Err(_) => false,
1128+
};
1129+
if !stopped
1130+
|| !wait_for_shutdown(info.pid, &info.data_dir, std::time::Duration::from_secs(5))
1131+
{
10831132
send_kill_signal(info.pid);
10841133
}
10851134
}

0 commit comments

Comments
 (0)