Improve performance of delete query and make sure it doesn't lock existing tracking requests #13894
Conversation
…ting tracking requests
tsteur
added
not-in-changelog
| $idSitesLogConversion = $this->getDistinctIdSitesInTable('log_conversion', $maxIdSite); | ||
| $idSitesUsed = array_unique(array_merge($idSitesLogVisit, $idSitesLogVisitAction, $idSitesLogConversion)); | ||
|
|
||
| $idSitesNoLongerExisting = array_diff($idSitesUsed, $allExistingIdSites); |
There was a problem hiding this comment.
Possible edge case that I haven't thought through too much, but what if a site hasn't tracked any visits yet, and this runs. It will be treated as a deleted site, correct? If tracking is installed while this process is under way, will it be a problem?
There was a problem hiding this comment.
It shouldn't cause it would only return sites here that were tracked into and not exist in sites?
There was a problem hiding this comment.
If sites has ids, [1,2,3,4,5] and sites that are tracked has ids [1,2,3], then the array_diff for $idSitesNoLongerExisting would be [4,5], correct?
There was a problem hiding this comment.
Not sure if I'm seeing it wrong. idSitesUsed contains the list of all tracked siteIds... then the second part is the number of sites that actually exist (it should be the other way around from your example?) and then it would correctly return the deleted sites that were tracked into in the diff?
There was a problem hiding this comment.
Let me try and explain again (I might be seeing it wrong too). There are 5 sites total:
[1,2,3,4,5]
No site is deleted. Only three of the sites have tracked data, [1,2,3]. So when looking through log tables for idsites, we only get [1,2,3].
In this situation, we will try to delete log data for [4,5], which would be ok since there isn't any log data, except possibly if tracking begins during the deletion process. Again, I'm not sure if this is a problem ultimately, just mentioning it.
There was a problem hiding this comment.
@diosmosis it would return [] not [4,5] see eg https://3v4l.org/Q4uci
array_diff returns all entries from the first array, that are not in the second array.
fix #13800
I'm thinking this should improve the performance of the delete queries:
where idsite in(...)should only lock rows belonging to that site. New tracking requests for those rows / idsite shouldn't come in cause the site no longer exists (would be discarded by tracker) so it shouldn't be a problem.Initially I also implemented a check for each table that had
idsitecolumn to check if tracking requests for that site exists and only delete if the log table has tracking requests for that site, but it shouldn't be needed.