On 3 December 2013 18:46, Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Dec 3, 2013 at 12:36 PM, Fabrízio de Royes Mello > <fabriziomello@gmail.com> wrote: >> On Tue, Dec 3, 2013 at 2:33 PM, Christian Kruse <christian@2ndquadrant.com> >> wrote: >>> >>> Hi Fabrizio, >>> >>> looks good to me. I did some testing on 9.2.4, 9.2.5 and HEAD. It >>> applies and compiles w/o errors or warnings. I set up a master and two >>> hot standbys replicating from the master, one with 5 minutes delay and >>> one without delay. After that I created a new database and generated >>> some test data: >>> >>> CREATE TABLE test (val INTEGER); >>> INSERT INTO test (val) (SELECT * FROM generate_series(0, 1000000)); >>> >>> The non-delayed standby nearly instantly had the data replicated, the >>> delayed standby was replicated after exactly 5 minutes. I did not >>> notice any problems, errors or warnings. >>> >> >> Thanks for your review Christian... > > So, I proposed this patch previously and I still think it's a good > idea, but it got voted down on the grounds that it didn't deal with > clock drift. I view that as insufficient reason to reject the > feature, but others disagreed. Unless some of those people have > changed their minds, I don't think this patch has much future here.
I had that objection and others. Since then many people have requested this feature and have persuaded me that this is worth having and that my objections are minor points. I now agree with the need for the feature, almost as written.
Not recalling the older thread, but it seems the "breaks on clock drift", I think we can fairly easily make that situation "good enough". Just have IDENTIFY_SYSTEM return the current timestamp on the master, and refuse to start if the time difference is too great. Yes, that doesn't catch the case when the machines are in perfect sync when they start up and drift *later*, but it will catch the most common cases I bet. But I think that's good enough that we can accept the feature, given that *most* people will have ntp, and that it's a very useful feature for those people. But we could help people who run into it because of a simple config error..
Or maybe the suggested patch already does this, in which case ignore that part :)