|From:||Venkata B Nagothi <nag1010(at)gmail(dot)com>|
|To:||David Steele <david(at)pgmasters(dot)net>|
|Cc:||Stephen Frost <sfrost(at)snowman(dot)net>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: patch proposal|
|Views:||Raw Message | Whole Thread | Download mbox|
On Wed, Mar 1, 2017 at 1:14 AM, Venkata B Nagothi <nag1010(at)gmail(dot)com> wrote:
> Hi David,
> On Tue, Jan 31, 2017 at 6:49 AM, David Steele <david(at)pgmasters(dot)net> wrote:
>> 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?
> Attached are both the patches with tests included.
>> > 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.
> Hoping to get some comments / feedback from the community on the second
> patch too.
> > Also, I don't think it's very intuitive how
>> > works. For instance, if I set recovery_target_incomplete=promote
>> > set recovery_target_time, but pick a backup after the specified
>> > then there will be an error "recovery_target_time %s is older..."
>> > 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.
> I have added all the tests for the second patch and all seem to be working
> Regarding the first patch, i have included all the tests except for the
> test case on recovery_target_time.
> I did write the test, but, the test is kind of behaving weird which i am
> working through to resolve.
This issue has been resolved. All good now. To avoid any further confusion,
i have attached the latest versions (4) of both the patches with all the
I have already changed the patch status to "Needs review".
Thank you !
Venkata B N
|Next Message||Rushabh Lathia||2017-03-01 05:00:09||Re: [POC] hash partitioning|
|Previous Message||Alvaro Herrera||2017-03-01 04:58:23||brin autosummarization -- autovacuum "work items"|