On Tue, Mar 12, 2019 at 10:08:19PM +0100, Fabien COELHO wrote:
> This refactoring patch is ok for me: applies, compiles, check is ok.
> However, Am I right in thinking that the change should propagate to other
> tools which manipulate the control file, eg pg_resetwal, postmaster… So that
> there would be only one shared API to update the control file?
Yes, that would be nice, for now I have focused. For pg_resetwal yes
we could do it easily. Would you like to send a patch?
> I'm wondering whether there should be something done so that the
> inter-release documentation navigation works? Should the section keep the
> former name? Is it managed by hand somewhere else? Maybe it would require to
> keep the refsect1 id, or to duplicate it, not sure.
When it came to the renaming of pg_receivexlog to pg_receivewal, we
did not actually do anything in the core code, and let the magic
happen on pgsql-www. I have also pinged pgsql-packagers about the
renaming and it is not really an issue on their side. So I have
committed the renaming to pg_checksums as well. So now remains only
the new options.
> In "doc/src/sgml/ref/allfiles.sgml" there seems to be a habit to align on
> the SYSTEM keyword, which is not fellowed by the patch.
Sure. I sent an updated patch to actually fix that, and also address
a couple of other side things I noticed on the way like the top
refentry in the docs or the header format at the top of
pg_checksums.c as we are on tweaking the area.
> This seem contradictory to me: you want to disable checksum, and they are
> already disabled, so nothing is needed. How does that qualifies as a
> "failed" operation?
If the operation is automated, then a proper reaction can be done if
multiple attempts are done. Of course, I am fine to tune things one
way or the other depending on the opinion of the crowd here. From the
opinions gathered, I can see that (Michael * 2) prefer failing with
exit(1), while (Fabien * 1) would like to just do exit(0).
> Further review will come later.
Thanks, Fabien!
> Indeed. I do not immediately see the use case where no syncing would be a
> good idea. I can see why it would be a bad idea. So I'm not sure of the
> concept.
To leverage the buildfarm effort I think this one is worth it. Or we
finish to fsync the data folder a couple of times, which would make
the small-ish buildfarm machines suffer more than they need.
I am going to send a rebased patch set of the remaining things at the
top of the discussion as well.
--
Michael