Skip to content

Commit

Permalink
fix(checker): Do not fetch extra metric data before lastCheck.Timestamp
Browse files Browse the repository at this point in the history
lastCheck.Timestamp denotes a most recent point in time when a trigger
was checked. While checking trigger, we are interested only in metric
points after lastCheck.Timestamp, because all previous points should
have been checked during the previous check. There is no obvious reason
to fetch more historical data points. At least, there is no test that
shows why we should do that.

Why is this bad? Because we don't have any limits on TTL value set by
user. Trying to fetch 1e15 metric points from a data source leads to OOM
death of all checker instances.

Close #190
  • Loading branch information
beevee committed Mar 25, 2020
1 parent ea96709 commit d23bc6a
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 12 deletions.
9 changes: 1 addition & 8 deletions checker/trigger_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func MakeTriggerChecker(triggerID string, dataBase moira.Database, logger moira.
metrics: metrics.GetCheckMetrics(&trigger),
source: source,

from: calculateFrom(lastCheck.Timestamp, trigger.TTL),
from: lastCheck.Timestamp,
until: until,

triggerID: triggerID,
Expand Down Expand Up @@ -98,10 +98,3 @@ func getTTLState(triggerTTLState *moira.TTLState) moira.TTLState {
}
return moira.TTLStateNODATA
}

func calculateFrom(lastCheckTimestamp, triggerTTL int64) int64 {
if triggerTTL != 0 {
return lastCheckTimestamp - triggerTTL
}
return lastCheckTimestamp - 600
}
8 changes: 4 additions & 4 deletions checker/trigger_checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func TestInitTriggerChecker(t *testing.T) {
ttl: trigger.TTL,
ttlState: *trigger.TTLState,
lastCheck: &lastCheck,
from: lastCheck.Timestamp - ttl,
from: lastCheck.Timestamp,
until: actual.until,
}
So(*actual, ShouldResemble, expected)
Expand All @@ -139,7 +139,7 @@ func TestInitTriggerChecker(t *testing.T) {
State: moira.StateOK,
Timestamp: actual.until - 3600,
},
from: actual.until - 3600 - ttl,
from: actual.until - 3600,
until: actual.until,
}
So(*actual, ShouldResemble, expected)
Expand Down Expand Up @@ -168,7 +168,7 @@ func TestInitTriggerChecker(t *testing.T) {
State: moira.StateOK,
Timestamp: actual.until - 3600,
},
from: actual.until - 3600 - 600,
from: actual.until - 3600,
until: actual.until,
}
So(*actual, ShouldResemble, expected)
Expand All @@ -190,7 +190,7 @@ func TestInitTriggerChecker(t *testing.T) {
ttl: 0,
ttlState: moira.TTLStateNODATA,
lastCheck: &lastCheck,
from: lastCheck.Timestamp - 600,
from: lastCheck.Timestamp,
until: actual.until,
}
So(*actual, ShouldResemble, expected)
Expand Down

0 comments on commit d23bc6a

Please sign in to comment.