Do not always extend visit for content impressions #15860
Conversation
tsteur
added
the
c: Performance
|
Sounds great, change makes sense. If users want accurate visit time, heartbeat timer would be a better solution. It would be fine to also remove the 1 minute logic and never update the visit. Be good to mention in developer changelog too. |
| // it is only numeric when directly being called afterRequestProcessed() and not eg handleExistingVisit | ||
| // because the VisitLastActionTime dimension will overwrite the original value of the visitor. | ||
| // we want to make sure to work on the value from the DB | ||
| $lastActionTimeDate = Date::factory($lastActionTime)->addPeriod(1, 'minutes'); |
There was a problem hiding this comment.
Should this be addPeriod(5 or is the comment below inaccurate?
| { | ||
| if ($request->getMetadata('Actions', 'doNotExtendVisit')) { | ||
| unset($valuesToUpdate['visit_last_action_time']); | ||
| unset($valuesToUpdate['visit_total_time']); |
There was a problem hiding this comment.
Just a suggestion, but doNotExtendVisit could be handled in Visit.php so any plugin could add behavior like this.
|
fyi it seems I can't make this work. It seems actually needed to update You can see here some content impressions and their
This means the data in the exported visitor log shows a wrong
As a result, it also changes eg
Technically, I noticed the page view report should maybe not include So I basically couldn't figure out a way to make this work unfortunately. FYI @mattab |
i'm not sure i fully understand yet, but say if we "forced" content impressions'
it would be fine I think to have EDIT: there is also this issue but it's not wanted for now due to performance impacts #9748 |
|
Yeah we'd need to implement #9748 to make this work which in the end would cause more issues. If we forced content impressions to have 0 values for |
@mattab the idea here is that content impressions is not quite an interactive action and as such it should not extend the visit or maybe only less often (if user scrolls after a while on the page we could consider it maybe interactive). The reason is mostly performance related to have less concurrent log_visit updates and therefore less potential locks. Often people have say 10-30 content blocks and either all of them are sent at the same time (eg if trackAllContentBlocks) or many of them are sent in parallel (when trackVisibleBlocksOnly and many become visible while scrolling). In Matomo 4 we can start queueing this requests so they are not sent all at the same time and instead as bulk (maybe they are already not sure) but in general we want to avoid too many concurrent requests here.
The idea is so far we only extend visit if the last action was more than a minute ago. Could also completely remove it and never extend the visit (only content interactions would extend it). This can avoid many log_visit locks.
Haven't fully tested it yet, but wanted to get feedback on it as I reckon it can help.
For content impressions it will basically no longer update anything on log_visit which be good.