Fix pushdown with deduplication on

[GiedriusS]

Add some cases for pushdown with deduplication to test how it works.

There are some bugs here :'/

Fix them by creating a new series iterator that performs the given function over all replicas.

[GiedriusS]

Here are some test cases to show some bugs in pushdown, unfortunately :/ cc @fpetkovski

[GiedriusS]

Actually, I think what happens is that query_range aligns the timestamps for us so then the deduplication doesn't work properly since they are all the same and the dedup depends on the timestamps :/

[fpetkovski]

Hm, I wonder if we could inject a synthetic label when a query is evaluated in the sidecar.

[GiedriusS]

I think we could leverage series response hints for that. I'd prefer not to add metadata to labels to avoid allocations. In my opinion, we could solve the problem shown in these tests with a custom iterator that performs these functions (max/min) on top of all series from each replica to replicate the old behavior. What do you think, @fpetkovski ?

[GiedriusS]

I have pushed a new pushdown series iterator that performs the given function over all replicas before returning the result. What do you think about this approach @bwplotka @fpetkovski ? Just need to decide how we'd like to indicate in SeriesResponse that results have been precomputed - via labels, via hints, or some other mechanism. I'd like to know your thoughts 🤗

[fpetkovski]

Can we add a test for the case where Querier is pushing a query to 2 different sets of replicated sidecars. For example, s1 and s2 being replicas of each other, and s3 and s4 also being replicas of each other. I am wondering what would happen if s1 and s3 have overlapping series, would the new iterator deduplicate them?

[GiedriusS]

Can we add a test for the case where Querier is pushing a query to 2 different sets of replicated sidecars. For example, s1 and s2 being replicas of each other, and s3 and s4 also being replicas of each other. I am wondering what would happen if s1 and s3 have overlapping series, would the new iterator deduplicate them?

Could you please elaborate on this with some examples? Because the deduplication should work the same.

I have pushed v2 of this change:

• Now the single dedup iterator handles interleaved cases no matter from where the data came from;
• All pushed-down series are passed through dedupSeriesSet. All pushed down series are shoved into pushdownSeriesIterator and then it becomes a regular series in dedupSeries.

I think this approach is much cleaner and handles all cases. PTAL @bwplotka @fpetkovski 🙏

[GiedriusS]

@yeya24 maybe this would be interesting to you as well

[GiedriusS]

Made a small picture showing how it works. @fpetkovski @bwplotka friendly ping 🤗

[GiedriusS]

Given the upcoming release and no other comments, I'm likely to merge this to avoid shipping a broken feature. Plus, everything is behind ifs so there should be no performance impact if a user does not have push down enabled. Next up on the list: #5069.