thanos icon indicating copy to clipboard operation
thanos copied to clipboard

compact: Ensure downsampled chunks are not larger than 120 samples; stream downsampling more

Open bwplotka opened this issue 5 years ago • 30 comments
trafficstars

During bug fixing on https://github.com/thanos-io/thanos/pull/2528 I found that downsampling always encodes whatever is given in the block into huge chunks. This can lead to inefficiency during query time when only a small part of chunk data is needed, but store GW needs to fetch and decode everything.

See: https://github.com/thanos-io/thanos/blob/55cb8ca38b3539381dc6a781e637df15c694e50a/pkg/compact/downsample/downsample.go#L141

AC:

  • Downsampled chunks are not larger than 120 samples
  • Chunks are expanded on demand in iterator.

bwplotka avatar Apr 30 '20 12:04 bwplotka

If no one is working on this issue, can I have a try? @bwplotka

yashrsharma44 avatar May 08 '20 00:05 yashrsharma44

I assume you are not on it right? (: Just making sure, so others can help as well (:

bwplotka avatar May 23 '20 07:05 bwplotka

Yeah, apologies for keeping blocked, I am not working on it currently 😅

yashrsharma44 avatar May 23 '20 07:05 yashrsharma44

I am happy to work on this.

jyizheng avatar May 23 '20 07:05 jyizheng

Hello 👋 Looks like there was no activity on this issue for last 30 days. Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

stale[bot] avatar Jun 22 '20 08:06 stale[bot]

Closing for now as promised, let us know if you need this to be reopened! 🤗

stale[bot] avatar Jun 29 '20 08:06 stale[bot]

Still relevant AFAIK.

GiedriusS avatar Jun 30 '20 16:06 GiedriusS

Hi, I still want to work on this. I will get back to you guy soon.

jyizheng avatar Jun 30 '20 18:06 jyizheng

Hello 👋 Looks like there was no activity on this issue for last 30 days. Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

stale[bot] avatar Jul 30 '20 19:07 stale[bot]

Closing for now as promised, let us know if you need this to be reopened! 🤗

stale[bot] avatar Aug 06 '20 21:08 stale[bot]

Hello 👋 Looks like there was no activity on this issue for last 30 days. Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

stale[bot] avatar Sep 05 '20 22:09 stale[bot]

Closing for now as promised, let us know if you need this to be reopened! 🤗

stale[bot] avatar Sep 12 '20 22:09 stale[bot]

Is this still valid? If not, which PR fixed it?

pracucci avatar Sep 14 '20 09:09 pracucci

Hello 👋 Looks like there was no activity on this issue for the last two months. Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

stale[bot] avatar Nov 13 '20 17:11 stale[bot]

I think this is still valid

pracucci avatar Nov 14 '20 14:11 pracucci

Hi, two questions I have about this issue.

  1. There is already a limit of 140 samples per chunk for downsampled chunks here. I guess the performance should be similar to 120 samples?

  2. What's the benefit of Chunks are expanded on demand in iterator.? I see that we need to expand all the chunks before the downsampling anyway. So what is the doing it on demand?

Thanks!

yeya24 avatar Nov 14 '20 23:11 yeya24

  1. There is already a limit of 140 samples per chunk for downsampled chunks here. I guess the performance should be similar to 120 samples?

Yes, performances should be similar and 140 should be a problem. However, I'm wondering if that 140 value was a "typo" of 120 or intentional 🤔

pracucci avatar Nov 17 '20 13:11 pracucci

  1. Does not matter 120 / 140. If there is check I might miss it and all good (: Let's double check.

What's the benefit of Chunks are expanded on demand in iterator.? I see that we need to expand all the chunks before the downsampling anyway. So what is the doing it on demand?

What I mean is to not expand all of chunks and only then process query, just instead one, process sample, then another and process those and another etc... (streaming)

bwplotka avatar Nov 17 '20 14:11 bwplotka

Hello 👋 Looks like there was no activity on this issue for the last two months. Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

stale[bot] avatar Jan 16 '21 16:01 stale[bot]

Closing for now as promised, let us know if you need this to be reopened! 🤗

stale[bot] avatar Jan 30 '21 23:01 stale[bot]

Still valid

yeya24 avatar Jan 31 '21 00:01 yeya24

Hey @bwplotka Can you elaborate on this - link

What I mean is to not expand all of chunks and only then process query, just instead one, process sample, then another and process those and another etc... (streaming)

yashrsharma44 avatar Apr 05 '21 19:04 yashrsharma44

Hello 👋 Looks like there was no activity on this issue for the last two months. Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

stale[bot] avatar Jun 05 '21 00:06 stale[bot]

Closing for now as promised, let us know if you need this to be reopened! 🤗

stale[bot] avatar Jun 19 '21 07:06 stale[bot]

#4275 fixes this

yeya24 avatar Jun 20 '21 03:06 yeya24

Hello 👋 Looks like there was no activity on this issue for the last two months. Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

stale[bot] avatar Aug 21 '21 03:08 stale[bot]

Closing for now as promised, let us know if you need this to be reopened! 🤗

stale[bot] avatar Sep 04 '21 23:09 stale[bot]

Hello 👋 Looks like there was no activity on this issue for the last two months. Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

stale[bot] avatar Jan 09 '22 09:01 stale[bot]

Hello 👋 Looks like there was no activity on this issue for the last two months. Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

stale[bot] avatar Apr 17 '22 04:04 stale[bot]

Hello 👋 Looks like there was no activity on this issue for the last two months. Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

stale[bot] avatar Sep 21 '22 06:09 stale[bot]