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

Non-optimal app layer iteration in MetricRepository #1900

Closed
3 tasks done
deanstalker opened this issue Jun 8, 2016 · 7 comments · Fixed by #2040
Closed
3 tasks done

Non-optimal app layer iteration in MetricRepository #1900

deanstalker opened this issue Jun 8, 2016 · 7 comments · Fixed by #2040

Comments

@deanstalker
Copy link

Before submitting your issue, please make sure that you've checked all of the checkboxes below.

  • You're running the latest release version of Cachet.
  • Ensure that you're running at least PHP 5.5.9, you can check this by running php -v
  • You've ran rm -rf bootstrap/cache/* from the root of your Cachet installation.

To help us better understand your issue, please answer the following — cheers!

Your setup

  • What version of Cachet? v2.3.0-RC5
  • What database driver? MySQL? 5.5.47
  • What version of PHP? 5.6

Expected behaviour

Iterating over the metric points should be quite performant.

Actual behaviour

I've noticed in the MetricRepository you are iterating over each minute/hour in the app layer - which will potentially generate between 12 - 60 queries of an average return time of 4-8 seconds each - depending on the data size.

Subsequently, we're seeing our MySql process jump to 100% CPU time once the Metric Points table gets close to 40,000 records

Wouldn't it be more performant to iterate this in the query itself? ie.MySql example

SELECT AVG(mp.value * mp.counter) AS value FROM metrics m INNER JOIN metric_points mp ON m.id = mp.metric_id WHERE m.id = 1 AND DATE_SUB(curdate(), INTERVAL 12 HOUR) <= mp.created_at GROUP BY HOUR(mp.created_at)

@GrahamCampbell
Copy link
Contributor

We're open to PRs. We have 200,000 entries on https://status.styleci.io/, and it is not exactly fast. :P

@jbrooksuk
Copy link
Member

Counting this as a bug. We'd love a PR for 2.3 :)

@deanstalker
Copy link
Author

Happy to oblige

@jbrooksuk
Copy link
Member

Thanks! Please let us know if we can help you.

@jbrooksuk jbrooksuk added this to the V2.3.0 milestone Jun 9, 2016
@jbrooksuk jbrooksuk modified the milestones: V2.4.0, V2.3.0 Jun 20, 2016
@jbrooksuk
Copy link
Member

@deanstalker did you get anywhere with this?

@deanstalker
Copy link
Author

I was sanctioned to work on a PR in company time a few weeks ago, however paid work means it was on the back burner for a few days. I managed to get the MySQL and Sqlite repository classes updated, and have just been waiting to get the Postgres one updated ...

Meanwhile I have seen #1971, and its along similar lines

@jbrooksuk
Copy link
Member

Awesome!

Well, if you can, I'd love to see the work you've done and look at merging both together.

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.

3 participants