@tsteur opened this Pull Request on August 18th 2019 Member

@mattab from what I've been reading this should avoid the issue re a long running create temporary table select ... possibly locking some tracking requests.

  • Converted it in general into two statements: create temporary table and insert into ... to avoid a metadata lock
  • Using read-uncommitted to prevent locks on the source and target DB. Target DB doesn't matter anyway since we know nobody can read from it until it's been created as it's a temporary table
  • Note: Doing the read-uncommitted only when innodb is configured in the config as we can't really know if innodb is available and it wouldn't make sense with myisam.
  • In general Myisam might be pretty bad as it locks the entire table... I wonder if we should disable this new temp table behaviour if someone is still using myisam on log_visit (and assume it is the same for other log tables)? Could also add a system check if not already done if myisam is still used?

This issue also fixes: https://github.com/matomo-org/matomo/issues/14818 making archiving of segments again faster

@tsteur commented on August 19th 2019 Member

BTW the only other solution I see to make this non-locking is selecting all results in PHP, then doing a batchInsert. Of course this can have memory and performance implications. Could try doing it for MyISAM users though

@tsteur commented on August 21st 2019 Member

Added a workaround for MyIsam users to actually fetch the idVisits into PHP and insert them all at once to not have it locking for them. This way we don't need to disable it for them as this behaviour should be used eventually for all users

@tsteur commented on August 27th 2019 Member

FYI: Was reading this morning a new blog the read-uncommitted status might be even fine for our regular archiving queries but this would be a different issue https://federico-razzoli.com/read-only-transactions-in-mysql

@mattab commented on August 30th 2019 Member

@sgiehl can you please review this PR and check it all makes sense?

@mattab commented on September 10th 2019 Member

ping @sgiehl

@tsteur we can otherwise merge and test in the next beta this week?

@tsteur commented on September 10th 2019 Member

sounds good @mattab most of the code we're actually running on production for a while

This Pull Request was closed on September 10th 2019
Powered by GitHub Issue Mirror