Re: Possible missing segments in archiving on standby

At Tue, 31 Aug 2021 01:54:36 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in 
> 
> 
> On 2020/06/30 16:55, Kyotaro Horiguchi wrote:
> > Hello.
> > While looking a patch, I found that a standby with archive_mode=always
> > fails to archive segments under certain conditions.
> 
> I encountered this issue, too.
> 
> 
> > 1. v1-0001-Make-sure-standby-archives-all-segments.patch:
> >    Fix for A and B.
> > 2. v1-0001-Make-sure-standby-archives-all-segments-immediate.patch:
> >    Fix for A, B and C.
> 
> You proposed two patches, but this patch should be reviewed
> preferentially
> because this addresses all the issues (i.e., A, B and C) that you
> reported?

Maybe. The point here was whether we regard C as an issue, but now I
think it is an issue.

> + * If we are starting streaming at the beginning of a segment,
> + * there may be the case where the previous segment have not been
> +			 * archived yet.  Make sure it is archived.
> 
> Could you clarify why the archive notification file of the previous
> WAL segment needs to be checked?
> 
> As far as I read the code, the cause of the issue seems to be that
> XLogWalRcvWrite() exits without creating an archive notification file
> even if the current WAL segment is fully written up in the last cycle
> of
> XLogWalRcvWrite()'s loop. That is, creation of the notification file
> and WAL archiving of that completed segment will be delayed
> until any data in the next segment is received and written (by next
> call
> to XLogWalRcvWrite()). Furthermore, in that case, if walreceiver exits
> before receiving such next segment, the completed current segment
> fails to be archived as Horiguchi-san reported.

Right.  Finally such segments are archived when a future checkpoint
removes them. In that sense the patch works to just let archiving
happens faster, but on the other hand I came to think we are supposed
to archive a segment as soon as it is completed. (That is, I think C
is a problem.)

> Therefore, IMO that the simple approach to fix the issue is to create
> an archive notification file if possible at the end of
> XLogWalRcvWrite().
> I implemented this idea. Patch attached.

I'm not sure which is simpler, but it works except for B, the case of
a long-jump by a segment switch.  When a segment switch happens,
walsender sends filling zero-pages but even if walreceiver is
terminated before the segment is completed, walsender restarts from
the next segment at the next startup. Concretely like the following.

- pg_switch_wal() invoked at 6003228 (for example)
- walreceiver terminates at 6500000 (or a bit later).
- walrecever rstarts from 7000000

In this case the segment 6 is not notified even with the patch, and my
old patches works the same way. (In other words, the call to
XLogWalRcvClose() at the end of XLogWalRcvWrite doens't work for the
case as you might expect.) If we think it ok that we don't notify the
segment earlier than a future checkpoint removes it, yours or only the
last half of my one is sufficient, but do we really think so?
Furthermore, your patch or only the last half of my second patch
doesn't save the case of a crash unlike the case of a graceful
termination.


The attached files are:

v2wip-0001-Make-sure... : a rebased patch of the old second patch
repro_helper.diff       : reproducer helper patch, used by the script below.
repro.sh                : reproducer script.

(The second diff conflicts with the first patch. Since the second just
inserts a single code block, it is easily applicable manually:p)

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center