Fujii Masao wrote:
On Wed, Dec 3, 2008 at 5:13 AM, Heikki Linnakangas
<[EMAIL PROTECTED]> 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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to