Thread: Re: [COMMITTERS] pgsql: Ensure that top level aborts call XLogSetAsyncCommit().

On Thu, May 13, 2010 at 8:39 PM, Simon Riggs <sriggs@postgresql.org> wrote:
> Log Message:
> -----------
> Ensure that top level aborts call XLogSetAsyncCommit(). Not doing
> so simply leads to data waiting in wal_buffers which then causes
> later commits to potentially do emergency writes and for all forms
> of replication to be potentially delayed without need or benefit.
> Issue pointed out exactly by Fujii Masao, following bug report
> by Robert Haas on a separate though related topic.
>
> Modified Files:
> --------------
>    pgsql/src/backend/access/transam:
>        xact.c (r1.290 -> r1.291)
>        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/transam/xact.c?r1=1.290&r2=1.291)

This commit changed XLogSetAsyncCommitLSN() so that it's called
for abort case. So we need to change the comment of the function
as follows:

diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index c886571..ae10108 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1860,7 +1860,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible,
bool xlog_switch)
/* * Record the LSN for an asynchronous transaction commit.
- * (This should not be called for aborts, nor for synchronous commits.)
+ * (This should not be called for synchronous commits.) */voidXLogSetAsyncCommitLSN(XLogRecPtr asyncCommitLSN)


Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Mon, 2010-05-31 at 20:11 +0900, Fujii Masao wrote:
> On Thu, May 13, 2010 at 8:39 PM, Simon Riggs <sriggs@postgresql.org> wrote:
> > Log Message:
> > -----------
> > Ensure that top level aborts call XLogSetAsyncCommit(). Not doing
> > so simply leads to data waiting in wal_buffers which then causes
> > later commits to potentially do emergency writes and for all forms
> > of replication to be potentially delayed without need or benefit.
> > Issue pointed out exactly by Fujii Masao, following bug report
> > by Robert Haas on a separate though related topic.
> >
> > Modified Files:
> > --------------
> >    pgsql/src/backend/access/transam:
> >        xact.c (r1.290 -> r1.291)
> >        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/transam/xact.c?r1=1.290&r2=1.291)
> 
> This commit changed XLogSetAsyncCommitLSN() so that it's called
> for abort case. So we need to change the comment of the function
> as follows:

Agreed, will fix.

Will also rename function to better document its new role.

-- Simon Riggs           www.2ndQuadrant.com

On Mon, May 31, 2010 at 8:48 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On Mon, 2010-05-31 at 20:11 +0900, Fujii Masao wrote:
>> On Thu, May 13, 2010 at 8:39 PM, Simon Riggs <sriggs@postgresql.org> wrote:
>> > Log Message:
>> > -----------
>> > Ensure that top level aborts call XLogSetAsyncCommit(). Not doing
>> > so simply leads to data waiting in wal_buffers which then causes
>> > later commits to potentially do emergency writes and for all forms
>> > of replication to be potentially delayed without need or benefit.
>> > Issue pointed out exactly by Fujii Masao, following bug report
>> > by Robert Haas on a separate though related topic.
>> >
>> > Modified Files:
>> > --------------
>> >    pgsql/src/backend/access/transam:
>> >        xact.c (r1.290 -> r1.291)
>> >        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/transam/xact.c?r1=1.290&r2=1.291)
>>
>> This commit changed XLogSetAsyncCommitLSN() so that it's called
>> for abort case. So we need to change the comment of the function
>> as follows:
>
> Agreed, will fix.
>
> Will also rename function to better document its new role.

New function name occurred to you?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Fri, 2010-07-16 at 17:03 +0900, Fujii Masao wrote:

> >> This commit changed XLogSetAsyncCommitLSN() so that it's called
> >> for abort case. So we need to change the comment of the function
> >> as follows:
> >
> > Agreed, will fix.
> >
> > Will also rename function to better document its new role.
> 
> New function name occurred to you?

Thanks for the ping, though not forgotten. 

I expect to make the change for next beta.

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Development, 24x7 Support, Training and Services