Re: PATCH: add pg_current_xlog_flush_location function

On 11 January 2016 at 12:01, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
wrote:

>
> On 01/11/2016 06:30 AM, Michael Paquier wrote:
>
>>
>> Updating LogwrtResult directly when calling your new function
>> GetXLogFlushRecPtr() does not strike me as a particularly good idea
>> per this portion in XLogFlush():
>>
> >
>
>> /* Quick exit if already known flushed */
>> if (record <= LogwrtResult.Flush)
>> return;
>>
>> The same counts for GetXLogWriteRecPtr, we had better allocate the
>> value in an independent variable as there are checks using it. For
>> now it does not matter much for the write position because all the
>> code paths doing the checks explicitly update again the pointer
>> before looking at it but it seems to me that using an independent
>> variable would make the code more robust.
>>
>
> Why? LogwrtResult only serves as a local cache of shared values, so there
> should be no danger of skipping something.

Comments in xlog.c say

"In addition to the shared variable, each backend has a private copy of
LogwrtResult, which is updated when convenient."

It is therefore valid to update the value of both Write and Flush positions
at the same time, any time either is required.

My suggested commit pattern for this is...

1. Update existing function to maintain LogwrtResult more eagerly (separate
patch)

2. Have the patch use the existing function name (main patch)

--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services