@diosmosis opened this Issue on April 11th 2018 Member

Dealing with concurrency occurs a lot when working on Matomo, but there are no re-usable, proven implementations. We should add a locking mechanism that satisfies the following conditions:

  • The lock mechanism should be completely thread safe, w/ no race conditions and w/o the potential for multiple processes to think they have a lock at the same time.
  • The locks should be saved in a new MySQL table, preferably a memory table. If a memory table is not available, then an InnoDb table will work (or whatever is configured in the INI config).
  • The current process' PID must be associated with the lock so we know who got a lock. Each process should not attempt to change an existing lock that belongs to another process, unless that process no longer exists.
    • Using a PID lets us solve the case where a process acquires a lock and dies w/o unlocking.
  • There should be a register_shutdown_function() function that, for safety, unlocks all locks held by the current process.
@tsteur commented on April 11th 2018 Member

@diosmosis just looking at https://github.com/matomo-org/matomo/blob/3.4.0/core/CronArchive/SharedSiteIds.php#L127 I notice the runExclusive is actually not that exclusive because it is only exclusive per server but when different servers try to execute, this would be still allowed even though they shouldn't.

For this we would as well need a lock in the DB eg a Memory table. However, by pid wouldn't work here (like we did in the other plugin) as it may be executed from various servers and the pid wouldn't be existing on another server. It is nothing of big importance right now but needs to be looked at when doing this change.

@diosmosis commented on April 12th 2018 Member

That's why I called it an 'InterprocessLock', won't work between servers. I think the queued tracking locking mechanism you wrote for the mysql backend would work for this use case? Could put that into a 'InterServerLock' class or something w/ a better name.

@tsteur commented on April 12th 2018 Member

Yes I think that one should work across servers.

@tsteur commented on April 16th 2018 Member

If someone needs this again please ping me and I can maybe provide some code for a LocalLock (lock per server) and a GlobalLock (lock across servers)

Powered by GitHub Issue Mirror