query: cleanup store statuses as they come and go #910

adrien-f · 2019-03-12T12:50:27Z

Signed-off-by: Adrien Fillon adrien.fillon@cdiscount.com

Changes

Cleanup unhealthy stores from UI view after 5m
Keep stores metadata between health states
Sort stores types in UI

Verification

With static
With K8S DNS

bwplotka

Generally OK, just minor nit.

pkg/query/storeset.go

bwplotka · 2019-03-12T14:44:51Z

Built v0.3.2-adrien-f with this change.

pkg/query/storeset.go

adrien-f · 2019-03-16T18:00:25Z

From Slack:

@mreichardt95 :

looks great so far! only unexpected behaviour i found, was when using static ip addresses. They will also get cleaned up after one minute downtime. But the only solution to fix this is probably give the StoreSet information on how it was discovered?

Thanks for the feedback, will get on it !

adrien-f · 2019-03-17T14:14:36Z

Alright. This was much simpler in the end. If the target leaves, it won't be checked again so I just need to check against the LastCheck property. The timeout before being cleaned up is 5 minutes. I figure if I get a page late at night, I'd be happy to catch the faulty store ?

Also, looking for input on how to do best the sorting of stores in UI page by their type.

bwplotka · 2019-03-18T14:38:14Z

I figure if I get a page late at night, I'd be happy to catch the faulty store ?

