Worry more about fingerprint clashes

[bboreham]

The chunk store fetch code assumes that fingerprints uniquely identify a timeseries; this is fairly likely when they are looking at a single metric, but we still could get clashes.

Perhaps use the sha256 from the index?

[gouthamve]

This is the relevant bits: https://github.com/cortexproject/cortex/blob/master/pkg/querier/chunk_store_queryable.go#L51-L68

The idea is that 2 different series could have same fingerprint and then we just say the chunks for both belong to one.

[pracucci]

From a quick look through the code, I've the feeling that we also suffer hash collisions in results cache too (see cache.HashKey usage).

[bboreham]

Not fixed by #3192 which addresses a similar problem in the querier.

I was thinking of code like this:

cortex/pkg/chunk/series_store.go

Line 248 in 0a33f9d

filtered, keys := filterChunksByUniqueFingerprint(filtered)

I also found

cortex/pkg/distributor/distributor.go

Line 717 in 0a33f9d

metrics[m.Fingerprint()] = m

cortex/pkg/distributor/query.go

Lines 114 to 120 in 0a33f9d

	fp := ss.Metric.Fingerprint()
	mss, ok := fpToSampleStream[fp]
	if !ok {
	mss = &model.SampleStream{
	Metric: ss.Metric,
	}
	fpToSampleStream[fp] = mss

[bboreham]

Whilst the first example can be dropped when we deprecate chunks (#4268), the others still seem to be valid.