Re: PATCH: add pg_current_xlog_flush_location function - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: PATCH: add pg_current_xlog_flush_location function
Date
Msg-id CAA4eK1KHwN0Y29kc=OiKSeOwYPCZL7Xnyr6J5iV+XYYQ_-Pepg@mail.gmail.com
Whole thread Raw
In response to Re: PATCH: add pg_current_xlog_flush_location function  (Tomas Vondra <tomas.vondra@2ndquadrant.com>)
Responses Re: PATCH: add pg_current_xlog_flush_location function  (Michael Paquier <michael.paquier@gmail.com>)
List pgsql-hackers
On Mon, Jan 11, 2016 at 3:29 AM, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
Hi,

On 12/13/2015 08:38 PM, Tomas Vondra wrote:
Hi,

On 12/13/2015 06:13 AM, Amit Kapila wrote:
 >
...
 >
Is there a reason why you can't use existing function
GetFlushRecPtr() in xlog.c?

No, not really. I think I somehow missed that function when writing
the initial version of the patch. Will fix in v2 of the patch.

Hmm, so I've been looking at this, and I've realized that I've written it like this because that's pretty much what pg_current_xlog_location() does. It calls GetXLogWriteRecPtr which does this:

/*
 * Get latest WAL write pointer
 */
XLogRecPtr
GetXLogWriteRecPtr(void)
{
        SpinLockAcquire(&XLogCtl->info_lck);
        LogwrtResult = XLogCtl->LogwrtResult;
        SpinLockRelease(&XLogCtl->info_lck);

        return LogwrtResult.Write;
}

so the patch does the same thing, except that I've returned "Flush".

OTOH GetFlushRecPtr does this:

XLogRecPtr
GetFlushRecPtr(void)
{
        XLogRecPtr      recptr;

        SpinLockAcquire(&XLogCtl->info_lck);
        recptr = XLogCtl->LogwrtResult.Flush;
        SpinLockRelease(&XLogCtl->info_lck);

        return recptr;
}

i.e. it does not update LogwrtResult, the local private copy. Not sure what's appropriate here ...


I think for the purpose of exposing the new API
pg_current_xlog_flush_location(), I see no reason why it has to
update the local variable LogwrtResult, although doing it either way
seems to be okay, however introducing new function
GetXLogFlushRecPtr() seems redundant.  The internal function
(GetXLogInsertRecPtr()) used for API pg_current_xlog_insert_location()
doesn't update the local variable which indicates that calling the existing
function GetFlushRecPtr() is sufficient for pg_current_xlog_flush_location().


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: ExecGather() + nworkers
Next
From: Amit Kapila
Date:
Subject: Re: Why doesn't src/backend/port/win32/socket.c implement bind()?