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(at)2ndquadrant(dot)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(at)gmail(dot)com> wrote:
>>>>> On Mon, Aug 3, 2015 at 10:31 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)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