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.

Sign up for GitHub

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

Jump to bottom

Stale marker accuracy #85

Merged
merged 1 commit into from
Dec 19, 2022
Merged

Stale marker accuracy #85

merged 1 commit into from
Dec 19, 2022

Conversation

codebien
Copy link
Contributor

@codebien codebien commented Dec 15, 2022 •
edited

It introduces a new option K6_PROMETHEUS_RW_STALE_MARKERS=<bool> for enabling the Staleness marking process at the end of the test. It also disables the option by default. Moved to #86

It adjusts the stale marker accuracy by adding slight padding.

@codebien codebien changed the title Stale marker timestamp Stale marker updates Dec 15, 2022
@codebien codebien self-assigned this Dec 15, 2022
@codebien codebien force-pushed the stale-marker-timestamp branch 2 times, most recently from b4feac3 to e8c1012 Compare December 15, 2022 09:16
@codebien codebien marked this pull request as ready for review December 15, 2022 09:21
@imiric imiric self-requested a review December 15, 2022 09:49
imiric
imiric suggested changes Dec 15, 2022
View reviewed changes
Copy link

@imiric imiric left a comment

Choose a reason for hiding this comment

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

The new option LGTM, but I think some test issues should be addressed.

pkg/remote/client_test.go Outdated Show resolved Hide resolved
pkg/remote/client_test.go Outdated Show resolved Hide resolved
pkg/remote/client_test.go Outdated Show resolved Hide resolved
pkg/remotewrite/remotewrite.go Outdated Show resolved Hide resolved
pkg/remote/client_test.go Outdated Show resolved Hide resolved
pkg/remotewrite/remotewrite_test.go Outdated Show resolved Hide resolved
pkg/remotewrite/remotewrite_test.go Outdated Show resolved Hide resolved
pkg/remotewrite/config_test.go Outdated Show resolved Hide resolved
@codebien codebien changed the title Stale marker updates Stale marker accuracy Dec 15, 2022
@codebien
Copy link
Contributor Author

codebien commented Dec 15, 2022 •
edited

Changes addressed in #86. I created a new PR to have a consistent branch name. I will keep this for timestamp accuracy.

@codebien codebien force-pushed the stale-marker-timestamp branch 4 times, most recently from 395ee7f to 2943fac Compare December 16, 2022 14:29
@codebien codebien requested a review from imiric December 16, 2022 14:36
olegbespalov
olegbespalov approved these changes Dec 19, 2022
View reviewed changes
Copy link
Contributor

@olegbespalov olegbespalov left a comment

Choose a reason for hiding this comment

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

Generally looks good 👍 (with the idea in my mind that I'm not familiar with the topic)

One thing that could be improved since we have time manipulation is to have more test cases, which could probably be done in the following PRs.

imiric
imiric approved these changes Dec 19, 2022
View reviewed changes
Copy link

@imiric imiric left a comment

Choose a reason for hiding this comment

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

I haven't tested this, but your explanation and test change makes sense. 👍

@codebien codebien merged commit 7aba207 into main Dec 19, 2022
@codebien codebien deleted the stale-marker-timestamp branch December 19, 2022 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@imiric imiric imiric approved these changes

@olegbespalov olegbespalov olegbespalov approved these changes

Assignees

@codebien codebien

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@codebien @imiric @olegbespalov