Fix ttl #5
Merged
Fix ttl #5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dehaansa
reviewed
Nov 26, 2024
| if seriesAge > q.ttl { | ||
| // TODO @mattdurham add metric here for ttl expired. | ||
| // Since we arent pushing the TS forward we should put it back into the pool. |
There was a problem hiding this comment.
Can this comment also explain what causes the expired series to be discarded, if it's not being discarded here?
There was a problem hiding this comment.
// For any series that exceeds the time to live (ttl) based on its timestamp we do not want to push it to the networking layer
// but instead drop it here by continuing.
The above sound reasonable?
There was a problem hiding this comment.
Sounds good. I had to get my head back into the codebase, the PutTimeSeriesIntoPool function sounds like you're returning data to the WAL, not returning the allocated object to be reused, at least when my head is not in the implementation.
dehaansa
approved these changes
Nov 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ensure the TTL is always comparing apples to apples.
Added a new metric for file vs network drift
Remove unneeded code