Re: 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(dot)fujii(at)gmail(dot)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 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...

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On Fri, Aug 13, 2010 at 8:28 AM, Robert Haas <robertmhaas(at)gmail(dot)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(dot)fujii(at)gmail(dot)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