roachprod: remove monitor netcat command

[ajkr]

roachprod monitor assumes nc will exit as soon as Cockroach server
exits. This actually is not the case in later versions of netcat (tested
on Ubuntu 18.04+).

This PR changes to a polling approach calling kill -0 once per second
to monitor the Cockroach server's liveness. This should give us better
portability and we verified the overhead is low (~0.65ms of a CPU core's
time per kill invocation). Tested by running roachprod monitor
locally, gradually killing the nodes, and observing the output:

3: 28342
1: 28176
2: 28257
3: kill exited nonzero
3: dead
2: kill exited nonzero
2: dead
1: kill exited nonzero
1: dead

Fixes #37370.

Release note: None

[cockroach-teamcity]

This change is 

[tbg]

I have bad news -- -d isn't a thing on OSX, and this stuff has to work there too (because of --local).

I'm thinking that this netcat thing is trying to be too clever. It's just trying to achieve "instant" detection of the dead node, but we could just replace it with a "sleep 1" and end up in a functionally similar, but much simpler place. Am I missing a reason for which this isn't viable?

PS great find. Are we using 18.04 in our nightly testing, or where did you run into it?

[petermattis]

When monitor was originally written, I recall trying to poll with lsof and finding it moderately expensive. nc is such a simple utility. It is sad that this behavior is changing between ubuntu releases.

Note that we know if the machine we're running nc on is local or not. We could optionally add -d based on that, or try to detect if -d is supported.

Reviewable status: complete! 0 of 0 LGTMs obtained

[petermattis]

Another thought is to add cockroach debug monitor or cockroach debug wait. I'd guess it is a few dozen lines of Go code to open the network connection and block waiting for it to be closed.

[tbg]

@petermattis are you saying you don't just want to rip out nc? That seems like a straightforward way to kill less eng time on this.

Before we write a utility that does low-level nc stuff against an open crdb port, we can just add a GRPC endpoint that lets you wait forever. But then you have all the migration concerns to deal with. I really think we should just remove nc from the equation and loop a little more often.

[petermattis]

@petermattis are you saying you don't just want to rip out nc? That seems like a straightforward way to kill less eng time on this.

I'm fine with ripping out nc, but see: "When monitor was originally written, I recall trying to poll with lsof and finding it moderately expensive." For performance tests, we don't want monitor to have an effect on performance.

[tbg]

Oh, I see. How about polling with kill -0 $PID? You use lsof to discover and then do a cheap thing frequently after.

[petermattis]

Oh, I see. How about polling with kill -0 $PID? You use lsof to discover and then do a cheap thing frequently after.

Seems possible. The irritating thing is testing that this doesn't have an effect on performance.

[ajkr]

I have bad news -- -d isn't a thing on OSX, and this stuff has to work there too (because of --local).

That is very bad news!

PS great find. Are we using 18.04 in our nightly testing, or where did you run into it?

Thanks. I'm using 18.04 and was addressing one of Peter's suggestions to make a callback for node deaths. When running with --local on my machine, that callback never got called.

[ajkr]

Should we measure how long kill takes? What is a good number? How about sacrificing at most 0.1% of CPU time since this is only used in tests? I'd guess it'll be way less than that since kill is a small program that just sends a signal, and we're talking about running it once a second. But we can reduce the frequency of running it if needed.

[tbg]

Sounds reasonable to me. I would be very surprised if it weren't negligible but worth checking once.

…

On Wed, May 8, 2019, 20:19 Andrew Kryczka ***@***.***> wrote: Should we measure how long kill takes? What is a good number? How about sacrificing at most 0.1% of CPU time since this is only used in tests (I'd guess it'll be way less than that since kill is a small program that just sends a signal, and we're talking about running it once a second)? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#37390 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABGXPZFQOGGPU22XWWGWYRDPUMKT7ANCNFSM4HLOZSPA> .

[tbg]

PS with -0 it doesn't even send a signal, I guess all it does is check whether the PID exists. Of course there might be the problem that the PID switches out from under you. Wonder if the old code handled that. The concern is theoretical so I wouldn't worry about regressing here.

…

On Wed, May 8, 2019, 20:39 Tobias Grieger ***@***.***> wrote: Sounds reasonable to me. I would be very surprised if it weren't negligible but worth checking once. On Wed, May 8, 2019, 20:19 Andrew Kryczka ***@***.***> wrote: > Should we measure how long kill takes? What is a good number? How about > sacrificing at most 0.1% of CPU time since this is only used in tests (I'd > guess it'll be way less than that since kill is a small program that > just sends a signal, and we're talking about running it once a second)? > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub > <#37390 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABGXPZFQOGGPU22XWWGWYRDPUMKT7ANCNFSM4HLOZSPA> > . >

[ajkr]

The kill command takes 0.646 ms so running it once per second is within our 0.1% overhead bound (although not by as large a margin as I would've guessed!).

$ rm -f ./kill-times.out ; for l in $(seq 1000); do perf stat -e cpu-clock kill -0 `pidof sleep` |& grep cpu-clock | awk '{print $1}' >>./kill-times.out ; done ; echo "scale=5; ($(paste -sd+ ./kill-times.out)) / 1000" | bc
.64649

[ajkr]

For comparison, here is the same methodology used to measure lsof times. Those were indeed way slower at 36ms per call. That'd be 3.6% of a core during benchmarking which I agree doesn't look good.

$ rm -f ./lsof-times.out ; for l in $(seq 1000); do perf stat -e cpu-clock lsof -i :12345 -sTCP:LISTEN |& grep cpu-clock | awk '{print $1}' >>./lsof-times.out ; done ; echo "scale=5; ($(paste -sd+ ./lsof-times.out)) / 1000" | bc
35.96782

[ajkr]

Alright, RFAL.

[ajkr]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)