On Sat, Dec 12, 2015 at 8:29 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Sat, Dec 12, 2015 at 11:37 AM, Noah Misch <n...@leadboat.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 >>> <alvhe...@2ndquadrant.com> wrote: >>> > Michael Paquier wrote: >>> >> On Fri, Dec 11, 2015 at 5:35 AM, Alvaro Herrera >>> >> <alvhe...@2ndquadrant.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
0001-Fix-default-configuration-of-MSVC-builds-ignoring-TA.patch
Description: binary/octet-stream
0002-Assign-node-name-to-TAP-tests.patch
Description: binary/octet-stream
0003-Add-recovery-test-suite.patch
Description: binary/octet-stream
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers