-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add docs for the Lock component #7364
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left lots of comments ... but they are for very minor typos. Overall this doc is in good shape!
@jderusse thanks for implementing this feature and documenting it!
components/lock.rst
Outdated
The Lock Component | ||
==================== | ||
|
||
The Lock Component provides a mechanism to garentee an exclusive access into |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
garentee
-> guarantee
components/lock.rst
Outdated
Then, you can ask to this store to create a Lock for your ``resource``. | ||
|
||
The :method:`Symfony\\Component\\Lock\\LockInterface::acquire` method tries to | ||
acquire the lock. If the lock is can not be acquired, the method throws a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is can not
-> can not
components/lock.rst
Outdated
The :method:`Symfony\\Component\\Lock\\LockInterface::acquire` method tries to | ||
acquire the lock. If the lock is can not be acquired, the method throws a | ||
:class:`Symfony\\Component\\Lock\\Exception\\LockConflictedException`. You can | ||
safly call the ``acquire()`` method several time, even if you already acquired |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
safly
-> safely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
several time
-> several times
components/lock.rst
Outdated
use Symfony\Component\Lock\Exception\LockConflictedException; | ||
|
||
$store = new SemaphoreStore(); | ||
$lock = $store->createLock('hello'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This component may be tricky to understand to some people, so I recomment to use realistic code samples (in other words, replace hello
, foo
, bar
with real-world looking examples).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll fix it.
Do you think a Key Concepts
introduction on top of this page (like in the cache component) would be usefull?
components/lock.rst
Outdated
// the resource "hello" is already locked by another process | ||
} | ||
|
||
The first argument of `createLock` is a string representation of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double backticks for createLock
components/lock.rst
Outdated
That's why, when you create a lock on an expirable ``Store``. You have to choose | ||
carrefully the correct TTL. When too low, you take the risk to "loose" the lock | ||
(and someone else acquire it) wheras you don't finish your task. | ||
When too hight and the process crash before you call the ``release()`` method, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hight
-> high
components/lock.rst
Outdated
|
||
.. tip:: | ||
|
||
To avoid to let the Lock in a locking state, try to always release an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid to let
-> To avoid letting
components/lock.rst
Outdated
To avoid to let the Lock in a locking state, try to always release an | ||
expirable lock by wraping the job in a try/catch block for instance. | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra blank line here
components/lock.rst
Outdated
|
||
|
||
When you have to work on a really long task, you should not set the TTL to | ||
overlaps the duration of this task. Instead, the Lock Component expose a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overlaps
-> overlap
components/lock.rst
Outdated
``Stores`` are classes that implement :class:`Symfony\\Component\\Lock\\StoreInterface`. | ||
This component provides several adapters ready to use in your applications. | ||
|
||
Here is a small summary of availanble ``Stores`` and theire capabilities. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
availanble
-> available
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
theire
-> their
Thank you @javiereguiluz for this review. Do you think this page should be split into several sub-pages? for instance, one page for advanced usage (expiration, blocking....) and one for listing stores? |
@jderusse about splitting this into several pages ... we usually start with one page and split if needed later. For now the docs of this component are not too long, so I guess we can keep the single page doc for now. |
components/lock.rst
Outdated
SemaphoreStore | ||
~~~~~~~~~~~~~~ | ||
|
||
The SemaphoreStore uses the PHP semaphore function to lock a ``resources``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This phrase is confusing "the PHP semaphore function" ... because PHP doesn't have a semaphore()
function, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, I was thinking about PHP semaphore function**s**
is it more clear? Or should I rewrite the sentence?
components/lock.rst
Outdated
|
||
$lock->acquire(true); | ||
|
||
Expirable Locks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some doubts about the Expirable
word. "Official" dictionaries don't include it, but Internet dictionaries include it. In any case, after reading this discussion maybe it's a good idea to rename it to Expiring Locks
.
The key is that "expirable lock" means "it can expire" (maybe it will expire or maybe it will not) whereas "expiring lock" means "it will expire" (it's 100% sure that this lock will expire).
components/lock.rst
Outdated
single: Components; Lock | ||
|
||
The Lock Component | ||
==================== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too many ==
components/lock.rst
Outdated
$lock->acquire(true); | ||
|
||
Expiring Locks | ||
--------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra -
components/lock.rst
Outdated
If you want to share a lock in several services. You have to share the | ||
instance of Lock returned by the ``Factory::createLock`` method. | ||
|
||
Blocking locks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing cap to locks
components/lock.rst
Outdated
Expiring Locks | ||
--------------- | ||
|
||
Working with a remote ``Store`` is hard. There is now way for the remote |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no way?
components/lock.rst
Outdated
--------------- | ||
|
||
Working with a remote ``Store`` is hard. There is now way for the remote | ||
``Store`` to know if the locker process is till alive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still?
components/lock.rst
Outdated
``release()`` method will be called, which would cause a ``resource`` to be locked | ||
infinitely. | ||
|
||
To fill this gap, the remote ``Stores`` provide an expiration mechanism: The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Store?
components/lock.rst
Outdated
|
||
To fill this gap, the remote ``Stores`` provide an expiration mechanism: The | ||
lock is acquired for a defined amount of time (named TTL for Time To Live). | ||
When the timeout occurred, the lock is automatically released even if the locker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
occurs?
components/lock.rst
Outdated
FlockStore | ||
~~~~~~~~~~ | ||
|
||
The FlockStore use the fileSystem on the local computer to lock and store the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uses
components/lock.rst
Outdated
~~~~~~~~~~ | ||
|
||
The FlockStore use the fileSystem on the local computer to lock and store the | ||
``resource``. It does not supports expiration, but the lock is automatically |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
support
components/lock.rst
Outdated
|
||
.. caution:: | ||
|
||
Beware, some filesystem (like some version of NFS) does not support locking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filesystems do not
components/lock.rst
Outdated
~~~~~~~~~~~~~~ | ||
|
||
The MemcachedStore stores state of ``resource`` in a Memcached server. This | ||
``Store`` does not support blocking, and expect a TLL to avoid infinity locks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expects
components/lock.rst
Outdated
|
||
.. note:: | ||
|
||
Memcached does not supports TTL lower than 1 seconds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 second
4539a99
to
2498ccd
Compare
Comment addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this great component and docs, I'm looking forward to have a use case to play with this.
I'm sorry again as I've left many minor comments here too, I've made some suggestions to avoid using "you" as much as possible.
----- | ||
|
||
Locks are used to guarantee exclusive access to some shared resource. In | ||
Symfony applications, you can use locks for example to ensure that a command is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"you can use locks" => "locks can be used"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be redundant with the first sentence Locks are used...
don't you think?
components/lock.rst
Outdated
Symfony applications, you can use locks for example to ensure that a command is | ||
not executed more than once at the same time (on the same or different servers). | ||
|
||
In order to manage the state of locks, you first need to create a ``Store`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"you first need to create a Store
" => "a Store
needs to be created first"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this change, I've the feeling that the rest of the sentence is weird: a Store needs to be created first and then use the Symfony\Component\Lock\Factory class ...
WDYT?
components/lock.rst
Outdated
$store = new SemaphoreStore(); | ||
$factory = new Factory($store); | ||
|
||
Then, call to the :method:`Symfony\\Component\\Lock\\LockInterface::acquire` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Then, a call ... will try to"
components/lock.rst
Outdated
$lock->release(); | ||
} | ||
|
||
If the lock can not be acquired, the method returns ``false``. You can safely |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"You can safely call the acquire()
method repeatedly" => "The acquire()
method can be safely called repeatedly"
components/lock.rst
Outdated
} | ||
|
||
If the lock can not be acquired, the method returns ``false``. You can safely | ||
call the ``acquire()`` method repeatedly, even if you already acquired it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"even if you already acquired it" => "even if the lock is already acquired"
components/lock.rst
Outdated
.. caution:: | ||
|
||
Beware that some file systems (such as some types of NFS) do not support | ||
locking. In those cases, it's better to use a local file or a remote store |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a local file?
components/lock.rst
Outdated
MemcachedStore | ||
~~~~~~~~~~~~~~ | ||
|
||
The MemcachedStore saves locks on a Memcached server, so first you must create |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"so first you must create a Memcached connection implements the \Memcached
class."
=>
"it requires a Memcached connection implementing the \Memcached
class."
components/lock.rst
Outdated
RedisStore | ||
~~~~~~~~~~ | ||
|
||
The RedisStore saves locks on a Redis server, so first you must create a Redis |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
components/lock.rst
Outdated
manages several stores in sync (for example, several Redis servers). When a lock | ||
is being acquired, it forwards the call to all the managed stores, and it | ||
collects their responses. If a simple majority of stores have acquired the lock, | ||
then the lock is considered as acquired; otherwise is not acquired:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
otherwise as not acquired?
components/lock.rst
Outdated
|
||
$store = new CombinedStore($stores, new ConsensusStrategy()); | ||
|
||
Instead of the simple majority strategy (``ConsensusStrategy``) you can use the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"you can use the UnanimousStrategy
to require the lock" => "an UnanimousStrategy
can be used to require the lock"
This PR was squashed before being merged into the 3.3-dev branch (closes #21093). Discussion ---------- [Lock] Create a lock component | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | they will | Fixed tickets | #20382 | License | MIT | Doc PR | symfony/symfony-docs#7364 This PR aim to add a new component Lock going further than the FileSystem\LockHandler by allowing remote backend (like Redis, memcache, etc) Inspired by ## Usage The simplest way to use lock is to inject an instance of a Lock in your service ```php class MyService { private $lock; public function __construct(LockInterface $lock) { $this->lock = $lock; } public function run() { $this->lock->acquire(true); // If I'm here, no exception had been raised. Lock is acquired try { // do my job } finally { $this->lock->release(); } } } ``` Configured with something like ```yaml services: app.my_service: class: AppBundle\MyService arguments: - app.lock.my_service app.lock.my_service: class: Symfony\Component\Lock\Lock factory: ['@locker', createLock] arguments: ['my_service'] ``` If you need to lock serveral resource on runtime, wou'll nneed to inject the LockFactory. ```php class MyService { private $lockFactory; public function __construct(LockFactoryInterface $lockFactory) { $this->lockFactory = $lockFactory; } public function run() { foreach ($this->items as $item) { $lock = $this->lockFactory->createLock((string) $item); try { $lock->acquire(); } catch (LockConflictedException $e) { continue; } // When I'm here, no exception had been, raised. Lock is acquired try { // do my job } finally { $lock->release(); } } } } ``` Configured with something like ```yaml services: app.my_service: class: AppBundle\MyService arguments: - '@locker' ``` This component allow you to refresh an expirable lock. This is usefull, if you run a long operation split in several small parts. If you lock with a ttl for the overall operatoin time and your process crash, the lock will block everybody for the defined TTL. But thank to the refresh method, you're able to lock for a small TTL, and refresh it between each parts. ```php class MyService { private $lock; public function __construct(LockInterface $lock) { $this->lock = $lock; } public function run() { $this->lock->acquire(true); try { do { $finished = $this->performLongTask(); // Increase the expire date by 300 more seconds $this->lock->refresh(); } while (!$finished) // do my job } finally { $this->lock->release(); } } } ``` ## Naming anc implementation choise ``` $lock->acquire() vs $lock->lock() ``` Choose to use acquire, because this component is full of `lock` Symfony\Component\Lock\Lock::Lock` raised a E_TOO_MANY_LOCK in my head. ``` $lock->acquire(false); $lock->acquire(true); vs $lock->aquire() $lock->waitAndAquire() ``` Not a big fan of flag feature and 2. But I choose to use the blocking flag to offer a simple (and common usecase) implementation ``` $lock = $factory->createLock($key); $lock->acquire(); vs $lock->aquire($key) ``` I choose to a the pool of locks implementation. It allow the user to create 2 instances and use cross lock even in the same process. ``` interface LockInterface final class Lock implements LockInterface vs final class Lock ``` I choose to use a Interface even if there is only one implementaiton to offer an extension point here # TODO ## In this PR * [x] tests * [x] add logs * [x] offer several redis connectors * [x] try other store implementation to validate the architecture/interface ## In other PR * documentation * add configuration in framework bundle * add stop watch in the debug bar * improve the combined store (takes the drift into account and elapsed time between each store) * implement other stores (memcache, ...) * use this component in session manipulation (fixes #4976) Commits ------- 018e0fc [Lock] Create a lock component
components/lock.rst
Outdated
|
||
Instead of the simple majority strategy (``ConsensusStrategy``) an | ||
``UnanimousStrategy`` can be used to require the lock to be acquired in all | ||
he stores. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be the
instead of he
.
6ae4eeb
to
01dc357
Compare
01dc357
to
3035fe9
Compare
A note to mergers: I propose to merge this as is, so we can have the docs for the Lock component as soon as possible. The integration of Lock inside the Symfony framework is still pending, so let's not wait until that is finished (we'll do that in a separate PR). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Jérémy, what a great work you did contributing these docs and the related component code. Thanks a lot! |
This PR was merged into the master branch. Discussion ---------- Add docs for the Lock component See symfony/symfony#21093 Commits ------- 3035fe9 Rename quorum into strategy d4326c0 Reduce usage of "you" d6a218c Rename quorum strategy 17248e1 Remove codeblock 6b6d865 Final review and some minor rewords/simplifications 2498ccd Fix typo d1d3b71 Fixed minor typos and syntax issues 60bfe03 Fix typo 6469016 Add the Lock Component
See symfony/symfony#21093