store: add consistency delay to fetch blocks #2009

khyatisoneji · 2020-01-17T15:56:06Z

Add consistency delay in store component to fetch blocks

khyatisoneji · 2020-01-17T19:47:37Z

Although this is a rough implementation of adding filters in store, I wanted to get feedback on this approach.

I have added two filters:
a. Consistency filter: This is to account for eventual consistency for storing data in external storage
b. Garbage Blocks filter: This is to filter out the blocks that will be deleted by compactor, so these blocks should not be synced by store.

Please let me know if there are any additional filters that I should apply and also if this is in the right direction.
I will add tests and also make the code more structured.

GiedriusS

Some small comments but overall looks good to me! Thanks a lot! Also, it seems like we have another case of flaky CI tests, will rerun it.

cmd/thanos/store.go

khyatisoneji · 2020-01-20T16:12:26Z

@GiedriusS the tests are still breaking, can you retrigger them? Also please check if that it because of code changes

GiedriusS

Yes, your changes are indeed related. It's a good opportunity to try to run the E2E tests on your machine locally and see what's wrong, and update / improve them. In the CircleCI page you can see the debug message block is too fresh for now being printed by Thanos Store which means that it has filtered some blocks which hasn't occurred before and hence most likely Thanos Query cannot find which StoreAPI nodes to query. This is because Thanos Store announces all of the label sets that it finds - you can see that in the /stores page.

pkg/block/fetcher.go

cmd/thanos/store.go

pkg/testutil/prometheus.go

pkg/compact/compact_e2e_test.go

test/e2e/store_gateway_test.go

khyatisoneji · 2020-01-24T19:50:31Z

The TestStoreGateway test has been failing.

The reason is that in this test, we are creating the block by calling tsdb LeveledCompactor's Write function.
In this function, it creates the folder using ulid.Now(), but here we want to use ulid.Now() - consistencyDelay as the folder to write to, since we want to use the default value of consistencyDelay in the test.

Is there anyway in which I can provide the id of the block to write to tsdb? Otherwise we have to use consistencyDelay as 0.

yeya24 · 2020-01-24T20:05:07Z

The TestStoreGateway test has been failing.

The reason is that in this test, we are creating the block by calling tsdb LeveledCompactor's Write function.
In this function, it creates the folder using ulid.Now(), but here we want to use ulid.Now() - consistencyDelay as the folder to write to, since we want to use the default value of consistencyDelay in the test.

Is there anyway in which I can provide the id of the block to write to tsdb? Otherwise we have to use consistencyDelay as 0.

AFAIK, there is no way to customize block id when you want to create a block in TSDB using the existing API

khyatisoneji · 2020-01-24T20:13:14Z

What would be an alternate way to test a block id that was created consistencyDelay time ago?

Our default value of consistencyDelay is 30m, so we want to only fetch the blocks which are older than 30m. If we use consistencyDelay as 30m, then the call to fetch blocks fails saying that the block is too fresh.
We cannot also mock the NewConsistencyDelayMetaFilter function.

Is there a way to test the retrieval of blocks that are old?

pkg/testutil/e2eutil/prometheus.go

bwplotka

Hi!

It's looking good, but I think we are trying to do same thing 3 times here:

duplicate block filter
garbage finder filter
garbage finer in compactor loop

Can we have... one way of dealing with duplicate/garbage blocks? ;p

cmd/thanos/store.go

cmd/thanos/compact.go

pkg/block/fetcher.go

pkg/compact/garbagecollector/garbage_collector.go

pkg/testutil/e2eutil/prometheus.go

cmd/thanos/store.go

pkg/compact/compact.go

docs/components/store.md

bwplotka

Awesome!!! Looking super nice, some last suggestions I think 👍

pkg/block/fetcher.go

pkg/compact/compact.go

pkg/testutil/e2eutil/prometheus.go

test/e2e/store_gateway_test.go

bwplotka

Amazing 👍

Good work I love this ❤️

LGTM, just minor nits.

pkg/block/fetcher.go

pkg/block/fetcher_test.go

pkg/testutil/e2eutil/prometheus.go

bwplotka

Thank you!

This looks amazing! Let's merge after rebase and super minor suggestion:muscle:

pkg/block/fetcher.go

Signed-off-by: khyatisoneji <khyatisoneji5@gmail.com>

bwplotka · 2020-02-18T11:32:45Z

👏 👏 👏

Signed-off-by: khyatisoneji <khyatisoneji5@gmail.com>

khyatisoneji force-pushed the store-consistency branch 2 times, most recently from 5a48716 to 9e9e53f Compare January 17, 2020 19:44

