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

Fixed #6398 - probably fixed/worked-around by sorting #7348

Conversation

medic123de
Copy link
Contributor

please review.
small-scale test has finished without error.

Deadlock occurs because
Thread A holds lock on row 1,5,8,9 and wants 20.
Thread B holds lock on 2,3,15,20,25 and wants 9.

this happens because the Hit Objects in Recorders are not ordered.
So Thread B can get a lock on row 20, and want to "warp back" to 9,
while Thread A holds row 9 and wants to advance to row 20.

Solution:
Ordering ensures Thread B waits for lock release on row 9, without having row 20 already locked.

@medic123de medic123de closed this Mar 3, 2015
sort by IP + Date
spread PHP load  more evenly by redefining "max_payload_size"
@medic123de medic123de reopened this Mar 4, 2015
@medic123de
Copy link
Contributor Author

please add to FAQ:
"Never have 2 seperate processes or servers import logs for the same id_site at the same time. This will cause InnoDB Transaction Deadlocks"

@medic123de medic123de changed the title Issue #6398 - probably fixed by sorting Fixed #6398 - probably fixed/worked-around by sorting Mar 4, 2015
@mattab mattab added Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Needs Review PRs that need a code review labels Mar 10, 2015
@mattab mattab added this to the Piwik 2.12.0 milestone Mar 10, 2015
@mattab
Copy link
Member

mattab commented Mar 10, 2015

Hi @medic123de I added the tag Needs review and hopefully we can review it for 2.12.0

@mattab
Copy link
Member

mattab commented Mar 12, 2015

Hi @medic123de do you maybe have an explanation for the System tests failures after your change? https://travis-ci.org/piwik/piwik/jobs/53025832

I'm moving to 2.13.0 milestone as we likely won't have time to merge for 2.12.0

@mattab mattab modified the milestones: Piwik 2.13.0, Piwik 2.12.0 Mar 12, 2015
@medic123de
Copy link
Contributor Author

Hi Matt,
we managed to get errors again even with this approach.. it seems to improve the situation but not entirely fix it.

Situation is: the tests fail, because numbers are not as expected. If i get the code in Tracker/Visit.php right, isLastActionInTheSameVisit should be true for last $visit_standard_length

So, there are only 3 criteria that are sufficient to match a visitor. The browser, the date and the IP.
I have to dig into the code wide deeper than expected to find why sorting does not entirely fix the situation, and why sorting leads to different results.
That should not happen as I understand from Tracker/Visitor.php

I will close the pull request by the current situation ( as it does nox fix this issue ), and will go another way to 1st fix the potential harm ( by assuring no data is lost ), then investigate what really happens.

@medic123de medic123de closed this Mar 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants