On Tue, Dec 30, 2025 at 02:34:21PM +0700, Alena Vinter wrote:
> I’m now back on track and have refined the implementation based on earlier
> discussions. The current version fully adopts the `include_if_exists`
> approach, writes temporary recovery settings to a separate config file, and
> disables it on exit by renaming to `.disabled`.
It seems roughly OK, so I have put my hands on it. A couple of notes
regarding the things done in the updated version attached:
- Addition of two tests to check pg_createsubscriber.conf.disabled in
the data folders of node S and K.
- More description in the tests.
- The "dry run mode" node has disappeared from the recovery parameter
StringInfo, so added it back at the top of the parameters generated.
- Missing a newline after the include_if_exists.
- dirable_rename() logs already something on failure, I see no need
for an extra warning to say the same. Adding the warning telling that
a manual intervention may be required is good, though.
- Let's group the document change with the main patch.
- More stylistic changes, comments and code.
- The new test fails if we undo the changes in pg_createsubscriber.c,
as we'd want, in the shape of node K FATAL-ing due to an incorrect
recovery configuration fed to the node.
Alena, what do you think?
--
Michael
