prometheus / compliance

A set of tests to check compliance with various Prometheus interfaces
Apache License 2.0
128 stars 45 forks source link

Set offset for the query end time to -12 minutes #95

Closed nikola-milikic closed 2 years ago

nikola-milikic commented 2 years ago

When running the PromQL Compliance test, we can sometimes observe the queries with negative offset failing. The reason for this is that by default query end time is now() - 2m. And we have queries with negative offsets of -5m and -10m. That, generally, means that we are querying into the future for 3m for the first query (with offset -5), and 8m for the second query (with offset -10). When running the compliance test, sometimes a race condition can be observed with data ingestion where one storage still hasn't ingested the same sample that the other system already has ingested. As a result, queries with negative offsets into the future can fail since the resulting time series will have additional samples at the end (the one that the other storage system still hasn't ingested at that point in time). This issue is raised in #94.

This PR offsets the query end time to -12m (previously it was -2m). This will ensure that even for the query with offset -10 we don't query into the future and thus we avoid the ingestion race condition problem.

juliusv commented 2 years ago

Thanks! Sounds like a decent workaround to me for the time being. The original 2m was before negative offsets were added, just to give implementations 2 minutes to ingest data. To keep that same amount of ingestion tolerance as before (but working for negative offsets), let's maybe change it to 12m instead of 11m?

nikola-milikic commented 2 years ago

Thanks for the comment! That makes sense. I have pushed the change with -12m.

juliusv commented 2 years ago

:+1: Thanks! :)