Re: Re: In-core regression tests for replication, cascading, archiving, PITR, etc.

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Amir Rohan <amir(dot)rohan(at)zoho(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)2ndquadrant(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, Greg Smith <gsmith(at)gregsmith(dot)com>
Subject: Re: Re: In-core regression tests for replication, cascading, archiving, PITR, etc.
Date: 2015-12-21 07:45:21
Message-ID: CAB7nPqTEqou=xrYrGSgA13QW1xxsSD6tFHz-Sm_J3EgDvSOCHw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Dec 12, 2015 at 8:29 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Sat, Dec 12, 2015 at 11:37 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
>> On Fri, Dec 11, 2015 at 09:34:34PM +0900, Michael Paquier wrote:
>>> On Fri, Dec 11, 2015 at 8:48 PM, Alvaro Herrera
>>> <alvherre(at)2ndquadrant(dot)com> wrote:
>>> > Michael Paquier wrote:
>>> >> On Fri, Dec 11, 2015 at 5:35 AM, Alvaro Herrera
>>> >> <alvherre(at)2ndquadrant(dot)com> wrote:
>>> >> I guess that to complete your idea we could allow PostgresNode to get
>>> >> a custom name for its log file through an optional parameter like
>>> >> logfile => 'myname' or similar. And if nothing is defined, process
>>> >> falls back to applname. So this would give the following:
>>> >> ${testname}_${logfile}.log
>>> >
>>> > Sure. I don't think we should the name only for the log file, though,
>>> > but also for things like the "## " informative messages we print here
>>> > and there. That would make the log file simpler to follow. Also, I'm
>>> > not sure about having it be optional. (TBH I'm not sure about applname
>>> > either; why do we keep that one?)
>>>
>>> OK, so let's do this: the node name is a mandatory argument of
>>> get_new_node, which is passed to "new PostgresNode" like the port and
>>> the host, and it is then used in the log file name as well as in the
>>> information messages you are mentioning. That's a patch simple enough.
>>> Are you fine with this approach?
>>
>> Sounds reasonable so far.
>
> OK, done so.
>
>>> Regarding the application name, I still think it is useful to have it
>>> though. pg_rewind should actually use it, and the other patch adding
>>> the recovery routines will use it.
>>
>> Using the application_name connection parameter is fine, but I can't think of
>> a reason to set it to "node_".$node->port instead of $node->name. And I can't
>> think of a use for the $node->applname field once you have $node->name. What
>> use case would benefit?
>
> I have the applname stuff, and updated the log messages to use the
> node name for clarity.
>
> The patch to address those points is attached.

As this thread is stalling a bit, please find attached a series of
patch gathering all the pending issues for this thread:
- 0001, fix config_default.pl for MSVC builds to take into account TAP tests
- 0002, append a node name in get_new_node (per Noah's request)
- 0003, the actual recovery test suite
Hopefully this facilitates future reviews.
Regards,
--
Michael

Attachment Content-Type Size
0001-Fix-default-configuration-of-MSVC-builds-ignoring-TA.patch binary/octet-stream 3.0 KB
0002-Assign-node-name-to-TAP-tests.patch binary/octet-stream 13.3 KB
0003-Add-recovery-test-suite.patch binary/octet-stream 22.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2015-12-21 07:48:14 Re: Parallel Aggregate
Previous Message Michael Paquier 2015-12-21 07:26:25 Re: Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby