Bonjour Michaël,
It adds an (now mandatory) --action parameter that takes either verify,
enable or disable as argument.
I'd rather have explicit switches for verify, enable & disable, and verify
would be the default if none is provided.
Okay, noted for the separate switches. But I don't agree with the
point of assuming that --verify should be enforced if no switches are
defined. That feels like a trap for newcomers of this tool..
Hmmm. It does something safe and useful, especially if it also works
online (patch pending), and the initial tool only does checking. However,
I'd be okay for no default.
For enable/disable, while the command is running, it should mark the cluster
as opened to prevent an unwanted database start. I do not see where this is
done.
You have pretty much the same class of problems if you attempt to
start a cluster on which pg_rewind or the existing pg_verify_checksums
is run after these have scanned the control file to make sure that
they work on a cleanly-stopped instance. In short, this is a deal
between code simplicity and trying to have the tool outsmart users in
the way users use the tool. I tend to prefer keeping the code simple
and not worry about cases where the users mess up with their
application, as there are many more things which could go wrong.
Hmmm. I do not buy the comparison.
A verify that fails is not a big problem, you can run it again. If
pg_rewind fails, you can probably run it again as well, the source is
probably still consistent even if it has changed, and too bad for the
target side, but it was scheduled to be overwritten anyway.
However, a tool which overwrites files beyond the back of a running server
is a recipee for data-loss, so I think it should take much more care, i.e.
set the server state into some specific safe state.
About code simplicity: probably there is, or there should be, a
change-the-state function somewhere, because quite a few tools could use
it?
--
Fabien.
