HI Michael, Thank you for the review!
> Why not just > switch recovery_params_set to false once ReplaceRecoveryConfig() is > called? Stupid me! > Why do we need to call again GenerateRecoveryConfig() when resetting > recovery.conf/postgresql.conf.sample with its original contents before > switching the system ID of the new replica? I may be missing > something, of course, but we're done with recovery so I don't quite > see the point in appending the recovery config generated with the > original contents. If this is justified (don't think it is), this > deserves a comment to explain the reason behind this logic. This relates to the point I mentioned earlier about being unsure whether we should preserve `primary_conninfo`: > I kept primary_conninfo as-is for now since I'm not totally sure if we need to touch it The reason I called `GenerateRecoveryConfig()` was to regenerate the `primary_conninfo` string in the recovery configuration file. If we should remove it, then the reset function can be much simpler. Could you please help me to clarify should we regenerate `primary_conninfo` or we can safely purge it? Best regards, Alena Vinter
