Allow ingester initialization to continue in background. #2104
Conversation
This helps to start the rest of Cortex binary, especially readiness and metrics handlers. Until ingester initialization has finished, it will return errors to most (but not all) API requests. This solves problem with slow ingester initialization when doing WAL replays on block databases. Signed-off-by: Peter Štibraný <[email protected]>
| if err != nil { | ||
| return | ||
| var errfut *ingester.ErrorFuture | ||
| t.ingester, errfut = ingester.New(cfg.Ingester, cfg.IngesterClient, t.overrides, t.store, prometheus.DefaultRegisterer) |
There was a problem hiding this comment.
A difference I see is that currently Cortex exits if the initialization fails. After this PR it doesn't and if the ingester initialization fails it will continue to run but not work.
There was a problem hiding this comment.
That is a valid point, and I can address that.
| return | ||
| var errfut *ingester.ErrorFuture | ||
| t.ingester, errfut = ingester.New(cfg.Ingester, cfg.IngesterClient, t.overrides, t.store, prometheus.DefaultRegisterer) | ||
| if done, err := errfut.Get(); done && err != nil { |
There was a problem hiding this comment.
Futures feel needlessly complicated here, and an anti-pattern with Go. Concurrency should be left to the caller, so I believe the module init function should call ingester.New in a goroutine that waits for an error response.
There was a problem hiding this comment.
Hmm, that is a valid point. Back to drawing board.
There was a problem hiding this comment.
However, by contract the ingester should be populated in the Cortex struct once the initIngester() terminates.
This PR solves problem with slow ingester initialization when doing WAL replays on block databases.
By allowing ingester initialization to continue in the background, the rest of Cortex binary can start in the meantime, especially readiness and metrics handlers. Until ingester initialization has finished, it will return errors to most (but not all, like Flush) API requests.
There may be implications for returning errors from API calls. Alternatively, we can delay registration of handlers until ingester initialization has finished. That would be easy to do, but I'm not sure about the right approach.
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]