Re: pg_rewind WAL segments deletion pitfall

On Tue, Sep 27, 2022 at 9:50 AM Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
wrote:

> Regarding the the patch, pg_rewind starts reading segments from the
> divergence point back to the nearest checkpoint, then moves foward
> during rewinding. So, the fact that SimpleXLogPageRead have read a
> segment suggests that the segment is required during the next startup.
> So I don't think we need to move around the keepWalSeg flag. All
> files that are wanted while rewinding should be preserved
> unconditionally.
>

I am probably not getting this right but as far as I see SimpleXLogPageRead
is called at most 3 times during pg_rewind run:
1. From readOneRecord to determine the end-of-WAL on the target by reading
the last shutdown checkpoint record/minRecoveryPoint on it
2. From findLastCheckpoint to find last common checkpoint (here it
indeed reads all the segments that are required during the startup, hence
the keepWalSeg flag set to true)
3. From extractPageMap to extract all the pages modified after the fork
(here we also read all the segments that should be kept but also the ones
further, until the target's end record. Doesn't seem we should
unconditionally preserve them all).
Am I missing something?

> + /*
> + * Some entries (WAL segments) already have an action
> assigned
> + * (see SimpleXLogPageRead()).
> + */
> + if (entry->action == FILE_ACTION_NONE)
> + continue;
> entry->action = decide_file_action(entry);

It might be more reasonable to call decide_file_action() when action
> is UNDECIDED.
>

Agree, will change this part.

Regards,
Polina Bungina