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

Provide a fast count of total measurements for all time #529

Closed
matschaffer opened this issue Dec 13, 2018 · 33 comments · Fixed by #678
Closed

Provide a fast count of total measurements for all time #529

matschaffer opened this issue Dec 13, 2018 · 33 comments · Fixed by #678
Assignees

Comments

@matschaffer
Copy link
Contributor

matschaffer commented Dec 13, 2018

We used to have this on https://api.safecast.org/en-US/measurements, but it would cause timeouts since counting the entire measurements table takes many minutes.

We'd like to get that number back and available without killing performance. Probably via pre-computation to a cached record (e.g., in the DB).

Ideally this should be a count of all radiation measurements we've collected during the safecast project:

  • api.safecast.org collection (should be all in the measurements table)
  • ingest.safecast.org collection (probably are also in the measurements table, or in Elasticsearch)

We should also consider how to make this data available for other measurement types (e.g., air quality data).

One thing to watch out for here is that ttserve has been dual publishing to both ingest & api a few years now. So we should be careful we're not doubling the final count.

@matschaffer
Copy link
Contributor Author

Labeling this as both API and ingest since ideally we'd have a count of all data

  • bgeigie uploads (api)
  • solarcast data (ingest/api)
  • pointcast data (ingest/api)

@sasharevzin
Copy link
Collaborator

@matschaffer I'm not sure I understand. What kind of cache are you talking to? Fragment cache?

@matschaffer
Copy link
Contributor Author

matschaffer commented May 6, 2020

Is there a good way to pre-compute fragment caches on a schedule? If so that might work. If not we could run a job to store it in a simple db record that just contains the most recently computed value.

@sasharevzin
Copy link
Collaborator

I checked the controller and view and didn't find any computing logic besides #count. What are computed values are you talking about?

@matschaffer
Copy link
Contributor Author

The value we’re looking for is a total of all radiation measurements (might be nice to have one for air too).

Similar to what @robouden and @auspicacious dug up for @nokton a few weeks back.

Nothing in the system computes this right now but we’d like it to.

@auspicacious
Copy link
Contributor

This might be a helpful reference. I think that this query would get the current measurement count for one month, but the performance is not sufficient to use live (which is why we need to cache):

SELECT count(id) FROM measurements WHERE captured_at < '2019-12-30T23:59:59Z' AND captured_at > '2019-11-01T00:00:00Z' AND unit = 'cpm';

I think that we should look for the places in the code where the measurements table is INSERTed into. That's probably one place we could generate the cache.

@sasharevzin
Copy link
Collaborator

This might be a helpful reference. I think that this query would get the current measurement count for one month, but the performance is not sufficient to use live (which is why we need to cache):

SELECT count(id) FROM measurements WHERE captured_at < '2019-12-30T23:59:59Z' AND captured_at > '2019-11-01T00:00:00Z' AND unit = 'cpm';

I think that we should look for the places in the code where the measurements table is INSERTed into. That's probably one place we could generate the cache.

I don't see the reason for caching plus it might be problematic with all these filters we have in place. It can be endless variations.

@auspicacious here the link to the example you provided and loads pretty fast to me: https://api.safecast.org/en-US/measurements?captured_after=2019%2F11%2F01+19%3A56%3A15&captured_before=2019%2F12%2F01+19%3A56%3A15&commit=Filter&distance=&latitude=&locale=en-US&longitude=&since=&unit=cpm&until=&utf8=%E2%9C%93

Is there an example URL that timing that doesn't include updated_at (#666)

@matschaffer
Copy link
Contributor Author

Maybe worth noting that the description in this issue is from 2018, when that page contained a total count of all measurements. That’s the value we’d like to have without tanking the page.

@sasharevzin
Copy link
Collaborator

Sounds like nothing to optimize here if these counts no longer on a page, no? Shell we close the ticket?

@matschaffer
Copy link
Contributor Author

Or reword it. We want the counts back

@matschaffer matschaffer changed the title Cache measurement count Provide a fast count of total measurements for all time May 7, 2020
@matschaffer
Copy link
Contributor Author

matschaffer commented May 7, 2020

I've updated the title and wording above to hopefully clarify the ask.

@sasharevzin
Copy link
Collaborator

So just to add a count?

Measurements____Safecast_API

@matschaffer
Copy link
Contributor Author

yeah. Doesn't have to be on that page necessarily, but it's where people are used to looking for it

@sasharevzin
Copy link
Collaborator

I think I found these pages:
https://api.safecast.org/en-US/measurements/count
https://api.safecast.org/en-US/measurements/count.json

So yea, we can cache it

@seanbonner
Copy link
Member

We're pulling it on the front page of https://safecast.org/ now too without any speed issues, so maybe a cache already exists? @norcross might know.

@sasharevzin
Copy link
Collaborator

We're pulling it on the front page of https://safecast.org/ now too without any speed issues, so maybe a cache already exists? @norcross might know.

