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

Speed up fetching range reports, evolution and sparkline reports #20121

Merged
merged 7 commits into from
Dec 22, 2022

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Dec 18, 2022

Description:

This index change will speed up all kind of reports fetching but especially range periods where the change will be most noticeable.

When fetching evolution graphs, then the method getArchiveIds can be very slow. Especially when looking for many periods.

See eg below where it executed 30 queries and it took 7 seconds:

image

Or this range period requests where it took 217 seconds for 2240 calls (there might be still more tweaks possible to reduce the number of calls which be a separate fix).

image

I've been testing this on two of our instances with different queries and profiling and we can reproduce this.

I've then looked at the current numeric index which is:

`idsite`,`date1`,`date2`,`period`,`ts_archived`

The ts_archived is not very useful for an index though because most of the queries wouldn't lower the result set. By changing this part of the index to name(6). I was able to make the queries 150-170 times faster on our instances (we ran the same queries 100 times with SQL_NO_CACHE and compared different queries with different indexes). That's roughly because our archives include 166 different record names. On some smaller instance it improved the query 40+.

On our account 1 such query takes typically 200ms vs with this index change it takes 1.2ms.

That means above screenshot goes roughly down from 7 seconds to say 50ms. The 217 seconds goes down to 1.5 seconds.

While not changing the index size in the end as I've looked over the queries containing ts_archived and the ts_archived index typically doesn't help much there but even more importantly these queries contain also the name column meaning the new index should be even more effective there.

This query getArchiveIds() is also the TOP contributor on our cloud for load.

ts_archived was added initially 12 years ago in f593a3e but a better choice would have been to use name column but that time there were likely certain MySQL limitations.

Why name(6)?
The done flag we use in those archive queries are typically done, done.VisitsSummary or done2499999 or done249999.VisitsSummary. By including the first 6 characters in the index we should be able to select more precise the best rows. Technically, name(5) would do an equally good job as out of 1M records there are only 6 done.* records but figured rather give it one more.

Other changes

  • I've also removed where query parts ts_archived is not null as it is always not null. I assume it was added for old MySQL versions as it otherwise wouldn't use the index unless all parts were covered.

We have tested these changes on production on our account and reports are now loading pretty much instant.

Review

@tsteur tsteur added the Needs Review PRs that need a code review label Dec 18, 2022
@tsteur tsteur added this to the 5.0.0 milestone Dec 18, 2022
@tsteur tsteur changed the title Speed up fetching range reports Speed up fetching range reports, evolution and sparkline reports Dec 18, 2022
@tsteur tsteur removed the Needs Review PRs that need a code review label Dec 18, 2022
@tsteur tsteur added the Needs Review PRs that need a code review label Dec 19, 2022
@sgiehl
Copy link
Member

sgiehl commented Dec 19, 2022

@tsteur Generally those changes are looking fine. But I was wondering if adding / changing indexes isn't a database operation we wanted to avoid for Matomo 5. We postponed issues like #17466 to Matomo 6, as they would add new dimensions. Just wondering how intense changing indexes is on bigger databases...

@tsteur
Copy link
Member Author

tsteur commented Dec 19, 2022

@sgiehl Only log table migrations would be an issue. All others are fine as they run through fairly quickly and won't block tracking. The complexity comes more from log table migrations as when you have a bit of data in a log table then users/admins/hosters will need to go through a specific process of enabling queued tracking while the migrations are being performed and then replaying that traffic after the migration finished a few minutes, hours, days later and ensure all the requests are being reprocessed etc (or alternatively go into maintenance mode and replaying the logs via the importer which is also not the easiest thing to do for people).

core/Updates/5.0.0-b1.php Outdated Show resolved Hide resolved
core/DataAccess/ArchiveSelector.php Show resolved Hide resolved
@sgiehl sgiehl removed the Needs Review PRs that need a code review label Dec 20, 2022
@sgiehl sgiehl merged commit 000cf71 into 5.x-dev Dec 22, 2022
@sgiehl sgiehl deleted the speedupreports branch December 22, 2022 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants