compactor: Add offline deduplication #4239
Conversation
|
I think the failed E2E test is related to my change, but I am not sure why. |
There was a problem hiding this comment.
This is epic, If not for the file move this would have been 50 line change to add something so powerful. Well done!
I have a strong suggestion regarding the bool flag, otherwise LGTM!
One extra question: Are we sure penalty algo will work properly on 1:1 dups? (: (extra test would help)
yeya24
force-pushed
the
dedup-comp
branch
2 times, most recently
from
4fdedb0
to
143cd07
Compare
yeya24
force-pushed
the
dedup-comp
branch
2 times, most recently
from
9b005a3
to
3e11f2b
Compare
|
This works for downsampled blocks now. Here is one example. Each qurier runs against one bucket. The two buckets contain the same downsample level1 blocks and one bucket is deduplicated then. The result is almost the same.
|
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
yeya24
force-pushed
the
dedup-comp
branch
2 times, most recently
from
9b53a9e
to
f1a7888
Compare
yeya24
force-pushed
the
dedup-comp
branch
3 times, most recently
from
5a71d03
to
a3b4834
Compare
yeya24
force-pushed
the
dedup-comp
branch
7 times, most recently
from
1ece1ce
to
3a1c8ef
Compare
yeya24
force-pushed
the
dedup-comp
branch
2 times, most recently
from
ced8fda
to
1b9630c
Compare
Signed-off-by: yeya24 <[email protected]>
Signed-off-by: yeya24 <[email protected]>
Signed-off-by: yeya24 <[email protected]>
Signed-off-by: yeya24 <[email protected]>
Signed-off-by: yeya24 <[email protected]>
Signed-off-by: yeya24 <[email protected]>
|
Friendly ping @bwplotka. |
| this requires more complex deduplication algorithms. For example one that is used to [deduplicate on the fly on the Querier](query.md#run-time-deduplication-of-ha-groups). This is common | ||
| case when Prometheus HA replicas are used. [Offline deduplication for this is in progress](https://github.com/thanos-io/thanos/issues/1014). | ||
| case when Prometheus HA replicas are used. You can enable this deduplication via `--deduplication.func=penalty` flag. |
There was a problem hiding this comment.
Could we please clarify in the docs in exactly which 1% of the cases the penalty-based deduplication algorithm doesn't work?
There was a problem hiding this comment.
Maybe let's say something like:
even though the deduplication algorithm has been battle-tested for quite a long time but still very few issues come up from time to time such as https://github.com/thanos-io/thanos/issues/2890
? It would be more concrete. And it wouldn't sound like we know that it is bad in some 1% of cases & we aren't fixing it 😄
Signed-off-by: yeya24 <[email protected]>
Signed-off-by: yeya24 <[email protected]>
Signed-off-by: yeya24 <[email protected]>
…-comp Signed-off-by: yeya24 <[email protected]>
Signed-off-by: yeya24 <[email protected]>
Signed-off-by: yeya24 <[email protected]>
|
I feel it is ready to go unless @bwplotka has some comments. |
Signed-off-by: yeya24 [email protected]
Fixes #1014
Changes
Another attempt on offline deduplication using the existing querier's penalty deduplication algorithm.
This pr is WIP as I need to do more testing and is currently blocked by prometheus/prometheus#8836, so I use my own fork here.But as mentioned in #1014 (comment), this deduplication algorithm works for 99% of cases but is wrong in 1% case. So it has its own risk.
Verification
Tested with 2 Prometheus replicas running in k8s. I replicate the minio bucket to a local directory as a filesystem objstore for testing. The local directory is used for running dedup compactor. There are 12h test data and I run 1 thanos store and 1 thanos querier for each bucket.
Undeduped bucket:
Query deduplication has to be enabled.
Bucket size: 41m (36m if normal compaction is enabled)
Offline deduped bucket:
Query deduplication disabled.
Bucket size: 19m