Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add out-of-order sample support to the TSDB #269

Merged
merged 1 commit into from
Jun 22, 2022
Merged

Add out-of-order sample support to the TSDB #269

merged 1 commit into from
Jun 22, 2022

Conversation

codesome
Copy link
Member

This implementation is based on this design doc:
https://docs.google.com/document/d/1Kppm7qL9C-BJB1j6yb6-9ObG3AbdZnFUBYPNNWwDBYM/edit?usp=sharing

This commit adds support to accept out-of-order ("OOO") sample into the TSDB
up to a configurable time allowance. If OOO is enabled, overlapping querying
are automatically enabled.

@codesome
Copy link
Member Author

This replaces #268. Somehow having a single commit while manually adding co-authors removes the CLA requirement from the co-authors. (See 268, CLA fails because Dieter hasn't signed, but it does not fail here).

@codesome
Copy link
Member Author

We can consider merging this after #270 is merged and synced in this PR.

@jesusvazquez
Copy link
Member

jesusvazquez commented Jun 21, 2022

Lets get the OOOAllowance changes and the OOO WBL dir rename into this PR.

@codesome codesome force-pushed the merge-ooo branch 2 times, most recently from 22b25f4 to 260ee0f Compare June 22, 2022 10:15
@codesome codesome marked this pull request as ready for review June 22, 2022 10:17
Copy link
Member

@jesusvazquez jesusvazquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did take a look at some of the TODO comments that we can get rid of. Can you take a look at the comments @codesome?

tsdb/head_read.go Outdated Show resolved Hide resolved
tsdb/index/postings.go Outdated Show resolved Hide resolved
tsdb/index/postings.go Outdated Show resolved Hide resolved
tsdb/ooo_head.go Outdated Show resolved Hide resolved
@codesome
Copy link
Member Author

Opened #273 for TODOs

Copy link
Member

@jesusvazquez jesusvazquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preemptively approving knowing that you will rebase this PR with the changes from #273

Let's goooo 🎉 🎉 🎉 🎉

This implementation is based on this design doc:
https://docs.google.com/document/d/1Kppm7qL9C-BJB1j6yb6-9ObG3AbdZnFUBYPNNWwDBYM/edit?usp=sharing

This commit adds support to accept out-of-order ("OOO") sample into the TSDB
up to a configurable time allowance. If OOO is enabled, overlapping querying
are automatically enabled.

Signed-off-by: Ganesh Vernekar <[email protected]>

Co-authored-by: Jesus Vazquez <[email protected]>
Co-authored-by: Ganesh Vernekar <[email protected]>
Co-authored-by: Dieter Plaetinck <[email protected]>
@codesome codesome enabled auto-merge (squash) June 22, 2022 11:28
@codesome codesome merged commit df59320 into main Jun 22, 2022
@jesusvazquez jesusvazquez deleted the merge-ooo branch June 22, 2022 14:12
@pstibrany
Copy link
Member

This PR changes ChunkReader interface in a way that's incompatible with usage in Thanos and Mimir. We could fix Mimir, but unless you plan to vendor mimir-prometheus in Thanos, it needs to be reverted :(

@nwpuCfy
Copy link

nwpuCfy commented Jan 6, 2023

This PR changes ChunkReader interface in a way that's incompatible with usage in Thanos and Mimir. We could fix Mimir, but unless you plan to vendor mimir-prometheus in Thanos, it needs to be reverted :(

Did this mean TSDB that supports out of order data is not compatible with thanos? So I can't use it in cortex v1.11 that supports obs storage?

@codesome
Copy link
Member Author

codesome commented Jan 6, 2023

@nwpuCfy that comment was about code interface and not the data format. Thanos has switched to the new interface I think. And they use prometheus/prometheus, and not grafana/mimir-prometheus.

@jesusvazquez
Copy link
Member

@nwpuCfy As @codesome pointed out, thanos is now vendoring prometheus/prometheus were we ported the changes you see in this PR so there should not be any problems 👍 Use prometheus/prometheus as your reference for the official state of out-of-order support.

@nwpuCfy
Copy link

nwpuCfy commented Jan 13, 2023

@nwpuCfy As @codesome pointed out, thanos is now vendoring prometheus/prometheus were we ported the changes you see in this PR so there should not be any problems 👍 Use prometheus/prometheus as your reference for the official state of out-of-order support.
ok, thanks for reply

@nwpuCfy
Copy link

nwpuCfy commented Jan 13, 2023

@nwpuCfy that comment was about code interface and not the data format. Thanos has switched to the new interface I think. And they use prometheus/prometheus, and not grafana/mimir-prometheus.

thanks for reply

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants