Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

store/compactor: Improve index-cache logic. #942

Closed
3 tasks done
bwplotka opened this issue Mar 19, 2019 · 7 comments · Fixed by #1952
Closed
3 tasks done

store/compactor: Improve index-cache logic. #942

bwplotka opened this issue Mar 19, 2019 · 7 comments · Fixed by #1952
Labels
component: compact component: store difficulty: medium feature request/improvement priority: P0

Comments

@bwplotka
Copy link
Member

bwplotka commented Mar 19, 2019 •
edited
Loading

AC:

  • [Precompute this in compactor
  • Move index-cache out of JSON to some flatbuffer/protobuf/binary
  • Store should just download them.

@xjewer question:
https://improbable-eng.slack.com/archives/CADR0UWS2/p1552931827180200

yep, but store has to have cache component as well, as sidecar sends block with no manipulations with data.
I hope I’ll have time this week to add this, so if you have any other ideas about this, you are welcome.
I personally want to split this work on 2 PRs:

  1. Store cache in object storage
  2. Implement binary format for cache and optimize store component to use it instead of loading/parsing json.

@bwplotka wdyt?

Fully agree with this 👍
@xjewer
Copy link
Contributor

xjewer commented Mar 19, 2019

What to do with 2 hours block uploaded by sidecar? Store component should get them right after they appear at the object storage.

as a workaround: if there's no index-cache, precompute in on the fly and use, ideally it should work well, but if compactor halts, it could be a problem for store not only work with non-compacted data, but also with calculation cache on each restart.

Thus the best way to solve it is to do cache on both sides: compactor to store cache to objstore and store to do index cache locally on store side.

@bwplotka bwplotka mentioned this issue Mar 19, 2019
Closed
5 tasks
@bwplotka
Copy link
Member Author

Good point. Alternative is to do that on sidecar, but I think having store gw doing that makes sense more - it can survive lack of compactor AND sidecar being misconfigured.

Also it fixes big problem of upgrade model (backward and forward compatibility) -> we should support system being in partial version. E.g old compactor and new store gw and vice versa.

@bwplotka
Copy link
Member Author

Can we assume you are on it @xjewer ?

@bwplotka bwplotka mentioned this issue Mar 26, 2019
Closed
@xjewer xjewer mentioned this issue Mar 28, 2019
Merged
bwplotka pushed a commit that referenced this issue Apr 12, 2019
@xjewer @bwplotka
store+compactor: pre-compute index cache during compaction (#986)
e6d5b49 
Fixes first part of #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>
@bwplotka
Copy link
Member Author

First part is done, now @GiedriusS ' #1013

smalldirector pushed a commit to smalldirector/thanos that referenced this issue Jun 20, 2019
Rebase upstream 0.4.0 (thanos-io#9)
1dfa865 
* 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
Copy link
Member Author

To sum up, last item on the list is still index-cache JSON being .. JSON (: @brancz also mentioned as alternative https://github.com/json-iterator/go

This was referenced Nov 1, 2019
Closed
Closed
@bwplotka
Copy link
Member Author

Just to make sure our goals here is not only size but actually even more:

  • data actually allocated in the memory.
  • latency of loading index-cache/index-header

@bwplotka
Copy link
Member Author

bwplotka commented Dec 2, 2019 •
edited
Loading

The current plan is to move index-cache to binary format. The proposal with rationales is in progress.

This was referenced Jan 10, 2020
Merged
Merged
brancz pushed a commit to brancz/objstore that referenced this issue Jan 28, 2022
@xjewer @bwplotka
store+compactor: pre-compute index cache during compaction (#986)
b2cc40d 
Fixes first part of thanos-io/thanos#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: compact component: store difficulty: medium feature request/improvement priority: P0
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Added binary index header implementation. thanos-io/thanos
2 participants
@xjewer @bwplotka