Re: [HACKERS] patch proposal

On 1/27/17 3:19 AM, Venkata B Nagothi wrote:

> I will be adding the tests in
> src/test/recovery/t/003_recovery_targets.pl
> <http://003_recovery_targets.pl>. My tests would look more or less
> similar to recovery_target_action. I do not see any of the tests added
> for the parameter recovery_target_action ? Anyways, i will work on
> adding tests for recovery_target_incomplete.

Do you know when those will be ready?

>
>
>     4) I'm not convinced that fatal errors by default are the way to go.
>     It's a pretty big departure from what we have now.
>
>
> I have changed the code to generate ERRORs instead of FATALs. Which is
> in the patch recoveryStartPoint.patch

I think at this point in recovery any ERROR at all will probably be
fatal.  Once you have some tests in place we'll know for sure.

Either way, the goal would be to keep recovery from proceeding and let
the user adjust their targets.  Since this is a big behavioral change
which will need buy in from the community.

>     Also, I don't think it's very intuitive how recovery_target_incomplete
>     works.  For instance, if I set recovery_target_incomplete=promote and
>     set recovery_target_time, but pick a backup after the specified time,
>     then there will be an error "recovery_target_time %s is older..." rather
>     than a promotion.
>
>
> Yes, that is correct and that is the expected behaviour. Let me explain -

My point was that this combination of options could lead to confusion on
the part of users.  However, I'd rather leave that alone for the time
being and focus on the first patch.

> I have split the patch into two different
> pieces. One is to determine if the recovery can start at all and other
> patch deals with the incomplete recovery situation.

I think the first patch looks promising and I would rather work through
that before considering the second patch.  Once there are tests for the
first patch I will complete my review.