feat store: added readiness and livenes prober #1460

FUSAKLA · 2019-08-26T21:01:05Z

Added the prober to yet another component. Store this time.

The store does the bucket initialization and sync synchronously, so the default http server will start up only once it is finished. Then it gets healthy(live) and once the gRPC server starts up the store gets also ready.

Personally I'd much rather see the liveness probe to answer right from the start during the bucket init since this way we leave the bucket sync duration to some initial back-off of an orchestrator.

CHANGELOG entry if change is relevant to the end user.

Changes

added prober to the thanos store

Verification

Started it and verified it gets ready end healthy as expected.

kakkoyun

Hey @FUSAKLA, thanks a lot for this work, we desperately need it. Let's try to push it to finish line.
I have left a small comment, could you please check it?

And maybe you can also rebase it.

cmd/thanos/store.go

kakkoyun · 2019-09-17T14:29:59Z

Hey @FUSAKLA, I realized that we miss liveness and healthiness checks for various components when I was working on https://github.com/metalmatze/kube-thanos/pull/42 and then I have seen your work #656, #1395 and #1297.

We need probes in place for all other components as well. I'd like to push this effort further as fast as possible. I'm happy to help. I could create PRs for components which you haven't touch yet if it is also ok for you? I'd be happy to hear your opinion on those PRs as well.

What do you think?

FUSAKLA · 2019-09-17T16:14:46Z

Hi @kakkoyun ! Yeah I didn't want to make more PRs at once since even this way it takes it's time to get these reviewed. Every one of them raises big discussion on when to report as ready and when as healthy. Thats also the reason it got eventually splitted up but help is always welcomed so feel free to add those PRS! I'd love to see this done finally :D
Thanks for the initiative!

kakkoyun · 2019-09-17T16:20:48Z

@FUSAKLA Alright, great! Let's do this then. I've already sent the first one and pinged you on #1534, PTAL.

It'd be great if we can push this one too.

CHANGELOG.md

cmd/thanos/store.go

bwplotka

