Skip to content

write path: handle created timestamp #11977

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

krajorama
Copy link
Contributor

@krajorama krajorama commented Jul 4, 2025

What this PR does

Modify the ingester and block builder to accept and pass the Created timestamp field from incoming write requests to TSDB.

Note1: if samples aren't ordered by time in a remote write requests, then the result is undefined.

Note2: as with regular samples, Mimir will have non-deterministic behavior if there's a sample and a created timestamp with the same timestamp ingested on two different ingesters for whatever reason. During query and block building the resolution of such conflicting samples has always been non-deterministic, as there is no way to know at that point which sample is the correct one.

Which issue(s) this PR fixes or relates to

Fixes #11976

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]. If changelog entry is not needed, please add the changelog-not-needed label to the PR.
  • about-versioning.md updated with experimental features.

@krajorama krajorama force-pushed the krajo/created-timestamp branch 4 times, most recently from 24c0591 to b9e1a00 Compare July 4, 2025 11:33
@krajorama krajorama changed the title ingester: handle created timestamp write path: handle created timestamp Jul 4, 2025
@krajorama krajorama force-pushed the krajo/created-timestamp branch from b9e1a00 to 1efc74a Compare July 4, 2025 11:39
Copy link
Contributor

github-actions bot commented Jul 4, 2025

@krajorama krajorama force-pushed the krajo/created-timestamp branch 4 times, most recently from 36c90aa to 7774cee Compare July 5, 2025 10:39
@krajorama krajorama marked this pull request as ready for review July 5, 2025 10:39
@krajorama krajorama requested a review from a team as a code owner July 5, 2025 10:39
@krajorama krajorama marked this pull request as draft July 5, 2025 11:20
@krajorama krajorama force-pushed the krajo/created-timestamp branch 2 times, most recently from 88805c5 to 44740eb Compare July 5, 2025 11:54
@krajorama krajorama force-pushed the krajo/created-timestamp branch from 44740eb to c7ebe12 Compare July 5, 2025 11:58
@krajorama krajorama marked this pull request as ready for review July 5, 2025 11:58
@@ -31,7 +31,8 @@
* `memberlist.acquire-writer-timeout`: `1s`
These defaults perform better but may cause long-running packets to be dropped in high-latency networks.
* [CHANGE] Query-frontend: Apply query pruning and check for disabled experimental functions earlier in query processing. #11939
* [FEATURE] Distributor: Experimental support for Prometheus Remote-Write 2.0 protocol. Limitations: Created timestamp is ignored, per series metadata is merged on metric family level automatically, ingestion might fail if client sends ProtoBuf fields out of order. The label `version` is added to the metric `cortex_distributor_requests_in_total` with a value of either `1.0` or `2.0` depending on the detected Remote-Write protocol. #11100 #11101 #11192 #11143
* [FEATURE] Distributor: Experimental support for Prometheus Remote-Write 2.0 protocol. Limitations: if samples in a write request are not ordered by time, the created timestamp might be dropped, per series metadata is merged on metric family level automatically, ingestion might fail if client sends ProtoBuf fields out of order. The label `version` is added to the metric `cortex_distributor_requests_in_total` with a value of either `1.0` or `2.0` depending on the detected Remote-Write protocol. #11100 #11101 #11192 #11143 #11977
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* [FEATURE] Distributor: Experimental support for Prometheus Remote-Write 2.0 protocol. Limitations: if samples in a write request are not ordered by time, the created timestamp might be dropped, per series metadata is merged on metric family level automatically, ingestion might fail if client sends ProtoBuf fields out of order. The label `version` is added to the metric `cortex_distributor_requests_in_total` with a value of either `1.0` or `2.0` depending on the detected Remote-Write protocol. #11100 #11101 #11192 #11143 #11977
* [FEATURE] Distributor: Experimental support for Prometheus Remote-Write 2.0 protocol. This feature includes some limitations. If samples in a write request aren't ordered by time, the created timestamp might be dropped. Additionally, per-series metadata is automatically merged on the metric family level, and ingestion might fail if the client sends ProtoBuf fields out-of-order. The label `version` is added to the metric `cortex_distributor_requests_in_total` with a value of either `1.0` or `2.0`, depending on the detected remote-write protocol. #11100 #11101 #11192 #11143 #11977

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried to break this up a bit for clarity. Let me know if the meaning is preserved.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I changed the second occurrence of remote-write to lower-case, since we aren't reference a specific remote-write protocol in this case.

@@ -31,7 +31,8 @@
* `memberlist.acquire-writer-timeout`: `1s`
These defaults perform better but may cause long-running packets to be dropped in high-latency networks.
* [CHANGE] Query-frontend: Apply query pruning and check for disabled experimental functions earlier in query processing. #11939
* [FEATURE] Distributor: Experimental support for Prometheus Remote-Write 2.0 protocol. Limitations: Created timestamp is ignored, per series metadata is merged on metric family level automatically, ingestion might fail if client sends ProtoBuf fields out of order. The label `version` is added to the metric `cortex_distributor_requests_in_total` with a value of either `1.0` or `2.0` depending on the detected Remote-Write protocol. #11100 #11101 #11192 #11143
* [FEATURE] Distributor: Experimental support for Prometheus Remote-Write 2.0 protocol. Limitations: if samples in a write request are not ordered by time, the created timestamp might be dropped, per series metadata is merged on metric family level automatically, ingestion might fail if client sends ProtoBuf fields out of order. The label `version` is added to the metric `cortex_distributor_requests_in_total` with a value of either `1.0` or `2.0` depending on the detected Remote-Write protocol. #11100 #11101 #11192 #11143 #11977
* [FEATURE] Ingester/Block-builder: handle the Created timestamp field of write requests. #11977
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* [FEATURE] Ingester/Block-builder: handle the Created timestamp field of write requests. #11977
* [FEATURE] Ingester/Block-builder: Handle the timestamp field for write requests. #11977

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.

Idea: handle created timestamp field in Remote-Write (PRW) 2.0
2 participants