On Tue, Nov 26, 2013 at 5:33 PM, Rajeev rastogi
<rajeev.rastogi@huawei.com> wrote:
> On 26 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_controlvalues section.
So I suggest it is better to remove it from pg_control values section.
Few new comments:
1.
Formatting for 'OldestMulti's DB' and 'OldestXID's DB' is not
consistent with other values, try by printing it.
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.
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.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
