Skip to content

[Lock] Added Symfony/Component/Lock/Store/MongoDbStore #27346

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.

Sign up for GitHub

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

Closed
wants to merge 29 commits into from
Closed

[Lock] Added Symfony/Component/Lock/Store/MongoDbStore #27346

Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
157fcd7
#27345 Added Symfony/Component/Lock/Store/MongoDbStore
May 23, 2018
e73f663
#27345 Fixed typo
May 23, 2018
3838412
#27345 fixed coding standards
May 23, 2018
1a660e6
#27345 Removed dev requirement for mongodb/mongodb
May 23, 2018
d65fddf
#27345 Removed unnessasary reduceLifetime
May 24, 2018
f3cc220
#27345 Improved documentation, exception handling, removed configurab…
May 31, 2018
ec8d217
#27345 fixed typo and @param indentation
May 31, 2018
a1a3be7
#27345 fixed @param indentation
May 31, 2018
1b150ba
#27345 moved clock discrepancy handling to a constructor option 'drift'
May 31, 2018
73bb802
#27345 fixed phpdoc spacing
May 31, 2018
5e86904
#27345 removed drift option in favour or recommending setting TTL higher
May 31, 2018
8572c21
#27345 Added public createTTLIndex method
Jun 5, 2018
dbdad36
#27345 Fixed coding standards
Jun 5, 2018
84c4d15
#27345 Fixed coding standards
Jun 5, 2018
d577191
#27345 Fixed coding standards
Jun 5, 2018
2ce68f5
#27345 Fixed coding standards
Jun 5, 2018
5446b11
#27345 Improved documentation
Jun 5, 2018
a9b85d6
#27345 Fixed spacing for fabbot
Jun 5, 2018
6fdc602
#27345 reordered documentation in order of importance
Jun 5, 2018
9d80a0d
#27345 Fixed spacing for fabbot
Jun 5, 2018
1ac702a
#27345 Fixed spacing for fabbot
Jun 5, 2018
4ba5b41
#27345 Updated return type and exception handling on createTTLIndex
Jun 6, 2018
58ef873
#27345 Updated exception handling on createTTLIndex
Jun 6, 2018
36c3af4
#27345 Fixed spacing for fabbot
Jun 6, 2018
168f194
#27345 Removed initialTtl from array and fixed unit test
Jun 6, 2018
abc72f8
#27345 phpdoc adjusted for fabbot
Jun 6, 2018
e062bf2
#27345 phpdoc adjusted for fabbot
Jun 6, 2018
f2e23b4
#27345 trigger ci retry
Jun 14, 2018
97e5711
#27345 trigger ci retry
Jun 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
#27345 Fixed typo
  • Loading branch information
Joe Bennett committed May 23, 2018
commit e73f663b62d4816c68e5d8e3aa39febdde17c8a6
10 changes: 5 additions & 5 deletions src/Symfony/Component/Lock/Store/MongoDbStore.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class MongoDbStore implements StoreInterface
* * collection: The name of the collection [default: lock]
* * resource_field: The field name for storing the lock id [default: _id] MUST be uniquely indexed if you chage it
* * token_field: The field name for storing the lock token [default: token]
* * aquired_field: The field name for storing the acquisition timestamp [default: aquired_at]
* * acquired_field: The field name for storing the acquisition timestamp [default: acquired_at]
* * expiry_field: The field name for storing the expiry-timestamp [default: expires_at].
*
* It is strongly recommended to put an index on the `expiry_field` for
Expand Down Expand Up @@ -70,7 +70,7 @@ public function __construct(\MongoDB\Client $mongo, array $options = [], float $
'collection' => 'lock',
'resource_field' => '_id',
'token_field' => 'token',
'aquired_field' => 'aquired_at',
'acquired_field' => 'acquired_at',
'expiry_field' => 'expires_at',
], $options);

Expand All @@ -90,7 +90,7 @@ public function __construct(\MongoDB\Client $mongo, array $options = [], float $
* {
* _id: "test",
* token: {# unique token #},
* aquired: new Date(),
* acquired: new Date(),
* expires_at: new Date({# now + ttl #})
* },
* {
Expand Down Expand Up @@ -120,7 +120,7 @@ public function save(Key $key)
'$set' => [
$this->options['resource_field'] => (string)$key,
$this->options['token_field'] => $this->getToken($key),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the method $this->getToken($key) is already called at line 108

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

$this->options['aquired_field'] => $this->createDateTime(),
$this->options['acquired_field'] => $this->createDateTime(),
$this->options['expiry_field'] => $expiry,
],
];
Expand All @@ -133,7 +133,7 @@ public function save(Key $key)
try {
$this->getCollection()->updateOne($filter, $update, $options);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it atomic? on every version of mongodb? What's about réplication propagation?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MongoDB is atomic on a singular statement / document only. Hence the use of a update with filter (1 statement). MongoDB ^4.0 introduces multi statement transactions however this implementation does not rely on that. It should be atomic on all versions. Replication propagation can be controlled with a statement level options called a write concern. I'm adding support for that now. By default, mongo will read and write only to the PRIMARY node in a replica set (this is equivalent of reading and writing only to the master of a replicated RDBMS with replication slaves).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With regards to writeConcern, readConcern and readPreference I've opted to leave this to the collection fallback placing the onus on the DBA to setup the collection with alternate settings if desired. The default when left unspecified is to read / write ONLY to the PRIMARY of a replica set. This is safe for the use of locks (it's like only reading / writing to a master in an RDBMS) however an alternate strategy could be left to the DBA depending on the number of nodes they have.

I have added some documentation regarding this decision to symfony-docs

} catch (\MongoDB\Driver\Exception\BulkWriteException $e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO you should distringuished 2 cases

  • The lock is not acquired because already acaquired by someone else
  • Something goes wrong (collection does not exists, database not reachable, bad permission...)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've improved the exception handling, I also noticed waitAndSave was throwing an InvalidArgumentException instead of a NotSupportedException. The RedisStore also had this issue. I have fixed the MondoDbStore but we should also fix the RedisStore. I also adjusted the phpdoc in StoreInterface to include LockExpiredException in save, waitAndSave and putOffExpiration as per other store implementations.

throw new LockConflictedException('Failed to aquire lock', 0, $e);
throw new LockConflictedException('Failed to acquire lock', 0, $e);
}

if ($key->isExpired()) {
Expand Down