On Wed, Mar 25, 2020 at 09:08:12PM -0300, Alvaro Herrera wrote: > ... as in the attached version.
Patch 0001 is one that has already been discussed previously (thanks for keeping the extra comments about GUCs and WAL receivers). That looks fine to me. > Sergei included LOG messages to indicate which setting has been changed. > I put these in "#if 0" for now, but I'm thinking to remove these > altogether; we already have LOG messages when a setting changes value, > per ProcessConfigFileInternal(); the log sequence looks like this, taken > from tmp_check/log/001_stream_rep_standby_2.log after running the tests: Yes, it makes sense to remove any knowledge of GUC updates from StartupRequestWalReceiverRestart(). I would add a description at the top of this routine by the way. +extern void ProcessStartupSigHup(void); This is declared, but used nowhere in patch 0002. Wouldn't it be better to be more careful about the NULL-ness of the string parameters in StartupRereadConfig()? + if (!slotnameChanged && strcmp(PrimarySlotName, "") == 0) + tempSlotChanged = tempSlot != wal_receiver_create_temp_slot; I would add parens here for clarity. > which looks sufficient. Maybe we can reword that new message, say "wal > receiver process shutdown forced by parameter change". Not sure if we > can or should adjust the FATAL line; probably not worth the trouble. There is merit to keep the LOG in StartupRequestWalReceiverRestart() unchanged, as the routine may get called in the future in some other context. -- Michael
signature.asc
Description: PGP signature
