On Mon, Oct 20, 2014 at 5:15 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: > > > On Fri, Oct 17, 2014 at 10:12 PM, Fujii Masao <masao.fu...@gmail.com> wrote: >> >> On Fri, Oct 17, 2014 at 9:23 PM, Fujii Masao <masao.fu...@gmail.com> >> wrote: >> > On Thu, Oct 9, 2014 at 3:26 PM, Michael Paquier >> > <michael.paqu...@gmail.com> wrote: >> >> >> >> >> >> On Wed, Oct 8, 2014 at 10:00 PM, Michael Paquier >> >> <michael.paqu...@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.
Thanks for the review! Applied. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers