store: fix deduplication in proxy heap for series with multiple chunks

[MichaHoffmann]

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

Changes

Consider the hash of the full chunk in deduplication

Verification

Added unittest case

Supposed to fix #6661

[MichaHoffmann]

The e2e tests work for me locally :thinking:

[MichaHoffmann]

The fix itself LGTM but now that we know about this bug, the Hash field in *storepb.Chunk becomes kind of useless. Perhaps we could add a Hash field to storepb.AggrChunk in a separate PR before merging this? That hash field would encompass of the different fields. It's needed so that the querier wouldn't have to hash everything again 😄

Storing hash in AggrChunk sounds correct! for backwards compatibility we need to do it in the querier for a bit right? I worry about the failing e2e test in CI that pass for me locally. Do you have an idea why that could be ?

[GiedriusS]

The fix itself LGTM but now that we know about this bug, the Hash field in *storepb.Chunk becomes kind of useless. Perhaps we could add a Hash field to storepb.AggrChunk in a separate PR before merging this? That hash field would encompass of the different fields. It's needed so that the querier wouldn't have to hash everything again 😄

Storing hash in AggrChunk sounds correct! for backwards compatibility we need to do it in the querier for a bit right? I worry about the failing e2e test in CI that pass for me locally. Do you have an idea why that could be ?

I'll try to take a look but just adding a new Hash field shouldn't trigger them so I think we can do that change safely as it looks like we will need it either way

[MichaHoffmann]

Note: e2e tests worked for me previously because i didnt rebuild the docker image.