Hi, I have looked at the patch and it looks OK to me. BTW I am not too much familiar with this area of code, so I am not at the position to argue that patch -:) . I haven't found an easy way to test the patch.
On Wed, Dec 3, 2008 at 1:24 PM, Heikki Linnakangas <[EMAIL PROTECTED]> wrote: > 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 > -- Ibrar Ahmed 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
