On Fri, Oct 17, 2014 at 9:23 PM, Fujii Masao <masao.fujii@gmail.com> wrote: > On Thu, Oct 9, 2014 at 3:26 PM, Michael Paquier > <michael.paquier@gmail.com> wrote: >> >> >> On Wed, Oct 8, 2014 at 10:00 PM, Michael Paquier <michael.paquier@gmail.com> >> wrote: >>> >>> The additional process at promotion sounds like a good idea, I'll try to >>> get a patch done tomorrow. This would result as well in removing the >>> XLogArchiveForceDone stuff. Either way, not that I have been able to >>> reproduce the problem manually, things can be clearly solved. >> >> Please find attached two patches aimed to fix this issue and to improve the >> situation: >> - 0001 prevents the apparition of those phantom WAL segment file by ensuring >> that when a node is in recovery it will remove it whatever its status in >> archive_status. This patch is the real fix, and should be applied down to >> 9.2. >> - 0002 is a patch implementing Heikki's idea of enforcing all the segment >> files present in pg_xlog to have their status to .done, marking them for >> removal. When looking at the code, I finally concluded that Fujii-san's >> point, about marking in all cases as .done segment files that have been >> fully streamed, actually makes more sense to not be backward. This patch >> would actually not be mandatory for back-patching, but it makes the process >> more robust IMO. > > Thanks for the patches!
While reviewing the patch, I found another bug related to WAL file in recovery mode. The problem is that exitArchiveRecovery() always creates .ready file for the last WAL file of the old timeline even when it's restored from the archive and has .done file. So this causes the already-archived WAL file to be archived again.... Attached patch fixes this bug.
That's a good catch! Patch looks good. I think that you should change xlogpath to use MAXFNAMELEN instead of MAXPGPATH in exitArchiveRecovery. This is only for correctness, so that's a master-only remark, because this variable is used only to calculate a segment file name, and not a path. Renaming the variable from xlogpath to xlogname would make sense as well.