Heatmap intervals

[mkasztelnik]

Interval for heat map readings (current and iteration)

Only data from concrete interval is returned. Following cases are taken into account:

• iteration interval (was newest reading starting from to)
• Time.now - interval for latest readings (was last readings)
• Last readings is used to set cache interval

The cache expires_at for last readings are not good enough, because we are taking newest result to calculate expires_at, oldest result should be taken into account. But to do this additional db query need to be performed (Reading.where('measurement_id = ? AND time > ?', params[:id], Time.current - interval.minutes).minimum(:time)).

@Nuanda do you think we should do this or give up cache in this case for now?

Fixes #55

[mkasztelnik]

Last commit implements correct interval calculation for last readings.

[Nuanda]

What can't we just set the last_reading_expires_in simply to, for instance, 2 x interval.minutes? This setting is only about cache cleaning, not about whether the cache hit happens or not. In case a new reading does not arrive in 2*interval, simply a single request will go without the cache.

[mkasztelnik]

From the #55 description I understood that interval should be respected by both modes: newest readings and iterations. As a conclusion we should not return values older than 15 minutes when asking about newest readings - that is why expires_at need to be calculated basing on oldest reading time. If this is not a case and newest readings can return values older than 15 minutes than sure we can use (2 * interval).minutes but then query for newest values need to be changed.

[Nuanda]

I'm not sure this is what expires_in does. It has nothing to do with the semantics of whether a cache key is valid or not. It only tells rails to remove that cache entry (for space preservation purposes), but I think this will be regenerated by another similar request, as long as the key does not change.

What I mean, if the cache expires, but the key_date does not change (after 15 minutes it's still the same last_reading_time), the function will get the same data, and will recreate the cache for another 15 minutes. If you want the function to stop returning that data if it's older than 15 minutes, I think you should use another mechanism (not the expires_in setting).

[mkasztelnik]

I believe that it is right tool to do the job. From documentation: "Setting :expires_in will set an expiration time on the cache.".

In our case cache should not be used in 2 cases:

1. new reading is created
2. in cached result record older than 15 occurs

Point no. 1 is solved by using key_date set to last reading time, point no. 2 is solved by using concrete expires_at time.

[Nuanda]

Well, that's more or less why I introduced the idea of interval number (counted from midnight) - to simplify that kind of computations. Wouldn't combining last_reading_time with interval_number in the key, for the "current situation case" help it, without making another DB query?

Of course this introduced hard division lines, but making query (when we miss the cache in, e.g., the first second of a new interval) for "real last 15 minutes" (not for the interval counted from midnight) might help.

[Nuanda]

I have just added a new key to cache, that checks the number of readings in the last interval minutes (I need this for another functionality). Do you think this might also help with he case here?

[mkasztelnik]

count will not help to solve this issue. If you want we can come back into last readings as readings from last 15 minutes interval. If would be trivial to convert current code to this case, but I thought that it is not good enough - only newest results should be returned.

Instead of adding count into cache key you can use Rails.cache.delete while creating new record in time older than current.

[Nuanda]

Are you sure? If a reading falls out of the last 15 min interval, the count of the query will change, and therefore the key will no longer match the cache key, so the data query will be performed again.

[mkasztelnik]

I'm not saying that your solution will not work :smile: The question is which operation will occur more frequently: (1) reading intervals or (2) creating old records. If 1 then using cache invalidation will save 1 query each time interval is fetched. If 2 then your solution is better than cache invalidation.

But right now it is premature optimization. Lets wait for real usage.

[Nuanda]

Oh, you misread my comment. I'm not commenting the Rails.cache.delete suggestion. I'm suggesting that now, when we count readings in the interval that we are interested in, your approach with complex expiration time computation might not be needed.

[mkasztelnik]

Oooo now I got it. It should. I will try to check if all tests pass.

[mkasztelnik]

Looks like everything works as expected after using readings count. I just commit change with simple expires_at set to interval.minutes