Re: Review: Patch to compute Max LSN of Data Pages - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: Review: Patch to compute Max LSN of Data Pages
Date
Msg-id CAA4eK1LM8b3ZU8=2TD+yg=nbsAxBp-zU3BDS+r1jxPcjcj9VEQ@mail.gmail.com
Whole thread Raw
In response to Re: Review: Patch to compute Max LSN of Data Pages  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
On Wed, Oct 9, 2013 at 11:58 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Sat, Sep 14, 2013 at 3:03 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>>On Monday, July 08, 2013 5:16 PM Andres Freund wrote:
>>>>On 2013-07-08 17:10:43 +0530, Amit Kapila wrote:
>>>>> On Monday, July 08, 2013 4:26 PM Andres Freund wrote:
>>>>> > On 2013-07-08 16:17:54 +0530, Hari Babu wrote:
>>>>> > > +    This utility can also be used to decide whether backup is
>>>>> > required or not when the data page
>>>>> > > +    in old-master precedes the last applied LSN in old-standby
>>>>> > (i.e., new-master) at the
>>>>> > > +    moment of the failover.
>>>>> > > +   </para>
>>>>> > > +  </refsect1>
>>>>> >
>>>>> > I don't think this is safe in any interesting set of cases. Am I
>>>>> > missing
>>>>> > something?
>>>>>
>>>>> No, you are not missing anything. It can be only used to find max LSN in
>>>>> database which can avoid further corruption
>>
>>>>Why is the patch submitted documenting it as a use-case then? I find it
>>>>rather scary if the *patch authors* document a known unsafe use case as
>>>>one of the known use-cases.
>>
>>>I got the problem which can occur with the specified use case. Removed the
>>>wrong use case specified above.
>>>Thanks for the review, please find the updated patch attached in the mail.
>>
>> Patch is not getting compiled on Windows, I had made following changes:
>> a. updated the patch for resolving Windows build
>> b. few documentation changes in (pg_resetxlog.sgml) for spelling
>> mistake and minor line change
>> c. corrected year for Copyright in file pg_computemaxlsn.c
>
> I am OK with this patch in its current form, modulo some grammar
> issues in the documentation which I can fix before committing.
> However, I'm unclear whether there was sufficient consensus to proceed
> with this.
  I would like to summarize in short about this patch.

This idea started with me proposing some solutions for cases where
user can recover from some of corruption scenario's, then Tom Lane had
suggested
this idea and I prepared a Patch for it, then you have given
suggestions about this patch at various phases during development.
Both Fujji Masao and Alvaro seems to be in favor of use case and
Alvaro has given few suggestions for patch as well. Muhammad Usama had
reviewed this patch.

It appears to me that Andres is not in favour of this Patch.

So, I think there are good number of people who had participated in
this patch and were in favour of this patch.

The above summarization is from what I remember about this Patch, so
if anybody feels that I have misunderstood, then kindly correct me.

> Can others weigh in?  If there is too much residual
> unhappiness with this, then we should just mark this as Rejected and
> stop wasting time on it; it can be pushed to PGXN or similar even if
> we don't put it in core.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com



pgsql-hackers by date:

Previous
From: Pavan Deolasee
Date:
Subject: Re: Patch for fail-back without fresh backup
Next
From: Magnus Hagander
Date:
Subject: Re: Auto-tuning work_mem and maintenance_work_mem