On 26 November 2013, Amit Kapila Wrote: > Further Review of this patch: > a. there are lot of trailing whitespaces in patch.
Fixed.
> 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++;
> c. I think it is better to display 'First log segment after reset' as
> file name as it was earlier.
> This utility takes filename as input, so I think we should display
> filename.
Fixed. Printing filename.
> d. I still think it is not good idea to use new parameter changedParam
> to detect which parameters are changed and the reason is
> code already has that information. We should try to use that
> information rather introducing new parameter to mean the same.
Fixed. Removed changedParam and made existing variables like set_xid
global and same is being used for check.
> e.
> if (guessed && !force)
> {
> PrintControlValues(guessed);
> if (!noupdate)
> {
> printf(_("\nIf these values seem acceptable, use -f to force
> reset.\n")); exit(1); }
>
> Do you think there will be any chance when noupdate can be true in
> above loop, if not then why to keep the check for same?
Fixed along with the last comment.
> f.
> if (changedParam & DISPLAY_XID)
> {
> printf(_("NextXID: %u\n"),
> ControlFile.checkPointCopy.nextXid);
> printf(_("oldestXID: %u\n"),
> ControlFile.checkPointCopy.oldestXid);
> }
> Here you are priniting oldestXid, but not oldestXidDB, if user provides
> xid both values are changed. Same is the case for multitransaction.
Fixed.
> g.
> /* newXlogSegNo will be always printed unconditionally*/ Extra space
> between always and printed. In single line comments space should be
> provided when comment text ends, please refer other single line
> comments.
Fixed.
> In case when the values are guessed and -n option is not given, then
> this patch will not be able to distinguish the values. Don't you think
> it is better to display values in 2 sections in case of guessed values
> without -n flag as well, otherwise this utility will have 2 format's to
> display values?
Yes you are right. Now printing the values in two section in two cases:
a. -n is given or
b. If we had to guess and -f not given.
Please let know your opinion.
Thanks and Regards,
Kumar Rajeev Rastogi
pg_resetxlogsectionV3.patch
Description: pg_resetxlogsectionV3.patch
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
