server: implement the systemd notification protocol

[bdarnell]

Add --background flag to cockroach start that uses the readiness
protocol to decide when the parent process exits (this is a more
complicated version of the traditional daemonization protocol because Go
doesn't expose the necessary system calls). This can reduce the need to
sleep during test scripts, although until we make quit synchronous
we can't eliminate sleeps from reference_test.go.

---

This change is

[tbg]

---

Reviewed 8 of 8 files at r1.
Review status: all files reviewed at latest revision, 9 unresolved discussions, some commit checks failed.

---

acceptance/reference_test.go, line 56 [r1] (raw file):
Was there anything wrong with wait?

---

cli/flags.go, line 66 [r1] (raw file):
s/control/, control/?

---

server/server.go, line 378 [r1] (raw file):
This would be a reason to exit, wouldn't it?

---

util/sdnotify/sdnotify.go, line 42 [r1] (raw file):
Could add that it must only be called once.

---

util/sdnotify/sdnotify.go, line 55 [r1] (raw file):
This stuff will break when not run on unix (?). Should it be made to no-op in these cases (for example via a build tag)?

---

util/sdnotify/sdnotify.go, line 85 [r1] (raw file):
Does the latest entry win (what happens if I have NOTIFY_SOCKET in my env)?

---

util/sdnotify/sdnotify.go, line 106 [r1] (raw file):
I didn't see this used.

---

util/sdnotify/sdnotify.go, line 113 [r1] (raw file):
no removal?

---

util/sdnotify/sdnotify.go, line 141 [r1] (raw file):
so technically s/readyMsg/readyPrefix/?

---

Comments from Reviewable

[petermattis]

---

Review status: all files reviewed at latest revision, 13 unresolved discussions, some commit checks failed.

---

cli/flags.go, line 64 [r1] (raw file):
Is that space before Start doing anything? I thought leading spaces where trimmed in these messages.

---

server/server.go, line 377 [r1] (raw file):
I would think this should be the very last thing to happen in this method. Otherwise, we signal readiness but aren't able to handle grpc-gateway calls.

---

util/sdnotify/sdnotify.go, line 88 [r1] (raw file):
Can combine this an the line above: if err := cmd.Start(); err != nil {

---

util/sdnotify/sdnotify.go, line 92 [r1] (raw file):
Do you plan to fix this? Seems like no, in which case you can get rid of the TODO.

---

Comments from Reviewable

[dt]

Review status: all files reviewed at latest revision, 13 unresolved discussions, some commit checks failed.

---

acceptance/reference_test.go, line 56 [r1] (raw file):
With the move to self-backgrounding rather than & on start, there's nothing left for bash to wait for.

---

Comments from Reviewable

[bdarnell]

Review status: 4 of 8 files reviewed at latest revision, 13 unresolved discussions, some commit checks pending.

---

cli/flags.go, line 64 [r1] (raw file):
Oops, bad fill-paragraph. The space is supposed to be a newline (just for consistency with the other usage text; it's technically irrelevant thanks to wrapText)

---

cli/flags.go, line 66 [r1] (raw file):
Done

---

server/server.go, line 377 [r1] (raw file):
Done.

---

server/server.go, line 378 [r1] (raw file):
I'm not sure. For comparison, etcd logs and continues in this case. The worst case of continuing past this error is that systemd will believe that the process never became ready.

---

util/sdnotify/sdnotify.go, line 42 [r1] (raw file):
Should, not must. I think it's harmless to call more than once.

---

util/sdnotify/sdnotify.go, line 55 [r1] (raw file):
It will only run when $NOTIFY_SOCKET is set, so it should be reasonably safe on other platforms. We can put it behind a build tag if it turns out to be a problem in practice.

---

util/sdnotify/sdnotify.go, line 85 [r1] (raw file):
Looks like it's undefined. (/usr/bin/env will print both values if there are duplicates; Go will use the first one). I'll change it to filter out duplicates (it's annoying that os.Setenv only runs on the singleton environment and there's no way to apply the same processing to a []string).

---

util/sdnotify/sdnotify.go, line 88 [r1] (raw file):
Done.

---

util/sdnotify/sdnotify.go, line 92 [r1] (raw file):
Removed the TODO (but left the comment). I was thinking this could be published as an independent package in which case this might be worth fixing, but there's no reason to fix it as long as start --background is the only use.

---

util/sdnotify/sdnotify.go, line 106 [r1] (raw file):
Removed.

---

util/sdnotify/sdnotify.go, line 113 [r1] (raw file):
It's removed in listener.close()

---

util/sdnotify/sdnotify.go, line 141 [r1] (raw file):
No, readyMsg is the whole message. The :n is because Read() only writes into a prefix of the supplied slice.

---

Comments from Reviewable