Обсуждение: typos in HS source code comment
Hi, When I was enjoying reading the HS source code, I found some typos. Attached patch fixes them. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
Вложения
On Thu, Aug 12, 2010 at 5:02 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > When I was enjoying reading the HS source code, > I found some typos. Attached patch fixes them. I've committed all of this except for the following, which I'm not certain is correct: --- a/src/backend/access/transam/clog.c +++ b/src/backend/access/transam/clog.c @@ -355,10 +355,10 @@ TransactionIdSetStatusBit(TransactionId xid, XidStatus sta /* * Update the group LSN ifthe transaction completion LSN is higher. * - * Note: lsn will be invalid when supplied during InRecovery processing, - * so we don't need to do anything special to avoid LSN updates during - * recovery. After recovery completes the next clog change will set the - * LSN correctly. + * Note: lsn will be invalid when supplied while we're performing + * recovery but hot standby is disabled, so we don't need to do + * anything special to avoid LSN updates in that case. After recovery + * completes the next clog change will set the LSN correctly. */ if (!XLogRecPtrIsInvalid(lsn)) { TransactionIdSetStatusBit is called from TransactionIdSetPageStatus, which seems to think that the validity of lsn is based on whether we're doing an async commit. Your change may be correct, but I'm not certain of it... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
On Fri, Aug 13, 2010 at 8:28 AM, Robert Haas <robertmhaas@gmail.com> wrote: > I've committed all of this except for the following, which I'm not > certain is correct: Thanks for the commit. > --- a/src/backend/access/transam/clog.c > +++ b/src/backend/access/transam/clog.c > @@ -355,10 +355,10 @@ TransactionIdSetStatusBit(TransactionId xid, XidStatus sta > /* > * Update the group LSN if the transaction completion LSN is higher. > * > - * Note: lsn will be invalid when supplied during InRecovery processing, > - * so we don't need to do anything special to avoid LSN updates during > - * recovery. After recovery completes the next clog change will set the > - * LSN correctly. > + * Note: lsn will be invalid when supplied while we're performing > + * recovery but hot standby is disabled, so we don't need to do > + * anything special to avoid LSN updates in that case. After recovery > + * completes the next clog change will set the LSN correctly. > */ > if (!XLogRecPtrIsInvalid(lsn)) > { > > TransactionIdSetStatusBit is called from TransactionIdSetPageStatus, > which seems to think that the validity of lsn is based on whether > we're doing an async commit. Your change may be correct, but I'm not > certain of it... Before 9.0, since xact_redo_commit always calls TransactionIdCommitTree, TransactionIdSetStatusBit always receives InvalidXLogRecPtr. In 9.0, xact_redo_commit calls TransactionIdCommitTree only when hot standby is disabled. OTOH, when hot standby is enabled, xact_redo_commit calls TransactionIdAsyncCommitTree, so TransactionIdSetStatusBit receives the valid lsn. Regards, PS. I'll be unable to read hackers from Aug 13th to 20th because of a vacation. -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
On Thu, Aug 12, 2010 at 9:37 PM, Fujii Masao <masao.fujii@gmail.com> wrote: > Before 9.0, since xact_redo_commit always calls TransactionIdCommitTree, > TransactionIdSetStatusBit always receives InvalidXLogRecPtr. In 9.0, > xact_redo_commit calls TransactionIdCommitTree only when hot standby is > disabled. OTOH, when hot standby is enabled, xact_redo_commit calls > TransactionIdAsyncCommitTree, so TransactionIdSetStatusBit receives the > valid lsn. Hmm. Maybe that should be spelled out a little more explicitly in the comment. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company