khyatisoneji force-pushed the store-consistency branch 2 times, most recently from 8d802d9 to e9ecbd6 Compare January 17, 2020 19:52

khyatisoneji requested review from bwplotka and GiedriusS January 17, 2020 19:57

GiedriusS reviewed Jan 18, 2020

View reviewed changes

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

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

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

khyatisoneji force-pushed the store-consistency branch 4 times, most recently from d012a46 to 568595b Compare January 20, 2020 15:50

GiedriusS reviewed Jan 22, 2020

View reviewed changes

pkg/block/fetcher.go Outdated Show resolved Hide resolved

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

khyatisoneji force-pushed the store-consistency branch 5 times, most recently from 3c68464 to 53230ac Compare January 24, 2020 18:49

yeya24 reviewed Jan 24, 2020

View reviewed changes

pkg/testutil/prometheus.go Outdated Show resolved Hide resolved

pkg/compact/compact_e2e_test.go Outdated Show resolved Hide resolved

test/e2e/store_gateway_test.go Outdated Show resolved Hide resolved

khyatisoneji force-pushed the store-consistency branch 2 times, most recently from 1a57ad2 to 1faaec1 Compare January 24, 2020 19:20

khyatisoneji force-pushed the store-consistency branch 5 times, most recently from b63cb1d to 3409c5b Compare January 29, 2020 17:10

khyatisoneji force-pushed the store-consistency branch from 3409c5b to 8dcd5ce Compare January 29, 2020 17:16

khyatisoneji force-pushed the store-consistency branch 7 times, most recently from dbcca66 to 76daceb Compare February 9, 2020 09:57

khyatisoneji requested a review from GiedriusS February 9, 2020 10:24

GiedriusS reviewed Feb 10, 2020

View reviewed changes

pkg/testutil/e2eutil/prometheus.go Outdated Show resolved Hide resolved

bwplotka requested changes Feb 10, 2020

View reviewed changes

khyatisoneji force-pushed the store-consistency branch from 76daceb to d6bed5f Compare February 13, 2020 06:33

khyatisoneji commented Feb 13, 2020

View reviewed changes

docs/components/store.md Show resolved Hide resolved

khyatisoneji force-pushed the store-consistency branch 4 times, most recently from f41be37 to 5fd6f5f Compare February 13, 2020 10:46

bwplotka reviewed Feb 13, 2020

View reviewed changes

khyatisoneji force-pushed the store-consistency branch from 5fd6f5f to be70e91 Compare February 13, 2020 17:31

bwplotka approved these changes Feb 14, 2020

View reviewed changes

khyatisoneji force-pushed the store-consistency branch from be70e91 to b01e437 Compare February 14, 2020 19:26

bwplotka approved these changes Feb 17, 2020

View reviewed changes

pkg/block/fetcher.go Show resolved Hide resolved

store: add consistency delay to fetch blocks

51d0baa

Signed-off-by: khyatisoneji <khyatisoneji5@gmail.com>

khyatisoneji force-pushed the store-consistency branch from b01e437 to 51d0baa Compare February 18, 2020 10:46

bwplotka merged commit 2e3ece1 into thanos-io:master Feb 18, 2020

vankop pushed a commit to monitoring-tools/thanos that referenced this pull request Feb 28, 2020

store: add consistency delay to fetch blocks (thanos-io#2009)

ef6721b

Signed-off-by: khyatisoneji <khyatisoneji5@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

store: add consistency delay to fetch blocks #2009

store: add consistency delay to fetch blocks #2009

khyatisoneji commented Jan 17, 2020

khyatisoneji commented Jan 17, 2020

GiedriusS left a comment

khyatisoneji commented Jan 20, 2020

GiedriusS left a comment •

edited

khyatisoneji commented Jan 24, 2020

yeya24 commented Jan 24, 2020 •

edited

khyatisoneji commented Jan 24, 2020

bwplotka left a comment

bwplotka left a comment

bwplotka left a comment

bwplotka left a comment

bwplotka commented Feb 18, 2020

store: add consistency delay to fetch blocks #2009

store: add consistency delay to fetch blocks #2009

Conversation

khyatisoneji commented Jan 17, 2020

khyatisoneji commented Jan 17, 2020

GiedriusS left a comment

Choose a reason for hiding this comment

khyatisoneji commented Jan 20, 2020

GiedriusS left a comment • edited

Choose a reason for hiding this comment

khyatisoneji commented Jan 24, 2020

yeya24 commented Jan 24, 2020 • edited

khyatisoneji commented Jan 24, 2020

bwplotka left a comment

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

bwplotka commented Feb 18, 2020

GiedriusS left a comment •

edited

yeya24 commented Jan 24, 2020 •

edited