@kaplun opened this Pull Request on May 24th 2018

(Closes #6398)

@kaplun commented on May 24th 2018

@kachenjr as you pointed out in #6398, the invalidation of reports happens continuously per every single event. With this PR I have introduce cumulations of all the invalidated reports in a single array that is then checked at the very end of the requests, and only once per report.

What do you think? I am no PHP expert, so feel free to report back on any technical issue you might find.

@diosmosis commented on June 1st 2018 Member

I'm going to try and reproduce this locally.

EDIT: Unfortunately, not able to reproduce, appears to work fine for me. The table in the error message you posted in the issue mentions log_visit, maybe the reason for this deadlock has to do w/ that table?

EDIT 2: Perhaps https://github.com/matomo-org/matomo/blob/3.x-dev/core/Db.php#L721 or something similar could be used in the Tracker Db class to get more information.

@kaplun commented on June 1st 2018

Yeah unfortunately as you have identified, putting the closing code in the __destructor doesn't seem to be effective (either the code is not really called, or it fails silently, because the net result is that with this path the option table is no longer updated).

I am not sure at this point if postponing and collection all updates at the end is enough to avoid the deadlock. For sure (assuming the code is really called properly, and not like in the current version of this patch), it should reduce the number of writing to the DB (currently the invalidation information is written down per every single event being treated, which would mean e.g. 500 times in a bulk request, instead of just once at the end).

@diosmosis commented on June 1st 2018 Member

What I mean about being confused about this PR is that the original PR fixed this specific case:

  1. tracker request A starts transaction & tracks action to site 1 / date 2018-05-11
  2. tracker request A updates option w/ key report_to_invalidate_1_2018-05-11
    • mysql now locks that specific option row & does not let go
  3. tracker request B starts & also tracks action to to site 1 / date 2018-05-11
  4. tracker request B tries to update to option w/ key reports_to_invalidate_1_2018-05-11, but since request A has a lock and is in a transaction, request B just sits waiting for the lock
  5. tracker request A takes a long time to finish & before it does, tracker request B gives up on getting the lock, resulting in a deadlock

The other PR fixed this by appending the PID, meaning request A & B above would not try to update the same option row, and thus no one would wait for a lock.

This PR moves the writing to one place, but that shouldn't have an effect on the concurrency at play here. Which doesn't mean it won't or doesn't fix the issue for you, it would just be good to find & verify the reason this should fix the deadlock before going ahead and just merging. (And note: thinking about this issue has made me very interested in this, so if you investigate further, please do share :). I can't do much since I can't reproduce it...).

@tsteur commented on April 11th 2019 Member

@kaplun are you still working on this? Be good to have a look at @diosmosis comment maybe

@kaplun commented on April 12th 2019

No unfortunately I am no longer having the resources to work on it. I will try to check if we still have deadlock, and see.

@tsteur commented on July 4th 2019 Member

@mattab I reckon we can close the PR while we can't reproduce things or while we don't know if there is still an issue? I reckon as @diosmosis suggested https://github.com/matomo-org/matomo/pull/12733 might have fixed the issue and this PR doesn't change much in the end re possible deadlock.

This Pull Request was closed on July 4th 2019
Powered by GitHub Issue Mirror