[PATCH] ctdb-scripts: Invoke pstree with -l option (bug 13531)

[Amitay Isaacs]

On Mon, Jul 16, 2018 at 10:27 PM, Martin Schwenke <martin at meltin.net> wrote:
> Hi Amitay,
>
> On Mon, 16 Jul 2018 14:24:05 +1000, Amitay Isaacs
> <amitay at gmail.com> wrote:
>
>> On Sun, Jul 15, 2018 at 4:46 PM, Martin Schwenke via samba-technical
>> <samba-technical at lists.samba.org> wrote:
>> > This should get rid of the current flappiness in various CTDB tests that
>> > use the pstree command, most notably ctdb/tests/eventd/eventd_022.sh,
>> > which I've seen flapping quite a bit.
>> >
>> > As the commit message says, a couple of recent commits have lengthened
>> > the command string printed by pstree.  This, in combination with the
>> > deep directory structure used by autobuild on sn-devel, is probably
>> > enough to take the test the flapping point.  Then it probably comes
>> > down to the width of the PID printed in the original (unfiltered)
>> > pstree output. Wow!  :-)
>> >
>> > ... and sorry about that... you never think about these things until
>> > they happen.  Who knew that pstree truncates long lines?  :-(
>> >
>> > I ran a private autobuild on sn-devel, running just the ctdb tests, in a
>> > loop this passed 9 times in a row (until I killed it).
>> >
>> > Please review and maybe push...
>>
>> NACK. Let's not make commands non-portable more than necessary.
>
> Hmmm... I thought that the version of pstree in, say, FreeBSD would be
> this one:
>
>   http://www.pell.portland.or.us/~orc/Code/ps-etc/
>
> But it turns out that it is probably this one:
>
>   https://www.freshports.org/sysutils/pstree
>
> where -l means something else entirely.
>
>> There is no real point in trying to match the output of pstree in
>> tests. We have two options.
>>
>> 1. Define PSTREE_CMD appropriately (this takes care of portability),
>> so we can match expected output.
>>
>> 2. Filter out pstree output before matching.  Easier done in eventd
>> tests as we can just drop pstree command from debug-script.sh.
>>
>> We can pick 1 or 2, or some combination of 1 and 2.
>
> I wouldn't like to lose all of the pstree output.  If for some reason
> something changes and the test fails then that output almost certainly
> provides enough information to understand why.

I don't want to lose pstree output. Just filter it out in the tests.
For eventd test, we are not really testing pstree.  We have other
output from the debug-script.sh that proves the correctness of the
behaviour.

> Perhaps something like the following is good enough to show the event
> script and immediate children?
>
> $ ps -e -o pid,ppid,args | awk -v pid=$$ '$1 == pid || $2 == pid { print }'
>  9115  1642 bash
> 10487  9115 ps -e -o pid,ppid,args
> 10488  9115 awk -v pid=9115 $1 == pid || $2 == pid { print }
>
> I've used $$, which illustrates it well, but we would obviously use
> "$pid" or similar.
>
> As far as I can tell the above is completely POSIX-complaint.
>
> I've tried adding -f.  With both that and -e Linux ps prints the
> environment along with each command, which doesn't make sense, since -e
> is only supposed to influence process selection.

Let's not re-invent the wheel.  I am all for keeping pstree in the
debug-hung-script.sh in the current form (pstree -a -p <pid>).  It's
been most useful for tracking down issues with timing out event
scripts.

> Do you think the above gives us what we need and is sufficiently
> portable?

Let's fix the test without jumping through many hoops.

Here's my suggestion:

1. For eventd tests, we can compress the output (drop -a)

2. For simple test, simply filter the pstree output and make sure we
see "sleep 9999" in the output.

Amitay.

Amitay.