On Sun, Jun 12, 2016 at 12:46 AM, David Steele <david@pgmasters.net> wrote:
> On 6/11/16 8:22 AM, Michael Paquier wrote:
>> On Sat, Jun 11, 2016 at 9:44 AM, Thom Brown <thom@linux.com> wrote:
>>> It may be the wrong way of going about it, but you get the idea of what I'm
>>> suggesting we output instead.
>>
>> Surely things could be better. So +1 to be more verbose here.
>>
>> + if (recoveryStopTime == 0)
>> + snprintf(reason, sizeof(reason),
>> + "recovery reached consistency before recovery
>> target time of \"%s\"\n",
>> + timestamptz_to_str(recoveryTargetTime));
>> "Reaching consistency" is not exact for here. I'd rather say "finished
>> recovery without reaching target blah"
>>
>> + if (recoveryStopXid == 0)
>> Checking for InvalidTransactionId is better here.
>>
>> And it would be good to initialize recoveryStopTime and
>> recoveryStopXid as those are set only when a recovery target is
>> reached.
>
> +1 for Michael's wording. It's not very clear in the logs right now if
> a recovery target was missed. That makes it very difficult for the user
> to determine if PITR worked or not.
By the way, we surely want to have the same logic for a recovery
target name. It could be possible to reach the end of recovery without
reaching the goal of recovery.conf. It would be good to be verbose in
this case as well by checking for recoveryStopName[0] = '\0'.
--
Michael