Re: BUG: *FF WALs under 9.2 (WAS: .ready files appearing on slaves)

On Mon, Oct 20, 2014 at 5:15 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
>
>
> On Fri, Oct 17, 2014 at 10:12 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>
>> On Fri, Oct 17, 2014 at 9:23 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
>> wrote:
>> > On Thu, Oct 9, 2014 at 3:26 PM, Michael Paquier
>> > <michael(dot)paquier(at)gmail(dot)com> wrote:
>> >>
>> >>
>> >> On Wed, Oct 8, 2014 at 10:00 PM, Michael Paquier
>> >> <michael(dot)paquier(at)gmail(dot)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