LGTM, some minor nits only (:

cmd/thanos/store.go

FUSAKLA · 2019-09-18T20:39:16Z

Thanks @bwplotka ! Should be resolved and rebased on master.

povilasv · 2019-09-19T05:50:49Z

cmd/thanos/store.go

 		if err != nil {
-			return errors.Wrap(err, "create bucket client")
+			runutil.CloseWithLogOnErr(logger, bkt, "bucket client")


I think we should remove if err !=nil, as it is run in defer, err will be overwritten by non related code which happens after this line.

I'd say it was meant to close the client if the runStore ended up with an error.
Otherwise it should not close the client since others are still using it.
Maybe that's why the if err != nil is there and the fact err will be overwritten by the following code was the intention?

But I did not touch this code actually, just removed the unneeded code block wrapping it.

povilasv · 2019-09-19T05:52:43Z

cmd/thanos/store.go

-			cancel()
+	ctx, cancel := context.WithCancel(context.Background())
+	g.Add(func() error {
+		defer runutil.CloseWithLogOnErr(logger, bkt, "bucket client")


this is a bit confusing, so we were closing bkt earlier, why are we closing here again?

commented here #1460 (comment)

cmd/thanos/store.go

Signed-off-by: Martin Chodur <[email protected]>

FUSAKLA · 2019-09-22T06:24:55Z

@kakkoyun PTAL I removed the metricHTTPListenGroup since it is not used anymore and unified the changelog

kakkoyun

Thanks, @FUSAKLA. LGTM. I believe, we've covered all components with this PR.

Signed-off-by: Martin Chodur <[email protected]>

Signed-off-by: Martin Chodur <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]>

* Some updates to compact docs Signed-off-by: Ivan Kiselev <[email protected]> * some formatting Signed-off-by: Ivan Kiselev <[email protected]> * Update docs/components/compact.md accept PR suggestions Co-Authored-By: Bartlomiej Plotka <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Add metalmatze to list of maintainers (#1547) Signed-off-by: Matthias Loibl <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * resolve comments Signed-off-by: Ivan Kiselev <[email protected]> * resolve last comment Signed-off-by: Ivan Kiselev <[email protected]> * receive: Add liveness and readiness probe (#1537) * Add prober to receive Signed-off-by: Kemal Akkoyun <[email protected]> * Add changelog entries Signed-off-by: Kemal Akkoyun <[email protected]> * Update README Signed-off-by: Kemal Akkoyun <[email protected]> * Remove default Signed-off-by: Kemal Akkoyun <[email protected]> * Wait hashring to be ready Signed-off-by: Kemal Akkoyun <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * downsample: Add liveness and readiness probe (#1540) * Add readiness and liveness probes for downsampler Signed-off-by: Kemal Akkoyun <[email protected]> * Add changelog entry Signed-off-by: Kemal Akkoyun <[email protected]> * Remove default Signed-off-by: Kemal Akkoyun <[email protected]> * Set ready Signed-off-by: Kemal Akkoyun <[email protected]> * Update CHANGELOG Signed-off-by: Kemal Akkoyun <[email protected]> * Clean CHANGELOG Signed-off-by: Kemal Akkoyun <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Document the dnssrvnoa option (#1551) Signed-off-by: Antonio Santos <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * feat store: added readiness and livenes prober (#1460) Signed-off-by: Martin Chodur <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Add Hotstar to adopters. (#1553) It's the largest streaming service in India that does cricket and GoT for India. They have insane scale and are using Thanos to scale their Prometheus. Spoke to them offline about adding the logo and will get a signoff here too. Signed-off-by: Goutham Veeramachaneni <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Fix hotstar logo in the adoptor's list (#1558) Signed-off-by: Karthik Vijayaraju <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Fix typos, including 'fomrat' -> 'format' in tracing.config-file help text. (#1552) Signed-off-by: Callum Styan <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Compactor: Fix for #844 - Ignore object if it is the current directory (#1544) * Ignore object if it is the current directory Signed-off-by: Jamie Poole <[email protected]> * Add full-stop Signed-off-by: Jamie Poole <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Adding doc explaining the importance of groups for compactor (#1555) Signed-off-by: Leo Meira Vital <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Add blank line for list (#1566) The format of these files is wrong in the web. Signed-off-by: dongwenjuan <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Refactor compactor constants, fix bucket column (#1561) * compact: unify different time constants Use downsample.* constants where possible. Move the downsampling time ranges into constants and use them as well. Signed-off-by: Giedrius Statkevičius <[email protected]> * bucket: refactor column calculation into compact Fix the column's name and name it UNTIL-DOWN because that is what it actually shows - time until the next downsampling. Move out the calculation into a separate function into the compact package. Ideally we could use the retention policies in this calculation as well but the `bucket` subcommand knows nothing about them :-( Signed-off-by: Giedrius Statkevičius <[email protected]> * compact: fix issues with naming Reorder the constants and fix mistakes. Signed-off-by: Giedrius Statkevičius <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * remove duplicate Signed-off-by: Ivan Kiselev <[email protected]>

Signed-off-by: Martin Chodur <[email protected]> Signed-off-by: Giedrius Statkevičius <[email protected]>

* Some updates to compact docs Signed-off-by: Ivan Kiselev <[email protected]> * some formatting Signed-off-by: Ivan Kiselev <[email protected]> * Update docs/components/compact.md accept PR suggestions Co-Authored-By: Bartlomiej Plotka <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Add metalmatze to list of maintainers (#1547) Signed-off-by: Matthias Loibl <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * resolve comments Signed-off-by: Ivan Kiselev <[email protected]> * resolve last comment Signed-off-by: Ivan Kiselev <[email protected]> * receive: Add liveness and readiness probe (#1537) * Add prober to receive Signed-off-by: Kemal Akkoyun <[email protected]> * Add changelog entries Signed-off-by: Kemal Akkoyun <[email protected]> * Update README Signed-off-by: Kemal Akkoyun <[email protected]> * Remove default Signed-off-by: Kemal Akkoyun <[email protected]> * Wait hashring to be ready Signed-off-by: Kemal Akkoyun <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * downsample: Add liveness and readiness probe (#1540) * Add readiness and liveness probes for downsampler Signed-off-by: Kemal Akkoyun <[email protected]> * Add changelog entry Signed-off-by: Kemal Akkoyun <[email protected]> * Remove default Signed-off-by: Kemal Akkoyun <[email protected]> * Set ready Signed-off-by: Kemal Akkoyun <[email protected]> * Update CHANGELOG Signed-off-by: Kemal Akkoyun <[email protected]> * Clean CHANGELOG Signed-off-by: Kemal Akkoyun <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Document the dnssrvnoa option (#1551) Signed-off-by: Antonio Santos <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * feat store: added readiness and livenes prober (#1460) Signed-off-by: Martin Chodur <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Add Hotstar to adopters. (#1553) It's the largest streaming service in India that does cricket and GoT for India. They have insane scale and are using Thanos to scale their Prometheus. Spoke to them offline about adding the logo and will get a signoff here too. Signed-off-by: Goutham Veeramachaneni <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Fix hotstar logo in the adoptor's list (#1558) Signed-off-by: Karthik Vijayaraju <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Fix typos, including 'fomrat' -> 'format' in tracing.config-file help text. (#1552) Signed-off-by: Callum Styan <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Compactor: Fix for #844 - Ignore object if it is the current directory (#1544) * Ignore object if it is the current directory Signed-off-by: Jamie Poole <[email protected]> * Add full-stop Signed-off-by: Jamie Poole <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Adding doc explaining the importance of groups for compactor (#1555) Signed-off-by: Leo Meira Vital <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Add blank line for list (#1566) The format of these files is wrong in the web. Signed-off-by: dongwenjuan <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * Refactor compactor constants, fix bucket column (#1561) * compact: unify different time constants Use downsample.* constants where possible. Move the downsampling time ranges into constants and use them as well. Signed-off-by: Giedrius Statkevičius <[email protected]> * bucket: refactor column calculation into compact Fix the column's name and name it UNTIL-DOWN because that is what it actually shows - time until the next downsampling. Move out the calculation into a separate function into the compact package. Ideally we could use the retention policies in this calculation as well but the `bucket` subcommand knows nothing about them :-( Signed-off-by: Giedrius Statkevičius <[email protected]> * compact: fix issues with naming Reorder the constants and fix mistakes. Signed-off-by: Giedrius Statkevičius <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]> * remove duplicate Signed-off-by: Ivan Kiselev <[email protected]> Signed-off-by: Giedrius Statkevičius <[email protected]>

FUSAKLA force-pushed the fus-store-prober branch 2 times, most recently from 0027f18 to e44d882 Compare August 26, 2019 21:05

FUSAKLA mentioned this pull request Aug 28, 2019

feat sicedar: added readiness prober #1395

Merged

1 task

FUSAKLA force-pushed the fus-store-prober branch 2 times, most recently from 8457aec to f15ba68 Compare August 29, 2019 19:37

kakkoyun requested changes Sep 17, 2019

View reviewed changes

cmd/thanos/store.go Show resolved Hide resolved

bwplotka reviewed Sep 17, 2019

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

bwplotka reviewed Sep 17, 2019

View reviewed changes

cmd/thanos/store.go Outdated Show resolved Hide resolved

bwplotka reviewed Sep 17, 2019

View reviewed changes

cmd/thanos/store.go Show resolved Hide resolved

cmd/thanos/store.go Outdated Show resolved Hide resolved

FUSAKLA force-pushed the fus-store-prober branch from f15ba68 to 315602f Compare September 18, 2019 20:36

FUSAKLA force-pushed the fus-store-prober branch from 315602f to 6d2e5f2 Compare September 18, 2019 20:42

povilasv reviewed Sep 19, 2019

View reviewed changes

kakkoyun mentioned this pull request Sep 19, 2019

All missing liveness and readiness probes thanos-io/kube-thanos#43

Merged

FUSAKLA force-pushed the fus-store-prober branch from 76c0da7 to 4d8ca19 Compare September 22, 2019 06:19

feat store: added readiness and livenes prober

d334786

Signed-off-by: Martin Chodur <[email protected]>

FUSAKLA force-pushed the fus-store-prober branch from 4d8ca19 to d334786 Compare September 22, 2019 06:23

kakkoyun approved these changes Sep 22, 2019

View reviewed changes

brancz approved these changes Sep 23, 2019

View reviewed changes

brancz merged commit 7a2f54e into thanos-io:master Sep 23, 2019

FUSAKLA mentioned this pull request Sep 23, 2019

Add missing /-/healthy endpoint #644

Closed

ivan-kiselev pushed a commit to ivan-kiselev/thanos that referenced this pull request Sep 26, 2019

feat store: added readiness and livenes prober (thanos-io#1460)

08fac9e

Signed-off-by: Martin Chodur <[email protected]>

ivan-kiselev pushed a commit to ivan-kiselev/thanos that referenced this pull request Sep 26, 2019

feat store: added readiness and livenes prober (thanos-io#1460)

aaf94d4

Signed-off-by: Martin Chodur <[email protected]> Signed-off-by: Ivan Kiselev <[email protected]>

GiedriusS pushed a commit that referenced this pull request Oct 28, 2019

feat store: added readiness and livenes prober (#1460)

f0b9d09

Signed-off-by: Martin Chodur <[email protected]> Signed-off-by: Giedrius Statkevičius <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat store: added readiness and livenes prober #1460

feat store: added readiness and livenes prober #1460

FUSAKLA commented Aug 26, 2019 •

edited

Loading

kakkoyun left a comment •

edited

Loading

kakkoyun commented Sep 17, 2019

FUSAKLA commented Sep 17, 2019

kakkoyun commented Sep 17, 2019 •

edited

Loading

bwplotka left a comment

FUSAKLA commented Sep 18, 2019

povilasv Sep 19, 2019

FUSAKLA Sep 22, 2019

povilasv Sep 19, 2019

FUSAKLA Sep 22, 2019

FUSAKLA commented Sep 22, 2019

kakkoyun left a comment •

edited

Loading

feat store: added readiness and livenes prober #1460

feat store: added readiness and livenes prober #1460

Conversation

FUSAKLA commented Aug 26, 2019 • edited Loading

Changes

Verification

kakkoyun left a comment • edited Loading

Choose a reason for hiding this comment

kakkoyun commented Sep 17, 2019

FUSAKLA commented Sep 17, 2019

kakkoyun commented Sep 17, 2019 • edited Loading

bwplotka left a comment

Choose a reason for hiding this comment

FUSAKLA commented Sep 18, 2019

povilasv Sep 19, 2019

Choose a reason for hiding this comment

FUSAKLA Sep 22, 2019

Choose a reason for hiding this comment

povilasv Sep 19, 2019

Choose a reason for hiding this comment

FUSAKLA Sep 22, 2019

Choose a reason for hiding this comment

FUSAKLA commented Sep 22, 2019

kakkoyun left a comment • edited Loading

Choose a reason for hiding this comment

FUSAKLA commented Aug 26, 2019 •

edited

Loading

kakkoyun left a comment •

edited

Loading

kakkoyun commented Sep 17, 2019 •

edited

Loading

kakkoyun left a comment •

edited

Loading