On Tue, Sep 26, 2023 at 06:44:50PM +0300, Aleksander Alekseev wrote: >> And also, I'm afraid that I'm not sure what kind of tests I have to make >> for fix this behavior. Would you mind giving me some advice? > > Personally I would prefer not to increase the scope of work. Your TAP > test added in 0001 seems to be adequate.
Yeah, agreed. I'm OK with what you are proposing, basically (the test could be made a bit cheaper actually).
I guess you meant that it contains an unnecessary insert and wait. I fixed this and some incorrect comments caused by copy & paste.
Please see the attached patch.
/* - * For Hot Standby, we could treat this like a Shutdown Checkpoint, - * but this case is rarer and harder to test, so the benefit doesn't - * outweigh the potential extra cost of maintenance. + * For Hot Standby, we could treat this like an end-of-recovery checkpoint */ + RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_WAIT);
I don't understand what you want to change here. Archive recovery and crash recovery are two different things, still this code would be triggered even if there is no standby.signal, aka the node is not a standby. Why isn't this stuff conditional?
You are absolutely right. It should only run in standby mode. Also, according to the document[1], a server can be "Hot Standby" even if it is
not in standby mode (i.e. when it is in archive recovery mode). So I fixed the comment above `RequestCheckpoint()`.