Re: TODO: Split out pg_resetxlog output into pre- and post-sections - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: TODO: Split out pg_resetxlog output into pre- and post-sections
Date
Msg-id CAA4eK1Jmc4sm0XHBhJFrp1aLJ6T4kFaiGo0BUos-MT6YgvsExQ@mail.gmail.com
Whole thread Raw
In response to Re: TODO: Split out pg_resetxlog output into pre- and post-sections  (Rajeev rastogi <rajeev.rastogi@huawei.com>)
List pgsql-hackers
On Fri, Nov 8, 2013 at 10:37 AM, Rajeev rastogi
<rajeev.rastogi@huawei.com> wrote:
> 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
>> >
>> > 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.
  I think then your documentation also need updates.

>> 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);  No, not this way,
maybe making set_xid as file level parameters,
 
so that you can use them later as well.  Your current way also doesn't seem to be too unreasonable, however
it is better if you can do without it.

One more thing, I think as per this patch few parameters will be
displayed twice once in "pg_control values .." section and once in
"Values to be used after reset:", so by doing this I guess you want to
make it easier for user to refer both pg_control's original/guessed
value and new value after reset. Here I wonder if someone wants to
refer to original values, can't he directly use pg_controldata? Anyone
else have thoughts about how can we display values which can make
current situation better for user.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: unaccent module - two params function should be immutable
Next
From: David Rowley
Date:
Subject: "Fix pg_isolation_regress to work outside its build directory" compiler warning