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

Goal Overview Report & widget on dashboard can cause a very large spike in requests #17774

Closed
Starker3 opened this issue Jul 15, 2021 · 3 comments · Fixed by #17937
Closed

Goal Overview Report & widget on dashboard can cause a very large spike in requests #17774

Starker3 opened this issue Jul 15, 2021 · 3 comments · Fixed by #17937
Labels
c: Performance For when we could improve the performance / speed of Matomo. duplicate For issues that already existed in our issue tracker and were reported previously. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.
Milestone

Comments

@Starker3
Copy link
Contributor

Starker3 commented Jul 15, 2021

Current Behavior

Currently when adding the "Goal Overview" widget to the dashboard or loading the Goal Overview report it will make at least 2 GET requests for each goal present in the Overview (At least one request for each sparkline per goal)
A Matomo server with just 7 goals for example makes around 25 requests just for sparklines on the Goals Overview.

On websites that have a large number of goals (For example >50) this can cause a spike in the number of PHP processes that can result in the maximum number of PHP child processes to be created, this results in the following warning:
"WARNING: [pool www] server reached pm.max_children setting (50), consider raising it"

This in turn results in unexpected performance issues that may be difficult to resolve (Other than simply removing the Goal Overview widget from the dashboard, but this doesn't solve the issue when loading the Goal Overview report)

Possible Solution

Not sure if there is a quick fix for this sort of thing, other than somehow batching the requests for the sparklines so that it doesn't cause the child processes to be maxed?

Context

For users running servers with a single endpoint (Tracking and UI on the same server) this can potentially result in the entire Matomo server being slow to respond to requests (Since PHP can't handle new requests coming in this could potentially lead to slow tracker performance or other issues)

@Starker3 Starker3 added the Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. label Jul 15, 2021
@diosmosis diosmosis added the c: Performance For when we could improve the performance / speed of Matomo. label Jul 18, 2021
@tsteur tsteur removed the Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. label Aug 30, 2021
tsteur added a commit that referenced this issue Aug 30, 2021
fix #17774

Test locally and worked nicely. Had 11 goals and on my screen it loaded the first 9 goals right away and the other ones while scrolling. This way, if someone has say 25 goals, then we won't issue like 80 requests at the same time on goals overview or funnels overview page which can cause issues in the web server as well as database
@tsteur tsteur self-assigned this Aug 30, 2021
@tsteur tsteur added this to the 4.5.0 milestone Aug 30, 2021
justinvelluppillai pushed a commit that referenced this issue Aug 30, 2021
fix #17774

Test locally and worked nicely. Had 11 goals and on my screen it loaded the first 9 goals right away and the other ones while scrolling. This way, if someone has say 25 goals, then we won't issue like 80 requests at the same time on goals overview or funnels overview page which can cause issues in the web server as well as database
@tsteur
Copy link
Member

tsteur commented Sep 1, 2021

I'll reopen this one as it's not fully fixed.

It's still sending a request for every goal on the goals overview page like https://demo.matomo.cloud/index.php?forceView=1&viewDataTable=sparklines&module=Goals&action=get&idGoal=6&allow_multiple=0&only_summary=1&idSite=1&period=day&date=yesterday&segment=&showtitle=1&random=7404

to fetch this data
image

In the referenced PR only the sparkline image itself is loaded when in viewport.

Ideally we load these widgets async only when in viewport as well or maybe better we create one widget that loads the data for all sparklines. Meaning have only one widget that loads all the individual sparklines (without changing the look)
image.

This way it be only one request instead of one request per goal (which could all launch archiving etc)

@tsteur tsteur reopened this Sep 1, 2021
@tsteur tsteur removed their assignment Sep 1, 2021
@tsteur tsteur modified the milestones: 4.5.0, 4.7.0 Sep 1, 2021
@tsteur
Copy link
Member

tsteur commented Sep 1, 2021

Something like this could fix this issue. Not tested it at all though. This may not work. Just an idea.

diff --git a/plugins/CoreHome/Controller.php b/plugins/CoreHome/Controller.php
index 1254260eaa..cfeb426f8f 100644
--- a/plugins/CoreHome/Controller.php
+++ b/plugins/CoreHome/Controller.php
@@ -70,6 +70,13 @@ class Controller extends \Piwik\Plugin\Controller
         Piwik::checkUserHasSomeViewAccess();
         $this->checkSitePermission();
 
+        // RENDER ALL THE WIDGETS HERE at once instead of requesting the rendering of these widgets using JS
+        $out = '';
+        foreach (widgets as $widget) {
+            $out .= FrontController::getInstance()->dispatch('CoreHome', 'renderWidget');;.
+        }
+        return $out;
+
         $view = new View('@CoreHome/widgetContainer');
         $view->isWidgetized = (bool) Common::getRequestVar('widget', 0, 'int');
         $view->containerId  = Common::getRequestVar('containerId', null, 'string');
diff --git a/plugins/Goals/Pages.php b/plugins/Goals/Pages.php
index 6b105bfb9a..c677d317ed 100644
--- a/plugins/Goals/Pages.php
+++ b/plugins/Goals/Pages.php
@@ -57,6 +57,13 @@ class Pages
         $config->setIsNotWidgetizable();
         $widgets[] = $config;
 
+        $container = $this->factory->createContainerWidget('goals_overview_goals');
+        $container->setSubcategoryId($subcategory);
+        $container->setName('');
+        $container->setOrder(15);
+        $container->setIsNotWidgetizable();
+        $widgets[] = $container;
+
         foreach ($goals as $goal) {
             $name = Common::sanitizeInputValue($goal['name']);
             $goalTranslated = Piwik::translate('Goals_GoalX', array($name));
@@ -69,7 +76,8 @@ class Pages
             $config->setOrder(25);
             $config->setIsNotWidgetizable();
             $config->addParameters(array('allow_multiple' => (int) $goal['allow_multiple'], 'only_summary' => '1'));
-            $widgets[] = $config;
+            $container->addWidgetConfig($config);
+            //$widgets[] = $config;
         }
 
         $container = $this->createWidgetizableWidgetContainer('GoalsOverview', $subcategory, $widgets);

We would then render all the widgets within a container server side at once. In the past we loaded them individually thinking it's better to load the widgets in parallel but for some pages like goals this might not be the best way (or in general when widgets within a container use the same report data)

@tsteur tsteur added the Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. label Sep 5, 2021
@tsteur tsteur modified the milestones: 4.7.0, 4.6.0 Sep 30, 2021
@tsteur tsteur added Regression Indicates a feature used to work in a certain way but it no longer does even though it should. and removed Regression Indicates a feature used to work in a certain way but it no longer does even though it should. labels Sep 30, 2021
@tsteur tsteur added duplicate For issues that already existed in our issue tracker and were reported previously. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it. labels Oct 7, 2021
@tsteur
Copy link
Member

tsteur commented Oct 7, 2021

This should be fixed by #18088

@tsteur tsteur closed this as completed Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Performance For when we could improve the performance / speed of Matomo. duplicate For issues that already existed in our issue tracker and were reported previously. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants