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.

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

Do not allow users to set TTL larger than internal storage window #190

Closed
borovskyav opened this issue Dec 28, 2018 · 0 comments · Fixed by #519
Closed

Do not allow users to set TTL larger than internal storage window #190

borovskyav opened this issue Dec 28, 2018 · 0 comments · Fixed by #519

Comments

@borovskyav
Copy link
Contributor

borovskyav commented Dec 28, 2018

Сейчас при чеке триггера берется интервал с учетом TTL, тоесть есть TTL = 10 min, то мы возьмем промежуток равный:
from: время последнего чека - TTL
to: now.

Соответсвенно если TTL слишком большой то мы берем неоправданно большой промежуток, даже если за такой промежуток нет. Самое плохое, к чему это может привести это утечка всей памяти (оооочень большой TTL).

Самое простое решение: ограничить это время на фронте: максимальный промежуток - время хранения метрик.
Посложнее: разобраться, зачем мы чекаем за такой большой промежуток? Надеемся что метрики появятся? Может вообще не стоит чекать за такой большой промежуток?

@beevee beevee changed the title Защита от большого TTL Do not allow users to set TTL larger than internal storage window Dec 29, 2018
beevee added a commit that referenced this issue Mar 25, 2020
lastCheckTimestamp denotes a most recent point in time when a trigger
was checked. While checking trigger, we are interested only in metric
points after lastCheckTimestamp, 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
beevee added a commit that referenced this issue Mar 25, 2020
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
beevee added a commit that referenced this issue Mar 25, 2020
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant