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

Faster Real Time Maps #12752

Merged
merged 3 commits into from Apr 23, 2018
Merged

Faster Real Time Maps #12752

merged 3 commits into from Apr 23, 2018

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Apr 21, 2018

Improves fetching visits for realtime map as mentioned in #12722 (comment)

refs #12722

@sgiehl sgiehl added the Needs Review PRs that need a code review label Apr 21, 2018
@sgiehl sgiehl added this to the 3.5.0 milestone Apr 21, 2018
maxVisits = config.maxVisits || 100,
changeVisitAlpha = typeof config.changeVisitAlpha === 'undefined' ? true : config.changeVisitAlpha,
removeOldVisits = typeof config.removeOldVisits === 'undefined' ? true : config.removeOldVisits,
doNotRefreshVisits = typeof config.doNotRefreshVisits === 'undefined' ? false : config.doNotRefreshVisits,
enableAnimation = typeof config.enableAnimation === 'undefined' ? true : config.enableAnimation,
forceNowValue = typeof config.forceNowValue === 'undefined' ? false : +config.forceNowValue,
width = main.width(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this variable removed on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't see any usage of that variable, or did I miss something?

@tsteur
Copy link
Member

tsteur commented Apr 22, 2018

In my case it resulted in this query:

SELECT sub.* FROM ( SELECT log_visit.* FROM piwik_log_visit AS log_visit WHERE log_visit.idsite in (?) AND log_visit.visit_last_action_time >= ? ORDER BY idsite DESC, visit_last_action_time DESC LIMIT 0, 100 ) AS sub GROUP BY sub.idvisit ORDER BY sub.visit_last_action_time DESC LIMIT 100

with bind

array ( 0 => '1', 1 => '2018-03-25 02:23:01', 

This looks good but wondering if we should reduce the last 30 days it looks at by default, change it to say last3 which should be still good enough for a real time map considering we show like only visits from the last 24 hours or so.

@sgiehl
Copy link
Member Author

sgiehl commented Apr 22, 2018

That should depend on the currently chosen date any period. If you are looking at today, it should only query for todays data. But as it's a realtime map guess we could change that to lower ranges in another PR.
We could also improve the way the query is built. If a minTimestamp is given the query will still contain two checks for time, one with the current period/date and one for the given minTimestamp...

@tsteur
Copy link
Member

tsteur commented Apr 22, 2018

The date selector was disabled in real time map when I tested it and the last30 comes from the controller.

@mattab mattab changed the title Improve RealTime Map JS Faster Real Time Maps Apr 23, 2018
@mattab mattab merged commit 2cdedac into 3.x-dev Apr 23, 2018
@mattab mattab deleted the realtimefix branch April 23, 2018 04:00
InfinityVoid pushed a commit to InfinityVoid/matomo that referenced this pull request Oct 11, 2018
* do not use min timestamp for realtime map queries if not needed

* remove some unused code

* Plot only the last 7 days rather than last 30 days
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants