On Fri, 08 November 2013 09:47
> On Tue, Nov 5, 2013 at 3:20 PM, Rajeev rastogi
> <rajeev.rastogi@huawei.com> wrote:
> > On execution of pg_resetxlog using the option -n
> >
> > 1. It will display values in two section.
> >
> > 2. First section will be called as "Current
> pg_control
> > values or Guess pg_control values".
> >
> > 3. In first section, it will display all current (i.e.
> > before change) values of control file or guessed values.
> >
> > 4. Second section will be called as "Values to be
> used
> > after reset".
> >
> > 5. In second section, it will display new values of
> > parameters to be reset as per user request.
> >
> >
> >
> > Please provide your opinion or expectation out of this patch.
>
> Your approach in patch seems to be inline with Todo item. On a quick
> glance, I observed few things which can make your patch better:
>
> 1. The purpose was to print pg_control values in one section and any
> other reset values in different section, so in that
> regard, should we display below in new section, as here newXlogSegNo
> is not directly from pg_control.
>
> PrintControlValues()
> {
> ..
> XLogFileName(fname, ControlFile.checkPointCopy.ThisTimeLineID,
> newXlogSegNo);
>
> printf(_("First log segment after reset: %s\n"),
> fname);
> }
Yes we can print newXlogSegNo.
> 2. why to have extra flags for changedParam, can't we do without it.
> For example, we already use set_xid value to check if user has provided
> xid.
You mean to say that we can print wherever values of control file parameter is
getting assigned.
If yes, then the problem is that every places will have to check the condition as
if(noupdate) and then print the corresponding value. E.g. If (noupdate) printf(_("NextMultiXactId:%u\n"),
ControlFile.checkPointCopy.nextMulti);
> 3.
> + static void
> + PrintNewControlValues(int changedParam) {
> + if (changedParam)
> + printf(_("\n\nValues to be used after reset:\n\n"));
>
> Even after first if check fails, still you continue to check other
> values, it is better if you can have check if (changedParam) before
> calling this function
Yes you are correct.
But now this check will not be required because always at-least one value (i.e. of newXlogSegNo)
will be printed.
Please let me know if understanding of all comments are correct.
After that I will update the patch.
Thanks and Regards,
Kumar Rajeev Rastogi
