At Tue, 24 Jul 2018 10:41:18 +0900, Michael Paquier <michael@paquier.xyz> wrote in <20180724014118.GA4035@paquier.xyz>
> On Mon, Jul 23, 2018 at 07:55:57PM +0900, Kyotaro HORIGUCHI wrote:
> With your patch applied I see one segment recycled after each
> checkpoint, which is correct. On HEAD, you would see no segments,
> followed by 2 segments recycled. But I also see sometimes one segment
> recycled.
Anyway good to hear that.
> >> The calculation of _logSegNo in CreateRestartPoint is visibly incorrect,
> >> this still uses PriorRedoPtr so the bug is not fixed for standbys. The
> >> tweaks for ThisTimeLineID also need to be out of the loop where
> >> PriorRedoPtr is InvalidXLogRecPtr, and only the distance calculation
> >> should be kept.
> >
> > Agreed. While I reconsidered this, I noticed that the estimated
> > checkpoint distance is 0 when PriorRedoPtr is invalid. This means
> > that the first checkpoint/restartpoint tries to reduce WAL
> > segments to min_wal_size. However, it happens only at initdb time
> > and makes no substantial behavior change so I made the change
> > ignoring the difference.
>
> Yes, same analysis here.
>
> >> 1) and 2) are in my opinion clearly oversights from 4b0d28d, but 3) is
> >> not.
> >
> > Thank you for the comments and suggestions. After all, I did the
> > following things in the attached patch.
> >
> > - Reverted the change on timeline switching. (Removed the (3))
> > - Fixed CreateRestartPoint to do the same thing with CreateCheckPoint.
> > - Both CreateRestart/CheckPoint now tries trimming of WAL
> > segments even for the first time.
>
> Thanks, pushed after some comment tweaks and fixing the variable names
> at the top of xlog.c for the static declarations. Perhaps we can do
> more refactoring here by moving all the segment calculation logic
> directly in RemoveOldXlogFiles, but that makes the end LSN calculation a
> bit blurry so I kept things as you proposed in version 3 of the patch.
Thank you for committing this.
I feel that XLOGfileslop() can be a function but I regret that I
didn't move it closer to RemoveOldXLogFiles a bit..
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center