Fujii Masao wrote:
> On Wed, Dec 3, 2008 at 5:13 AM, Heikki Linnakangas
> <heikki.linnakangas@enterprisedb.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