Re: TODO: Split out pg_resetxlog output into pre- and post-sections

On 28 November 2013, Amit Kapila Wrote:
> >> Further Review of this patch:
> >> b. why to display 'First log segment after reset' in 'Currrent
> >> pg_control values' section as now the same information
> >> is available in new section "Values to be used after reset:" ?
> >
> > May not be always. Suppose if user has given new value of seg no and
> TLI, then it will be different.
> > Otherwise it will be same.
> > So now I have changed the code in such way that the value of XLOG
> > segment # read from checkpoint record gets printed as part of current
> > value and any further new value gets printed in Values to be reset
> > (This will be always at-least one plus the current value). Because of
> following code in FindEndOfXLOG
> > xlogbytepos = newXlogSegNo *
> ControlFile.xlog_seg_size;
> > newXlogSegNo = (xlogbytepos + XLogSegSize - 1)
> / XLogSegSize;
> > newXlogSegNo++;
>
> It can be different, but I don't think we should display it in both
> sections because:
> a. it doesn't belong to control file.
> b. if you carefully look at original link
> (http://www.postgresql.org/message-id/1283277511-sup-2152@alvh.no-
> ip.org),
> these values are not getting displayed in pg_control values section.
>
> So I suggest it is better to remove it from pg_control values section.

Done as per suggestion.

>
> Few new comments:
>
> 1.
> Formatting for 'OldestMulti's DB' and 'OldestXID's DB' is not
> consistent with other values, try by printing it.

Changed to align output.

> 2.
> + It will print values in two sections. In first section it will
> print all original/guessed values
> + and in second section all values to be used after reset. In
> second section filename will be
> + always printed as segment number will be at-least one more than
> current. Also if any other parameter
> + is given using appropriate switch, then those new values will be
> also printed.
>
> a. the length of newly added lines is not in sync with previous text,
> try to keep length less than 80 chars.
> b. I think there is no need of text (In second section ....), you can
> remove all text after that point.

Done.

> 3.
> ! printf(_(" -n no update, just show extracted control
> values (for testing) and to be reset values of parameters(if
> given)\n")); I find this line too long and elaborative, try to make it
> shorter.

Changed accordingly.

Please provide your opinion.

Thanks and Regards,
Kumar Rajeev Rastogi