query: Fix counter resets for chain deduplication algorithm

[lachruzam]

• I added CHANGELOG entry for this change.
• Change is not relevant to the end user.

It's a fix to a not yet released feature.

Changes

Fix counter resets handling for chain deduplication algorithm by not applying resets on series replicas.
The feature issue: #7807

Verification

Added a new e2e test.

[lachruzam]

hey @yeya24 , @MichaHoffmann
Could you look at it?
Sorry for tagging directly but I hope to get this fix into 0.38 and just yesterday found the issue on our clusters.
The feature is labelled as experimental, however it's rather useless in the current state.

[MichaHoffmann]

Can we call it dedupAlgo?

[lachruzam]

hey @MichaHoffmann
Thanks for looking at it.

deduplicationFunc is already used in a few places since #7808
Originally, to stay in sync with compactor's --deduplication.func.

At the time I didn't want to propose any bigger refactoring or code duplication for an experimental feature. Still, I don't like how that configuration spread out. Maybe, it can be refactored later on, e.g. with the removal of the experimental label. Till then I'd prefer to stay with the same name in all places, WDYT?

[yeya24]

Thanks. LGTM

[yeya24]

How does this work for downsampled counter aggr chunk? Counter aggr chunk needs to use ApplyCounterReset iterator.

[lachruzam]

How does this work for downsampled counter aggr chunk? Counter aggr chunk needs to use ApplyCounterReset iterator.

A good question, it does not work in all cases.

Counter aggr chunks come only from stores after the downsampling.
If all chunks come from receivers then there are no problem.
It appears on the intersection of chunks from store and receivers.
Here, I'm assuming that for stores chain (one-to-one) deduplication has been already done by compactor.

So, there may be a need for overlaps removal for chunks coming from both receivers, stores.
In practice, I didn't notice the problem on the clusters, yet.

Let me know, if you seen any error in my assumptions.

Honestly, I missed that part in the first PR🤦. And as 0.38 is closing, I'd like to fix the issue for receivers at least.
As a next step, I plan to build tests for other chunk types but I'd need some time for it.

[MichaHoffmann]

Since this flag is experimental i dont mind if we merge this and followup with fixes later

[yeya24]

I think deduplication is not an issue but chunkSeriesIterator cannot deal with aggr counter chunk well as it has some special encoding mechanism.

// Counter aggregation chunks must have the first and last values from their
// original raw series: the first raw value should be the first value encoded
// in the chunk, and the last raw value is encoded by the duplication of the
// previous sample's timestamp.

I wonder how chunkSeriesIterator deals with samples with same timestamp but different values. I don't think there is any special handle logic there. But maybe it is fine to deal with it at the engine level.

[yeya24]

If all chunks come from receivers then there are no problem.

If chain deduplication is assumed to only work with receivers but not other types of chunks I am wondering if it is really usable. At least, we should document it so that users are aware of the issue and should only use it to query for receiver only data

[lachruzam]

If all chunks come from receivers then there are no problem.

If chain deduplication is assumed to only work with receivers but not other types of chunks I am wondering if it is really usable. At least, we should document it so that users are aware of the issue and should only use it to query for receiver only data

I expect it to work in all cases where are no overlap between raw and counter chunks. I'm working on a test that would verify it. If it's the case then still you're right that it should be documented.

[lachruzam]

@yeya24 You have been right on counters, adjusting is required even between aggregated chunks.

The last commit contains a new approach - chain dedup on a chunk level.
Here, the code path for penalty and chain dedup is separated in querier. Only raw blocks are chained together while aggregated are randomly dropped. Adjustment for counters is done but on already merged data.

I'm changing this PR to draft as the code is not yet cleaned up, necessary tests are missing and the change has been verified only on synthetic data.

Still, I'd appreciate any comments.