On Wed, Mar 1, 2017 at 1:14 AM, Venkata B Nagothi <nag1...@gmail.com> wrote:
> Hi David, > > On Tue, Jan 31, 2017 at 6:49 AM, David Steele <da...@pgmasters.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 >> 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. >> > > I have added all the tests for the second patch and all seem to be working > fine. > > 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 tests included. I have already changed the patch status to "Needs review". Thank you ! Regards, Venkata B N Database Consultant
recoveryStartPoint.patch-version4
Description: Binary data
recoveryTargetIncomplete.patch-version4
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