That makes sense, but I think there must be other means than just UI.. like metric or something. And if you really want UI to be useful there, then 5m is not enough. Most alerts have 5m for or/and uses 5m rates. (:

adrien-f · 2019-03-18T17:05:53Z

I'd like to have the store statuses also available as a JSON endpoint. We have the metrics part coming with #900

pkg/query/storeset.go

cmd/thanos/query.go

bwplotka

Awesome, just minor nit related to RWLock (:

CHANGELOG.md

bwplotka · 2019-03-26T21:09:11Z

pkg/query/storeset.go

+	status.LastError = err
+	status.LastCheck = time.Now()
+
+	if err == nil {


This is quite weird to me. Essentially not consistent. What if

We have success check and then fail. Why we have obsolete label, type ,min, max?

We have err as first think. We have it empty.

That's kind of inconsistent?

Might not be the more explicit way of doing things, didn't want to add too many cases. Here's my logic:

Query with a --store=foo.local argument

Healthy first, status populated

It's now broken, after a few it will be dropped

Next updates won't have component, labels and time info so would overwrite existing status info

Does that make sense ?

For the second point, not sure I follow you

I was talking about consistent error handling. You have two basic cases.

1 as talked through
2 which is store=foo.local was never healthy.

I think I am missing the fact that 2 does not exists because it was never healthy ;p Ignore me then.

bwplotka · 2019-03-26T21:10:03Z

pkg/query/storeset.go

-	storeNodeConnections prometheus.Gauge
-	externalLabelStores  map[string]int
-	storeStatuses        map[string]*StoreStatus
+	mtx                   sync.RWMutex


let's move to sync.Mutex if we don use R and W separation (:

pkg/query/storeset.go

bwplotka

All good, only minors, approving, thanks!

pkg/query/storeset.go

bwplotka · 2019-04-08T12:00:05Z

pkg/ui/query.go

+	sort.Slice(sources, func(i int, j int) bool {
+		if sources[i] == nil {
+			return false
+		} else if sources[j] == nil {


You never need else if you are returning, let's simplify this (:

bwplotka · 2019-04-08T12:02:32Z

pkg/query/storeset.go

+	status.LastError = err
+	status.LastCheck = time.Now()
+
+	if err == nil {


I was talking about consistent error handling. You have two basic cases.

1 as talked through
2 which is store=foo.local was never healthy.

I think I am missing the fact that 2 does not exists because it was never healthy ;p Ignore me then.

Signed-off-by: Adrien Fillon <adrien.fillon@cdiscount.com>

bwplotka

Legit, thanks for this! LGTM <3

* query: cleanup store statuses as they come and go (thanos-io#910) Signed-off-by: Adrien Fillon <adrien.fillon@cdiscount.com> * [docs] Example of using official prometheus helm chart to deploy server with sidecar (thanos-io#1003) * update documentation with an example of using official prometheus helm chart Signed-off-by: Ivan Kiselev <ivan@messagebird.com> * a little formatting to values Signed-off-by: Ivan Kiselev <ivan@messagebird.com> * satisfy PR comments Signed-off-by: Ivan Kiselev <ivan@messagebird.com> * Compact: group concurrency (thanos-io#1010) * compact: add concurrency to group compact * add flag to controll the number of goroutines to use when compacting group * update compact.md for group-compact-concurrency * fixed: miss wg.Add() * address CR * regenerate docs * use err group * fix typo in flag description * handle context * set up workers in main loop * move var initialisation * remove debug log * validate concurrency * move comment * warn -> error * remove extra newline * fix typo * dns: Added miekgdns resolver as a hidden option to query and ruler. (thanos-io#1016) Fixes: thanos-io#1015 Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * query: set default evaluation interval (thanos-io#1028) Subqueries allows request with no [specified resolution](https://prometheus.io/blog/2019/01/28/subquery-support/). Set it up and allow to configure default evaluation interval. * store+compactor: pre-compute index cache during compaction (thanos-io#986) Fixes first part of thanos-io#942 This changes allow to safe some startup & sync time in store gateway as it is no longer is needed to compute index-cache from block index on its own. For compatibility store GW still can do it, but it first checks bucket if there is index-cached uploaded already. In the same time, compactor precomputes the index cache file on every compaction. To allow quicker addition of index cache files we added `--index.generate-missing-cache-file` flag, that if enabled precompute missing files on compactor startup. Note that it will take time and it's only one-off step per bucket. Signed-off-by: Aleksei Semiglazov <xjewer@gmail.com> * Added website for Thanos' docs using Hugo. (thanos-io#807) Hosted in github pages. Signed-off-by: adrien-f <adrien.fillon@gmail.com> Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * gcs: Fixed scopes for inline ServiceAccount option. (thanos-io#1033) Without this that option was unusable. Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * Fixed root docs and liche is now checking root dir as well. (thanos-io#1040) Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * storage docs: add detail about GCS policies and testing (thanos-io#1037) * add more details about GCS policies and testing * remove fixed names from exec command * Prometheus library updated to v2.8.1 (thanos-io#1009) * compact: group concurrency improvements (thanos-io#1029) * group concurrency improvements * remove unnecessary error check * add to wg in main goroutine * receive: Add block shipping (thanos-io#1011) * receive: Add retention flag for local tsdb storage (thanos-io#1046) * querier: Add /api/v1/labels support (thanos-io#905) * Feature: add /api/v1/labels support Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Disabled gossip by default, marked all flags as deprecated. (thanos-io#1055) + changed small label. Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * ruler: Fixed Chunk going out or Max Uint16. (thanos-io#1041) Fixes thanos-io#1038 Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * store: azure: allow passing an endpoint parameter for specific regions (thanos-io#980) Fix thanos-io#968 Signed-off-by: Adrien Fillon <adrien.fillon@cdiscount.com> * feature: support POST method for series endpoint (thanos-io#1021) Signed-off-by: Joseph Lee <joseph.t.lee@outlook.com> * bucket verify: repair out of order labels (thanos-io#964) * bucket verify: repair out of order labels * verify repair: correctly order series in the index on rewrite When we have label sets that are not in the correct order, fixing that changes the order of the series in the index. So the index must be rewritten in that new order. This makes this repair tool take up a bunch more memory, but produces blocks that verify correctly. * Fix the TSDB block safe-delete function The directory name must be the block ID name exactly to verify. A temp directory or random name will not work here. * verify repair: fix duplicate chunk detection Pointer/reference logic error was eliminating all chunks for a series in a given TSDB block that wasn't the first chunk. Chunks are now referenced correctly via pointers. * PR feedback: use errors.Errorf() instead of fmt.Errorf() * Use errors.New() Some linters catch errors.Errorf() as its not really part of the errors package. * Liberally comment this for loop We're comparing items by pointers, using Go's range variables is misleading here and we need not fall into the same trap. * Take advantage of sort.Interface This prevents us from having to re-implement label sorting. * PR Feedback: Comments are full sentences. * Cut release 0.4.0-rc.0 (thanos-io#1017) * Cut release 0.4.0-rc.0 🎉 🎉 NOTE: This is last release that has gossip. Signed-off-by: Bartek Plotka <bwplotka@gmail.com> Co-Authored-By: povilasv <p.versockas@gmail.com> * Fixed crossbuild. Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * ci: Env fixes. (thanos-io#1058) Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * Removed bzr requirement for make crossbuild. Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * Bump github.com/hashicorp/golang-lru from 0.5.0 to 0.5.1 (thanos-io#1051) Bumps [github.com/hashicorp/golang-lru](https://github.com/hashicorp/golang-lru) from 0.5.0 to 0.5.1. - [Release notes](https://github.com/hashicorp/golang-lru/releases) - [Commits](hashicorp/golang-lru@v0.5.0...v0.5.1) Signed-off-by: dependabot[bot] <support@dependabot.com> * Initialze and correctly register all index cache metrics. (thanos-io#1069) * store/cache: add more tests (thanos-io#1071) * Fixed Downsampling process; Fixed `runutil.CloseAndCaptureErr` (thanos-io#1070) * runutil. Simplified CloseWithErrCapture. Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * Fixed Downsampling process; Fixed runutil.CloseAndCaptureErr Fixes thanos-io#1065 Root cause: * runutil defered capture error function was not passing error properly so unit tests were passing, event though there was bug * streamed block write index cache requires index file which was not closed (saved) properly yet. Closers need to be closed to perform this. Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * objstore: Expose S3 region attribute (thanos-io#1060) Minio is able to autodetect the region for cloud providers like AWS but the logic fails with Scaleway Object Storage solution. Related issue on Minio: minio/mc#2570 * Fixed fetching go-bindata failed (thanos-io#1074) * Fixed bug: - fetching go-bindata failed. - change the repo of go-bindata to github.com/go-bindata/go-bindata, because old repo has been archived. - pin the go-bindata as v3.3.1. Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Add CHANGELOG Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Remove CHANGELOG Signed-off-by: jojohappy <sarahdj0917@gmail.com> * add compare flags func to compare flags between prometheus and sidecar (thanos-io#838) Original message: * update documentation for a max/min block duration add compare flags func to compare flags between prom and sidecar * fix some nits Functional change: now we check the configured flags (if possible) and error out if MinTime != MaxTime. We need to check this always since if that is not true then we will get overlapping blocks. Additionally, an error message is printed out if it is not equal to 2h (the recommended value). * Ensured index cache is best effort, refactored tests, validated edge cases. (thanos-io#1073) Fixes thanos-io#651 Current size also includes slice header. Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * website: Moved to netlify. (thanos-io#1078) Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * website: Fixing netlify. (thanos-io#1080) Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * website: Added "founded by" footer. (thanos-io#1081) Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * store/proxy: properly check if context has ended (thanos-io#1082) How the code was before it could happen that we might receive some series from the stream however by the time we'd send them back to the reader, it would not read it anymore since the deadline would have been exceeded. Properly use a `select` here to get out of the goroutine if the deadline has been exceeded. Might potentially fix a problem where we see one goroutine hanging constantly (and thus blocking from work being done): ``` goroutine profile: total 126 25 @ 0x42f62f 0x40502b 0x405001 0x404de5 0xe7435b 0x45cc41 0xe7435a github.com/improbable-eng/thanos/pkg/store.startStreamSeriesSet.func1+0x18a /go/src/github.com/improbable-eng/thanos/pkg/store/proxy.go:318 ``` * Cut release v0.4.0-rc.1 (thanos-io#1088) Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * website: Removed ghpages handling; fixed docs; and status badge. (thanos-io#1084) Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * Fix readme (thanos-io#1090) * store: Compose indexCache properly allowing injection for testing purposes. (thanos-io#1098) Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * website: add sponsor section on homepage (thanos-io#1062) * website: Adjusted logos sizing and responsiveness. (thanos-io#1105) Signed-off-by: Bartek Plotka <bwplotka@gmail.com> * Add Monzo to "Used by" section 🎉 (thanos-io#1106) * Compactor: remove malformed blocks after delay (thanos-io#1053) * compactor removes malformed blocks after delay * compactor removes malformed blocks after delay * include missing file * reuse existing freshness check * fix comment * remove unused var * fix comment * syncDelay -> consistencyDelay * fix comment * update flag description * address cr * fix dupliacte error handling * minimum value for --consistency-delay * update * docs * add test case * move test to inmem bucket * Add Utility Warehouse to "used by" section (thanos-io#1108) * Add Utility Warehouse logo * Make logo smaller * website: add Adform as users (thanos-io#1109) We use Thanos extensively as well so I have added Adform. * Cut release v0.4.0 (thanos-io#1107) Signed-off-by: Bartek Plotka <bwplotka@gmail.com>

bwplotka approved these changes Mar 12, 2019

View reviewed changes

pkg/query/storeset.go Outdated Show resolved Hide resolved

pkg/query/storeset.go Outdated Show resolved Hide resolved

GiedriusS reviewed Mar 15, 2019

View reviewed changes

pkg/query/storeset.go Outdated Show resolved Hide resolved

domgreen added the component: query label Mar 16, 2019

adrien-f force-pushed the feat/cleanup-store-statuses branch from 01972a8 to 598e6fc Compare March 17, 2019 14:13

adrien-f force-pushed the feat/cleanup-store-statuses branch from 598e6fc to d277421 Compare March 17, 2019 14:30

adrien-f mentioned this pull request Mar 17, 2019

Query: add thanos_store_up metric to StoreSet #900

Closed

povilasv reviewed Mar 22, 2019

View reviewed changes

pkg/query/storeset.go Outdated Show resolved Hide resolved

povilasv requested changes Mar 22, 2019

View reviewed changes

pkg/query/storeset.go Outdated Show resolved Hide resolved

adrien-f force-pushed the feat/cleanup-store-statuses branch 3 times, most recently from 02e6ffb to f9f7f11 Compare March 22, 2019 20:54

GiedriusS reviewed Mar 23, 2019

View reviewed changes

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

adrien-f force-pushed the feat/cleanup-store-statuses branch 2 times, most recently from ad98501 to 09d19c2 Compare March 23, 2019 17:18

FUSAKLA approved these changes Mar 23, 2019

View reviewed changes

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

povilasv approved these changes Mar 25, 2019

View reviewed changes

adrien-f force-pushed the feat/cleanup-store-statuses branch 3 times, most recently from def88a7 to 8aa085f Compare March 25, 2019 12:00

bwplotka requested changes Mar 26, 2019

View reviewed changes

adrien-f force-pushed the feat/cleanup-store-statuses branch from 8aa085f to 45c6043 Compare March 26, 2019 22:00

domgreen requested a review from bwplotka March 27, 2019 09:36

bwplotka approved these changes Apr 8, 2019

View reviewed changes

adrien-f force-pushed the feat/cleanup-store-statuses branch from 45c6043 to 4ad70e0 Compare April 8, 2019 12:42

query: cleanup store statuses as they come and go

9015c2c

Signed-off-by: Adrien Fillon <adrien.fillon@cdiscount.com>

adrien-f force-pushed the feat/cleanup-store-statuses branch from 4ad70e0 to 9015c2c Compare April 8, 2019 13:05

bwplotka approved these changes Apr 8, 2019

View reviewed changes

bwplotka merged commit c0bfe35 into thanos-io:master Apr 8, 2019

bwplotka deleted the feat/cleanup-store-statuses branch April 8, 2019 14:02

bwplotka mentioned this pull request Apr 16, 2019

[UI] Hide unhealthy stores after some time #748

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

query: cleanup store statuses as they come and go #910

query: cleanup store statuses as they come and go #910

adrien-f commented Mar 12, 2019 •

edited

bwplotka left a comment

bwplotka commented Mar 12, 2019

adrien-f commented Mar 16, 2019

adrien-f commented Mar 17, 2019 •

edited

bwplotka commented Mar 18, 2019

adrien-f commented Mar 18, 2019

bwplotka left a comment

bwplotka Mar 26, 2019

adrien-f Mar 26, 2019

bwplotka Apr 8, 2019

bwplotka Mar 26, 2019

adrien-f Mar 26, 2019

bwplotka left a comment

bwplotka Apr 8, 2019

bwplotka Apr 8, 2019

adrien-f Apr 8, 2019

bwplotka Apr 8, 2019

bwplotka left a comment

query: cleanup store statuses as they come and go #910

query: cleanup store statuses as they come and go #910

Conversation

adrien-f commented Mar 12, 2019 • edited

Changes

Verification

bwplotka left a comment

Choose a reason for hiding this comment

bwplotka commented Mar 12, 2019

adrien-f commented Mar 16, 2019

adrien-f commented Mar 17, 2019 • edited

bwplotka commented Mar 18, 2019

adrien-f commented Mar 18, 2019

bwplotka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

adrien-f commented Mar 12, 2019 •

edited

adrien-f commented Mar 17, 2019 •

edited