ruler: probes are not started while WAL is being replayed

[OGKevin]

Thanos, Prometheus and Golang version used:
Thanos: 20e8105ec5b697a4755f2ff735509924173bd5cf

What happened:
Ruler crashed and was restarted, it now needs to replay the WAL. However, the probes are started after the WAL has been read, This code

thanos/cmd/thanos/rule.go

Lines 320 to 323 in f2564a7

	db, err := tsdb.Open(conf.dataDir, log.With(logger, "component", "tsdb"), reg, tsdbOpts)
	if err != nil {
	return errors.Wrap(err, "open TSDB")
	}

eventually calls head.Init in tsdb that replays the wall. The health probes are started afterwards here:

thanos/cmd/thanos/rule.go

Lines 531 to 537 in f2564a7

	g.Add(func() error {
	statusProber.Ready()
	return s.ListenAndServe()
	}, func(err error) {
	statusProber.NotReady(err)
	s.Shutdown(err)
	})

This means that while ruler is reading the WAL, the liveness probe will fail and you end up in a crash loop unless you significantly increase the initial delay.

What you expected to happen:
Probe be the first thing to get started while the rest of ruler starts up. Reading WAL should be considered healthy but not ready. While failure to read WAL should be unhealthy.

[bwplotka]

Valid bug. Maybe @ianbillett want to try to fix it? 🤗

[bwplotka]

BTW note that we are close to make stateless ruler happen (#4250), but WAL will be still there 🙃

[OGKevin]

Cool, I'm also happy to work on this. Was just not sure if this was an actual bug or not.

[bill3tt]

@OGKevin nice report! Since you reported the bug, you have the right to fix it. It doesn't seem too complicated, so a nice opportunity to get a commit into the Thanos code base :)

To fix, I think we just need to change the ordering of the ruler startup (as you point out above) so that we initialise the probes, and mark ourselves as healthy before then initialising the TSDB.

[OGKevin]

So due to the current dependency chain, the current order makes sense.

HTTP and GRPC server need Rule Manager, Rule Manager needs tsdb.

I can think of 2 solutions atm:

1. Start HTTP server first with probes, and later register the additional /api routes to HTTP and start GRPC server once Rule Manager has been set up.
2. Create some "complex" implementations for HTTP and GRPC server that run fully on start and return errors until the status probe is set to ready. So basically make them status probe aware. But this most likely would require more discussion to come up with a design. This one could also introduce null pointer bugs.

[bwplotka]

I think we need 2 but it won't be complex. It will be exactly the same as

thanos/pkg/receive/multitsdb.go

Line 383 in 13ab756

type ReadyStorage struct {

(: so some readiness struct that will tell API when to return success.

[stale]

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[stale]

Closing for now as promised, let us know if you need this to be reopened! 🤗