Prometheus 2: sighup not handled gracefully during initialization phase

[mwitkow]

What did you do?
Ran P2 from checkout ff0ee26.
We have an external job that SIGHUPs prometheus when config reloads. There is a potential race condition between the first SIGHUP and prometheus being ready. That bug was fixed in P1, but seems to be back in P2.

What did you see instead? Under which circumstances?

2017-05-09T14:00:15.768151000Z 2017/05/09 14:00:15 dinit: pid 37 finished: [/prometheus/prometheus -config.file=/prometheus.yml -log.level=info -query.staleness-delta=60s -query.max-concurrency=100 -query.timeout=2m -storage.local.path=/prometheus-data -storage.tsdb.appendable-blocks=2 -storage.tsdb.max-block-duration=36h -storage.tsdb.min-block-duration=2h -storage.tsdb.retention=360h -storage.tsdb.no-lockfile=true -web.external-url=http://hubmon2.h-te1.internal.improbable.io:9090/] with error: signal: hangup

The error: signal: hangup shouldn't happen. Note: this is after Prometheus starts with an existing volume and existing data, so the issue may be in the code initializing the TSDB.

@fabxc

[fabxc]

Do you recall an issue for Prometheus 1.x on that? The signal handler registration has always been more towards the bottom. However, the storage was initialized asynchronously whereas TSDB does all initialization blocking in the constructor as it is generally expected to not take longer than a couple of seconds. (Unlike P1 crash recovery IIRC)
I'd generally like to stick with that as synchronous component initialization makes wiring everything together a bit easier to reason about.

Would just moving our signal handler registration to the start of main() fix it?

[mwitkow]

@fabxc There was definitely a bug related to crash-recovery and SIGHUPs, but my issue-search-fu is failing me.

It seems to me that moving at least the registration of a signal handler from:

prometheus/cmd/prometheus/main.go

Line 163 in 5890619

// Wait for reload or termination signals. Start the handler for SIGHUP as

to above localstorage init will be both safe and would work around the issue.

[mwitkow]

Closed with the merge of #2700.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.