Moodle

PHP locking class

Details

  • Type: New Feature New Feature
  • Status: Open Open
  • Priority: Critical Critical
  • Resolution: Unresolved
  • Affects Version/s: 2.0
  • Fix Version/s: DEV backlog
  • Component/s: Other
  • Labels:
  • Database:
    Any
  • Affected Branches:
    MOODLE_20_STABLE

Description

This locking class depends only on the database's UPDATE operation being atomic. See the code for some more comments and a test mechanism.

Attachments

  1. File
    namedlock.php
    19/Jan/10 5:24 AM
    23 kB
    Matt Oquist
  2. File
    namedlock.php
    19/Jan/10 5:16 AM
    23 kB
    Matt Oquist

Issue Links

This issue will be resolved by:
MDL-25500 Implement the locking code, either Matt's (in MDL-21110) or something similar and write robust tests for it Major Open

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Matt Oquist added a comment -

Minor bugfix; exceptions thrown in constructor referenced $this->name before it was set.

Show
Matt Oquist added a comment - Minor bugfix; exceptions thrown in constructor referenced $this->name before it was set.
Hide
Permalink
Matt Oquist added a comment -

Another bugfix. I switched from checking a return code to checking $this->locked, but failed to edit one of the tests. Now I have rectified that failure.

Show
Matt Oquist added a comment - Another bugfix. I switched from checking a return code to checking $this->locked, but failed to edit one of the tests. Now I have rectified that failure.
Hide
Permalink
Penny Leach added a comment -

added some watchers

Show
Penny Leach added a comment - added some watchers
Hide
Permalink
Sam Marshall added a comment -

This code is cool (I am not sure it works but assuming it does!) but could it perhaps be better implemented as:

abstract base class: named_lock
subclass: db_named_lock
factory method named_lock::construct() which initially just returns a new db_named_lock, but could later use $CFG etc

reason for this is that since we are talking about transient data here, filesystem locking, or other locking strategies, might be much faster/lighter-weight so it would be nice to allow an easy place to implement these later if needed.

just my thoughts, I don't know what you are planning to use it for so maybe it would never apply.

Show
Sam Marshall added a comment - This code is cool (I am not sure it works but assuming it does!) but could it perhaps be better implemented as: abstract base class: named_lock subclass: db_named_lock factory method named_lock::construct() which initially just returns a new db_named_lock, but could later use $CFG etc reason for this is that since we are talking about transient data here, filesystem locking, or other locking strategies, might be much faster/lighter-weight so it would be nice to allow an easy place to implement these later if needed. just my thoughts, I don't know what you are planning to use it for so maybe it would never apply.
Hide
Permalink
Matt Oquist added a comment -

Clarified in comments that atomicity is from update of only a single row in the DB. Added (probably) host-unique parameters to uniqid() call, which is otherwise pointless.

Show
Matt Oquist added a comment - Clarified in comments that atomicity is from update of only a single row in the DB. Added (probably) host-unique parameters to uniqid() call, which is otherwise pointless.
Hide
Permalink
Matt Oquist added a comment -

Switched from mdl_ to $CFG->prefix. Will consider Sam's suggestion.

Show
Matt Oquist added a comment - Switched from mdl_ to $CFG->prefix. Will consider Sam's suggestion.
Hide
Permalink
Helen Foster added a comment -

Attachments deleted as requested.

Show
Helen Foster added a comment - Attachments deleted as requested.
Hide
Permalink
Matt Oquist added a comment -

Here's the latest version of this code, with more comments and improved testing options. And I have a bzr repo at http://majen.net/namedlock/ .

Show
Matt Oquist added a comment - Here's the latest version of this code, with more comments and improved testing options. And I have a bzr repo at http://majen.net/namedlock/ .
Hide
Permalink
Matt Oquist added a comment -

The included tests should use system("flock..."). I don't know why I didn't think of that until just now. I'm not updating the code ATM – must do other things.

Show
Matt Oquist added a comment - The included tests should use system("flock..."). I don't know why I didn't think of that until just now. I'm not updating the code ATM – must do other things.
Hide
Permalink
Matt Oquist added a comment -

Perhaps I should mention that after some discussion with Penny, I adopted Sam's suggestion about using a factory method. The attached code is still current.

Show
Matt Oquist added a comment - Perhaps I should mention that after some discussion with Penny, I adopted Sam's suggestion about using a factory method. The attached code is still current.
Hide
Permalink
Martin Dougiamas added a comment -

I'm a bit perplexed. What is this useful for?

Show
Martin Dougiamas added a comment - I'm a bit perplexed. What is this useful for?
Hide
Permalink
Matt Oquist added a comment -

This was inspired by the need to provide named locking for a non-monolithic re-implementation of Moodle's cron, as discussed by Penny and Tim at the dev summit. I realized just now that I'd left Sam's comment hanging, so I figured it wouldn't hurt to note that I'd taken his advice.

Petr, Brian, and I discussed locking quite a bit at the summit, and (with help from Brian) I wrote this code as a result. The first problem is that FS locking doesn't work over NFS, so if we want named locking we need something else.

In short, this library was designed to provide named locking while also avoiding all of Petr's objections.

Show
Matt Oquist added a comment - This was inspired by the need to provide named locking for a non-monolithic re-implementation of Moodle's cron, as discussed by Penny and Tim at the dev summit. I realized just now that I'd left Sam's comment hanging, so I figured it wouldn't hurt to note that I'd taken his advice. Petr, Brian, and I discussed locking quite a bit at the summit, and (with help from Brian) I wrote this code as a result. The first problem is that FS locking doesn't work over NFS, so if we want named locking we need something else. In short, this library was designed to provide named locking while also avoiding all of Petr's objections.
Hide
Permalink
Martin Dougiamas added a comment -

Ah for cron, ok it's all coming back to me. Thanks

Show
Martin Dougiamas added a comment - Ah for cron, ok it's all coming back to me. Thanks

People

Vote (0)
Watch (5)

Dates

  • Created:
    Updated:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.