Not for the page above. I assume that safecast.org pull the count and caches on its own side

@matschaffer
Copy link
Contributor Author

@seanbonner yeah, would be nice to know how that number is getting created.

@matschaffer
Copy link
Contributor Author

Screenshot_5_7_20__10_13_AM

It's the first I'd heard that this count existed. If it's fast/accurate enough then maybe we can just close this issue.

@sasharevzin
Copy link
Collaborator

sasharevzin commented May 7, 2020

According to https://api.safecast.org/en-US/measurements/count (143,634,213), it's off ~300k

Safecast_API

@seanbonner
Copy link
Member

I just spoke to @norcross and he's about to crash but will chime in tomorrow - he's pulling from an API endpoint that someone here gave him that was supposed to be cached every 12 hours, so maybe that was recently broken as well. He'll clarify when he wakes up.

@sasharevzin
Copy link
Collaborator

Is there a repo for safecast.org? I can see the archive, but I guess it hosted somewhere else now

@seanbonner
Copy link
Member

Deferring to @norcross again, but if I remember correctly he was planning to put the code up there once the site was launched (earlier this year) though that obviously hasn't happened yet.

@auspicacious
Copy link
Contributor

Since we've found this endpoint, we should probably look at the code for that before deciding what to do. I see two counter_cache declarations but I'm not familiar enough with ActiveRecord to know how that works right away.

Also, you'll notice that in the SQL query I described above, I'm filtering by unit = 'cpm' on Rob's advice that measurements in that table that use units other than CPM are calculated from CPM measurements. (This also ignores air quality measurements).

https://github.com/Safecast/safecastapi/blob/master/app/controllers/measurements_controller.rb#L104

https://github.com/Safecast/safecastapi/blob/master/app/models/measurement.rb#L12

@sasharevzin
Copy link
Collaborator

Since we've found this endpoint, we should probably look at the code for that before deciding what to do. I see two counter_cache declarations but I'm not familiar enough with ActiveRecord to know how that works right away.

These counter_cache not related to the total count. It's just when you want to scope count per user like (user.measurements.count) and per device (device.measurements.count)
https://guides.rubyonrails.org/association_basics.html#options-for-belongs-to-counter-cache

Also, you'll notice that in the SQL query I described above, I'm filtering by unit = 'cpm' on Rob's advice that measurements in that table that use units other than CPM are calculated from CPM measurements. (This also ignores air quality measurements).

Yep, I included in my query captured_before=2019%2F12%2F01+19%3A56%3A15&commit=Filter&distance=&latitude=&locale=en-US&longitude=&since=&unit=cpm&until=&utf8= and it still works pretty fast. Unless I didn't understand what you are trying to say :)

@norcross
Copy link

norcross commented May 7, 2020

safecast.org is pulling the the number displayed on the home page from https://api.safecast.org/en-US/measurements/count.json

I'm caching that return inside of WP, not anywhere on the actual API. the site will make an API call and store it using the WP transient caching. That value will be stored for an hour, then on the next new page request it'll make a new API call to update itself. there is also a button to manually refresh the number on the admin side. there is also a WP-cron task (which is not a real cron task, it's a long story) that will update once a day if by chance none of the other updates are happening.

as for the site code, yes, i need to merge the repo i used during development into this one.

@sasharevzin
Copy link
Collaborator

@norcross Would you check my reply: #529 (comment)
For some reason, the count on the org side didn't change.

@norcross
Copy link

norcross commented May 8, 2020

ok. i’ve confirmed that when i manually refresh, i get the accurate number (it was off by 7 between when i refreshed in WP and manually loaded the .json page). so the outdated number displayed on the home page is due to some caching inside of WP that i'm going to fine tune.

@norcross
Copy link

norcross commented May 8, 2020

ok, i’ve changed how the WP side is refreshing the data. i’ve included some additional cache cleanup when the count is refreshed (manually or automatically) and i’ve also added a remote refresh request function that i’m hitting with a real cron job every 30 mins (time can be adjusted if need be) so the site should have a relatively accurate count updating itself.

@matschaffer
Copy link
Contributor Author

matschaffer commented May 8, 2020 via email

@sasharevzin
Copy link
Collaborator

@matschaffer The only thing I would improve is .count (https://github.com/Safecast/safecastapi/blob/master/app/controllers/measurements_controller.rb#L106). I really liked what @eitoball proposed in #556
I think it could be a perfect use case

@matschaffer
Copy link
Contributor Author

matschaffer commented May 8, 2020 via email

@norcross
Copy link

norcross commented May 8, 2020

Sweet, if we’re happy with that we can close this issue I think.

as far as my side is concerned, everything is now working as expected. if there's any changes I need to make making the request, we can open a new issue.

@sasharevzin
Copy link
Collaborator

sasharevzin commented May 13, 2020

@matschaffer I just added caching: #678

@matschaffer matschaffer linked a pull request May 21, 2020 that will close this issue
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.

5 participants