Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

correct Metricdef.LastUpdate when loading from persistent index #1983

Merged
merged 2 commits into from
Jun 3, 2021

Conversation

Dieterbe
Copy link
Contributor

@Dieterbe Dieterbe commented Jun 3, 2021

(instead of at previous fix, which did it at query time for Find(),
see #1532)

This has two main benefits:

  1. by making the change to the data, it works equally well across all
    types of queries, in particular this fixes the behavior for TagQuery
  2. we no longer over-eagerly adjust the check at query time (if MT has
    seen a new point for a given metric - e.g. if the process has been up for
    a while - than the LastUpdate value in the memory index is perfectly
    accurate, and we don't need to make any adjustment)

fix #1979

(instead of at previous fix, which did it at query time for Find(),
see #1532)

This has two main benefits:
1) by making the change to the data, it works equally well across all
   types of queries, in particular this fixes the behavior for TagQuery
2) we no longer over-eagerly adjust the check at query time (if MT has
   seen a new point for a given metric - e.g. if the process has been up for
   a while - than the LastUpdate value in the memory index is perfectly
   accurate, and we don't need to make any adjustment)

fix #1979
@Dieterbe Dieterbe requested a review from shanson7 June 3, 2021 13:48
Copy link
Collaborator

@shanson7 shanson7 left a comment

Choose a reason for hiding this comment

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

Small comments on comments.

One interesting bit here is that all nodes should now have the same update interval (whether they are configured to update the index or not).

idx/bigtable/bigtable.go Outdated Show resolved Hide resolved
idx/cassandra/cassandra.go Outdated Show resolved Hide resolved
Co-authored-by: Sean Hanson <shanson7@bloomberg.net>
@Dieterbe Dieterbe merged commit d8e2ff9 into master Jun 3, 2021
@Dieterbe Dieterbe deleted the fix-lastUpdate-1979 branch June 3, 2021 14:47
@shanson7
Copy link
Collaborator

shanson7 commented Jun 3, 2021

Thanks for that quick turnaround! I know a couple of users that will be glad for this fix when we roll it out.

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

Successfully merging this pull request may close these issues.

Stored 'lastupdate' being approximate can cause inconsistent missing data
2 participants