On Sun, Mar 17, 2019 at 12:44:39PM +0100, Fabien COELHO wrote: > I kept the initial no-parameter function which calls the new one with 4 > parameters, though, because it looks more homogeneous this way in the > backend code. This is debatable.
From a compatibility point of view, your position actually makes sense, at least to me and after sleeping on it as UpdateControlFile is not static, and that there are interactions with the other local routines to read and write the control file because of the variable ControlFile at the top of xlog.c. So I have kept the original interface, being now only a wrapper of the new routine. > Attached is an update. Thanks, I have committed the patch after fixing a couple of things. After considering the interface, I have switched to a single boolean as I could not actually imagine with what kind of fancy features this could be extended further more. If I am wrong, let's adjust it later on. Here are my notes about the fixes: - pg_resetwal got broken because the path to the control file was incorrect. Running tests of pg_upgrade or the TAP tests of pg_resetwal showed the failure. - The previously-mentioned problem with close() in the new routine is fixed. - Header comments at the top of update_controlfile were a bit messed up (s/Not/Note/, missed an "up" as well). - pg_rewind was issuing a flush of the control file even if --no-sync was used. - Nit: incorrect header order in controldata_utils.c. I have kept the backend-only includes grouped though. -- Michael
signature.asc
Description: PGP signature
