Have drop node --force --destroy work as expected again#743
Conversation
DimCitus
left a comment
DimCitus
left a comment
There was a problem hiding this comment.
I think this needs more comments... it's not obvious what your intention are in those code paths...
| "drop", | ||
| "node", | ||
| "--destroy", | ||
| "--force", |
There was a problem hiding this comment.
Do we really want to change the way we run all of our test suite to use --force?
DimCitus
added
Developer productivity
JelteF
force-pushed
the
drop-force-without-wait
branch
2 times, most recently
from
d587133 to
0a0ad4e
Compare
| * If --force is used, we skip the transition to "dropped". So a currently | ||
| * running process won't realise it's dropped, so it will not exit by | ||
| * itself. So there's no point in waiting for 30 seconds. Instead we | ||
| * manually kill it using SIGQUIT right away. |
There was a problem hiding this comment.
Can we bypass calling wait_for_pid_to_exit entirely in this case, then?
There was a problem hiding this comment.
I think it's still nice to stop a currently running pg_autoctl process in this case. The first wait_for_pid_to_exit then simply becomes an easy way to check if there is a running process.
JelteF
force-pushed
the
drop-force-without-wait
branch
2 times, most recently
from
8646e91 to
43af617
Compare
DimCitus
left a comment
DimCitus
left a comment
There was a problem hiding this comment.
Thanks for the changes. I think we only need a last pass of small changes!
| } | ||
| else | ||
| { | ||
| if (!is_process_stopped(config->pathnames.pid, &stopped, &pid)) |
There was a problem hiding this comment.
The function is_process_stopped is used a lot and often with a negation before it. I think it would be all easier if we had the function pidfile_process_is_running (similar to pg_setup_is_running and others).
There was a problem hiding this comment.
Is discussed over chat, flipping this brings in other consistency problems. Leaving it like this.
JelteF
force-pushed
the
drop-force-without-wait
branch
from
c7d708c to
d446770
Compare
2 participants
The following command would get stuck for 30 seconds when the pg_autoctl service was still running.