Skip to content

Avoid possible error subtable already exists but not loaded #15779

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 6, 2020
Merged

Avoid possible error subtable already exists but not loaded #15779

merged 1 commit into from
Apr 6, 2020

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Apr 6, 2020

Below is part of the stack trace. The change seems to fix the issue from what I can see. I would honestly not know though how this really happens or what the issue is or how to create a test for it. I basically guessed the issue was there after setting a breakpoint when a summary row was created with a subtable, went up the backtrace and noticed this code. Saw the code didn't handle summary rows (which was the case in below warning) so added it. With this change the archive warning is gone.

WARNING: Row with label '-1' (columns = -1, 40, 40, 316, 4.243, 36, 0.01, 1.177) has already a subtable id=129 but it was not loaded - overwriting the existing sub-table. Trace: [{"file":"core\/DataTable\/Row.php","line":291,"function":"warnIfSubtableAlreadyExists","class":"Piwik\\DataTable\\Row","object":{"label":-1,"2":40,"12":40,"13":316,"30":4.2429999999999986,"31":36,"32":"0.01","33":"1.177"},"type":"->","args":[]},{"file":"core\/DataTable.php","line":1916,"function":"sumSubtable","class":"Piwik\\DataTable\\Row","object":{"label":-1,"2":40,"12":40,"13":316,"30":4.2429999999999986,"31":36,"32":"0.01","33":"1.177"},"type":"->","args":[{}]},{"file":"core\/DataTable.php","line":644,"function":"aggregateRowWithLabel","class":"Piwik\\DataTable","object":{},"type":"->","args":[{"label":"-1","2":1,"12":1,"13":0,"30":0.344,"31":1,"32":"0.344","33":"0.344"},{"33":"max","32":"min"}]},{"file":"\/var\/www\/html
\/core\/DataTable\/Row.php","line":298,"function":"addDataTable","class":"Piwik\\DataTable","object":{},"type":"->","args":[{}]},{"file":"core\/DataTable.php","line":1916,"function":"sumSubtable","class":"Piwik\\DataTable\\Row","object":{"label":"example","2":260,"12":595,"13":14985,"30":48.316,"31":548,"32":"0.002","33":"2.852","15":7,"19":2,"20":16,"21":64,"22":1},"type":"->","args":[{}]},{"file":"core\/DataTable.php","line":644,"function":"aggregateRowWithLabel","class":"Piwik\\DataTable","object":{},"type":"->","args":[{"label":"example","2":112,"12":229,"13":3467,"30":15.905,"31":203,"32":"0.006","33":"0.645","15":2,"19":1,"20":1,"21":0,"22":1},{"33":"max","32":"min"}]},{"file":"core\/ArchiveProcessor.php","line":560,"function":"addDataTable","class":"Piwik\\DataTable","object":{},"type":"->","args":[{}]},{"file":"core\/ArchiveProcessor.php","line":541,"function":"aggregatedDataTableMapsAsOne","class"
:"Piwik\\ArchiveProcessor","object":{"archive":{}},"type":"->","args":[{},{}]},{"file":"core\/ArchiveProcessor.php","line":366,"function":"getAggregatedDataTableMap","class":"Piwik\\ArchiveProcessor","object":{"archive":{}},"type":"->","args":[{},{"33":"max","32":"min"}]},{"file":"core\/ArchiveProcessor.php","line":208,"function":"aggregateDataTableRecord","class":"Piwik\\ArchiveProcessor","object":{"archive":{}},"type":"->","args":["Actions_actions_url",{"33":"max","32":"min"},{"1":11,"17":18,"14":16}]},{"file":"plugins\/Actions\/Archiver.php","line":525,"function":"aggregateDataTableRecords","class":"Piwik\\ArchiveProcessor","object":{"archive":{}},"type":"->","args":[["Actions_actions","Actions_actions_url"],500,100,2,{"33":"max","32":"min"},{"1":11,"17":18,"14":16},[]]},{"file":"core\/Plugin\/Archiver.php","line":103,"function":"aggregateMultipleReports","class":"Piwik\\Plugins\\Actions\\Archiver","object":{
"maximumRows":500},"type":"->","args":[]},{"file":"core\/ArchiveProcessor\/PluginsArchiver.php","line":173,"function":"callAggregateMultipleReports","class":"Piwik\\Plugin\\Archiver","object":{"maximumRows":500},"type":"->","args":[]},{"file":"core\/ArchiveProcessor\/Loader.php","line":165,"function":"callAggregateAllPlugins","class":"Piwik\\ArchiveProcessor\\PluginsArchiver","object":{"archiveProcessor":{"archive":{}},"archiveWriter":{"idArchive":355,"idSite":1,"segment":{},"period":{},"doneFlag":"done","dateStart":{}}},"type":"->","args":[1041,0,false]},{"file":"core\/ArchiveProcessor\/Loader.php","line":111,"function":"prepareAllPluginsArchive","class":"Piwik\\ArchiveProcessor\\Loader","object":{"invalidateBeforeArchiving":false},"type":"->","args":[1041,0]},{"file":"core\/ArchiveProcessor\/Loader.php","line":84,"function":"prepareArchiveImpl","class":"Piwik\\ArchiveProcessor\\Loader","object":{"invalidateBef
oreArchiving":false},"type":"->","args":["VisitsSummary"]},{"file":"core\/Context.php","line":75,"function":"Piwik\\ArchiveProcessor\\{closure}","class":"Piwik\\ArchiveProcessor\\Loader","object":{"invalidateBeforeArchiving":false},"type":"->","args":[]},{"file":"core\/ArchiveProcessor\/Loader.php","line":85,"function":"changeIdSite","class":"Piwik\\Context","type":"::","args":[1,{}]},{"file":"core\/Archive.php","line":803,"function":"prepareArchive","class":"Piwik\\ArchiveProcessor\\Loader","object":{"invalidateBeforeArchiving":false},"type":"->","args":["VisitsSummary"]},{"file":"core\/Archive.php","line":607,"function":"prepareArchive","class":"Piwik\\Archive","object":{},"type":"->","args":[["VisitsSummary"],{},{}]},{"file":"core\/Archive.php","line":551,"function":"cacheArchiveIdsAfterLaunching","class":"Piwik\\Archive","object":{},"type":"->","args":[["VisitsSummary"],["VisitsSummary"]]},
{"file":"core\/Archive.php","line":480,"function":"getArchiveIds","class":"Piwik\\Archive","object":{},"type":"->","args":[{"0":"nb_visits","1":"nb_actions","2":"max_actions","3":"bounce_count","7":"sum_visit_length"}]},{"file":"core\/Archive.php","line":295,"function":"get","class":"Piwik\\Archive","object":{},"type":"->","args":[{"0":"nb_visits","1":"nb_actions","2":"max_actions","3":"bounce_count","7":"sum_visit_length"},"numeric"]},{"file":"plugins\/VisitsSummary\/API.php","line":36,"function":"getDataTableFromNumeric","class":"Piwik\\Archive","object":{},"type":"->","args":[{"0":"nb_visits","1":"nb_actions","2":"max_actions","3":"bounce_count","7":"sum_visit_length"}]},{"function":"get","class":"Piwik\\Plugins\\VisitsSummary\\API","object":{},"type":"->","args":["1","month","last2",false,{"0":"nb_visits","1":"nb_actions","2":"max_actions","3":"bounce_count","7":"sum_visit_length"}]},

@tsteur tsteur added the Needs Review PRs that need a code review label Apr 6, 2020
@tsteur tsteur added this to the 3.13.5 milestone Apr 6, 2020
@diosmosis
Copy link
Member

DataTableFactory doesn't query the db directly, so a test could be added to DataTableFactoryTest. Let me know if you don't have time I could add one.

@tsteur
Copy link
Member Author

tsteur commented Apr 6, 2020

@diosmosis I was more meaning it's hard to write a test for this particular case which happened only in some random case (first time we had this in many years) and caused a warning somewhere else etc. I reckon a test might not be actually needed.

@tsteur tsteur merged commit 7af49ee into 3.x-dev Apr 6, 2020
@tsteur tsteur deleted the archivesumrowwarning branch April 6, 2020 20:43
diosmosis added a commit that referenced this pull request Apr 16, 2020
@diosmosis @tsteur
@mattab @sgiehl
merge 3.x to 4.x (#15821)
37182bf 
* Avoid possible error subtable already exists but not loaded (#15779)

* Make sure to always set JSON_PIWIK to native JSON when possible (#15785)

* make sure to always set JSON_PIWIK to native JSON when possible

* rebuilt piwik.js

* Force POST for bulk requests, fix alwaysUseSendBeacon not respected for bulk requests (#15784)

* Force POST for bulk requests, fix alwaysUseSendBeacon not respected for bulk requests

* rebuilt piwik.js

* Make sure to clean up tracking failures before sending email notification (#15798)

Feedback from a customer... Eg the daily `cleanupTrackingFailures()` action might be only executed after the weekly `notifyTrackingFailures` therefore we should try to clean up failures first and then check if any are left. Avoids the case where a user opens hours later the email they receive and then there are no tracking failures reported. This could still happen but it's a bit less likely.

* 3.13.5-b1

* Faster segmented suggested values when browser archiving is disabled (#15786)

* Faster segmented suggested values when browser archiving is disabled

* make sure no segment is set

* remove wrong var type

* fix/add tests

* add more segment values

* detect if we should flatten or not

* add docs

* Fix problem when comparing segments or opening popovers (#15809)

refs #15805

* purge all old archives regardless of done value (#15800)

* purge all old archives regardless of done value, we only care about the newest usable one

* Fix test and start on new one.

* Add coverage for change in tests.

* there is no longer an inner join so should not need the idsite check

* Add more parameters to the computeNbUnique event (#15808)

* 3.13.5-b2

* One click update in two parts so new code is loaded for second. (#15770)

* One click update in two parts so new code is loaded for second.

* remove no longer needed code

Co-authored-by: Thomas Steur <tsteur@users.noreply.github.com>
Co-authored-by: Matthieu Aubry <mattab@users.noreply.github.com>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
jonasgrilleres pushed a commit to 1024pix/pix-analytics that referenced this pull request Sep 22, 2020
@tsteur @jonasgrilleres
Avoid possible error subtable already exists but not loaded (matomo-o…
dfb22b9 
…rg#15779)
jbuget pushed a commit to 1024pix/pix-analytics that referenced this pull request Sep 26, 2020
@tsteur @jbuget
Avoid possible error subtable already exists but not loaded (matomo-o…
449409d 
…rg#15779)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diosmosis diosmosis diosmosis approved these changes

Assignees
No one assigned
Labels
Needs Review PRs that need a code review
Milestone
3.13.5
Development

Successfully merging this pull request may close these issues.
2 participants
@tsteur @diosmosis