Re: pg_stop_backup wait bug fix

Fujii Masao wrote:
> On Wed, Dec 3, 2008 at 5:13 AM, Heikki Linnakangas
> <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
>> Agreed, should use XLByteToPrevSeg. But I wonder if we can just replace the
>> current XLByteToSeg call with XLByteToPrevSeg? That would offset the return
>> value of the function by one byte as well, as well as the value printed to
>> the backup history file. In fact, I think the original patch got that wrong;
>> it would return the location of the *beginning* of the last xlog file.
>
> You're right. As you say, the value (stopxlogfilename) printed to the backup
> history file is wrong. But, since the value is not used fortunately,
> any troubles
> have not come up. So, I think that we can just replace them.

Changing the return value doesn't seem like a good idea. If nothing
else, it would be complicated to explain what it returns. I committed a
patch that changes the waiting behavior, but not the return value or
what's written into the backup label file,

>> I also noticed that the 2nd BackupHistoryFileName call in that function is
>> useless; histfilepath variable is already filled in earlier.
>
> Somewhat confusingly, BackupHistoryFileName is called only once. Isn't 1st
> (which probably you thought) BackupHistoryFilePath?

Ouch, you're right. That's subtle.

> In order to prevent
> confusion, we should add new local variable (histfilename) for the backup
> history file name?

Agreed. I included that in the patch.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com