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-2...@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

Attachment: pg_resetxlogsectionV4.patch
Description: pg_resetxlogsectionV4.patch

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to