At Tue, 24 Jul 2018 10:41:18 +0900, Michael Paquier <mich...@paquier.xyz> wrote in <20180724014118.ga4...@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
