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

Jump to bottom

Create re-usable interprocess locking utility #12712

Open
diosmosis opened this issue Apr 11, 2018 · 4 comments
Open

Create re-usable interprocess locking utility #12712

diosmosis opened this issue Apr 11, 2018 · 4 comments
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself.
Milestone
For Prioritization

Comments

@diosmosis
Copy link
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.
@diosmosis diosmosis added the c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. label Apr 11, 2018
@tsteur
Copy link
Member

tsteur commented Apr 11, 2018

@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
Copy link
Member Author

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
Copy link
Member

tsteur commented Apr 12, 2018

Yes I think that one should work across servers.

@tsteur
Copy link
Member

tsteur commented Apr 16, 2018

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself.
Projects
None yet
Milestone
For Prioritization
Development

No branches or pull requests
3 participants
@diosmosis @tsteur @innocraft-automation