Re: track_commit_timestamp and COMMIT PREPARED

On 2015-09-29 13:44, Fujii Masao wrote:
> On Tue, Sep 29, 2015 at 12:05 PM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
>> Petr Jelinek wrote:
>>> On 2015-09-02 16:14, Fujii Masao wrote:
>>>> On Wed, Aug 5, 2015 at 2:16 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>>>>> On Mon, Aug 3, 2015 at 10:31 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>>>>> track_commit_timestamp tracks COMMIT PREPARED as expected in standby server,
>>>>>> but not in master server. Is this intentional? It should track COMMIT PREPARED
>>>>>> even in master? Otherwise, we cannot use commit_timestamp feature to check
>>>>>> the replication lag properly while we use 2PC.
>>>>>
>>>>> That sounds like it must be a bug.  I think you should add it to the
>>>>> open items list.
>>>
>>> Attached fixes this. It includes advancement of replication origin as well.
>>> I didn't feel like doing refactor of commit code this late in 9.5 cycle
>>> though, so I went with the code duplication + note in xact.c.
>>
>> Thanks, your proposed behavior looks reasonable.  I didn't like the
>> existing coding nor the fact that with your patch we'd have two copies
>> of it, so I changed a bit instead to be more understandable.  Hopefully I
>> didn't break too many things.  This patch includes the patch for the
>> other commitTS open item too.
>
> -#define RecoveryRequiresBoolParameter(param_name, currValue, masterValue) \
> -do { \
> -    bool _currValue = (currValue); \
> -    bool _masterValue = (masterValue); \
> -    if (_currValue != _masterValue) \
> -        ereport(ERROR, \
> -                (errcode(ERRCODE_INVALID_PARAMETER_VALUE), \
> -                 errmsg("hot standby is not possible because it
> requires \"%s\" to be same on master and standby (master has \"%s\",
> standby has \"%s\")", \
> -                        param_name, \
> -                        _masterValue ? "true" : "false", \
> -                        _currValue ? "true" : "false"))); \
> -} while(0)
>
> This code should not be deleted because there is still the caller of
> the macro function.
>

Looks like Alvaro didn't merge the second patch correctly, the only 
caller should have been removed as well.

--  Petr Jelinek                  http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services