Enable a custom starting id #1634

stampycode · 2017-08-22T14:07:04Z

Need to be able to set a custom starting point for an auto-increment field - to prevent collisions when combining datasets, for example.

Need to be able to set a custom starting ID for a collection - to prevent collisions when combining datasets, for example.

alcaeus

See mentioned issue. Also needs tests.

alcaeus · 2017-08-22T14:10:24Z

lib/Doctrine/ODM/MongoDB/Id/IncrementGenerator.php

@@ -59,7 +65,7 @@ public function generate(DocumentManager $dm, $document)
        $key = $this->key ?: $dm->getDocumentCollection($className)->getName();

        $query = array('_id' => $key);
-        $newObj = array('$inc' => array('current_id' => 1));
+        $newObj = array('$inc' => array('current_id' => $this->startingId));


This doesn't change the starting value, but changes the increment. Setting startingId to 5 would create a sequence of [5, 10, 15, 20, ...], not [5, 6, 7, 8, 9, ...] as one would expect.

Had to look it up in the docs, but you can use $setOnInsert to set the initial value if the document was inserted:

$newObj = [ '$inc' => ['current_id' => 1], '$setOnInsert' => ['current_id' => $this->startingId], ];

@alcaeus you legend. Will commit shortly. With tests.

Unable to use '$inc' and '$setOnInsert' together due to known bug. https://jira.mongodb.org/browse/SERVER-10711 Results in error: Cannot update 'current_id' and 'current_id' at the same time Workaround from here: https://stackoverflow.com/questions/41552405/mongodb-collection-update-initialize-a-document-with-default-values/41953190#41953190 _Tests to follow..._

alcaeus · 2017-08-23T08:42:50Z

lib/Doctrine/ODM/MongoDB/Id/IncrementGenerator.php

-        $command['new'] = true;
+        /*
+         * Unable to use '$inc' and '$setOnInsert' together due to known bug.
+         * @see https://jira.mongodb.org/browse/SERVER-10711


Almost 4 years and counting. Yay! Workaround looks good though 👍

Only thing that isn't 100% is the atomicity - I assume the upsert is atomic, but this workaround certainly isn't. Not sure whether adding a collection lock would suck performance...

The upsert would be atomic, yes. With the workaround, I'm hoping that the combination of "how often will this feature be used" and "how often would an upsert have to happen" narrows it down enough for it not to be a problem.

I think we can preserve atomicity with the following two-step process:

// 1. Optimize for the common case: increment existing sequence $command = [ 'findAndModify' => $coll, 'query' => ['_id' => $key, 'current_id' => ['$exists' => true]], 'update' => ['$inc' => ['current_id' => $this->startingId]], 'upsert' => false, 'new' => true, ]; /* 2. If no result was returned, we upsert a new sequence. Don't bother to * incorporate {$exists: false} into the criteria as that doesn't won't avoid * an exception during a possible race condition. */ $command = [ 'findAndModify' => $coll, 'query' => ['_id' => $key], 'update' => ['$inc' => ['current_id' => $this->startingId]], 'upsert' => true, 'new' => true, ]; /* 3. If a duplicate key exception was thrown, we encountered a race condition * where another process created the sequence between our previous findAndModify * commands. In that case, we can ignore the exception and attempt the first, * increment approach one more time (perhaps throwing our exception if that then * fails -- indicating that someone is rapidly inserting and deleting documents * in the sequence table. */

Ok I've made some of the recommended changes, just need to find and catch the duplicate key exception...
... and write tests...

jmikola · 2017-08-23T21:55:34Z

lib/Doctrine/ODM/MongoDB/Id/IncrementGenerator.php

+                'insert' => $coll,
+                'documents' => array(array('_id' => $key, 'current_id' => $this->startingId))
+            );
+            $result = $db->command($command);


If we're going to perform an insert, this should use Collection::insert() for consistency instead of manually crafting an insert command (something the PHP library) doesn't even do).

jmikola · 2017-08-23T21:59:07Z

lib/Doctrine/ODM/MongoDB/Id/IncrementGenerator.php

+         * Results in error: Cannot update 'current_id' and 'current_id' at the same time
+         */
+        $command = array(
+            'findandmodify' => $coll,


I don't know offhand what case variations mongod allows, but the canonical name is findAndModify.

jmikola · 2017-08-23T22:08:24Z

lib/Doctrine/ODM/MongoDB/Id/IncrementGenerator.php

-        $command['new'] = true;
+        /*
+         * Unable to use '$inc' and '$setOnInsert' together due to known bug.
+         * @see https://jira.mongodb.org/browse/SERVER-10711


I think we can preserve atomicity with the following two-step process:

// 1. Optimize for the common case: increment existing sequence $command = [ 'findAndModify' => $coll, 'query' => ['_id' => $key, 'current_id' => ['$exists' => true]], 'update' => ['$inc' => ['current_id' => $this->startingId]], 'upsert' => false, 'new' => true, ]; /* 2. If no result was returned, we upsert a new sequence. Don't bother to * incorporate {$exists: false} into the criteria as that doesn't won't avoid * an exception during a possible race condition. */ $command = [ 'findAndModify' => $coll, 'query' => ['_id' => $key], 'update' => ['$inc' => ['current_id' => $this->startingId]], 'upsert' => true, 'new' => true, ]; /* 3. If a duplicate key exception was thrown, we encountered a race condition * where another process created the sequence between our previous findAndModify * commands. In that case, we can ignore the exception and attempt the first, * increment approach one more time (perhaps throwing our exception if that then * fails -- indicating that someone is rapidly inserting and deleting documents * in the sequence table. */

alcaeus · 2017-09-27T17:49:04Z

@stampycode could you take a look at the issues @jmikola and I mentioned? I'd like to wrap up ODM 1.2 soon and this is currently on the milestone.

stampycode · 2017-10-20T08:53:43Z

@jmikola @alcaeus
So I've run into a stumbling block in my tests.
Using the code I originally suggested works ok:

set $startingId to 1000001
first element fails to find non-existent counter
creates counter with number 1000001
returns 1000001
second request finds counter, increments it
returns 1000002

Unfortunately, and for reasons I have not yet fathomed, the suggested changes do not have the desired effect:

set $startingId to 1000001
first element fails to find non-existent counter
creates counter with number 2000001
returns 2000001
second request finds counter, increments it
returns 3000001

I have no idea what is going on here.

alcaeus · 2017-10-22T17:45:41Z

@stampycode I took a look at the changes, looks like there was an error in the code @jmikola suggested. The first command of course has to increment the current_id field by one, not by the starting value. Only the second command that is supposed to upsert a record may increment it with the starting value. I've pushed a fix to #1661 and merge it once the build has finished. Thanks!

jmikola · 2017-10-23T18:03:52Z

@alcaeus: Indeed, my example was incrementing by startingId instead of 1. @stampycode: Sorry for the misleading suggestion!

Enable a custom starting id

5b72eab

Need to be able to set a custom starting ID for a collection - to prevent collisions when combining datasets, for example.

alcaeus requested changes Aug 22, 2017

View reviewed changes

alcaeus added the Feature label Aug 22, 2017

alcaeus added this to the 1.2 milestone Aug 22, 2017

alcaeus added the Needs Test label Aug 22, 2017

alcaeus reviewed Aug 23, 2017

View reviewed changes

jmikola reviewed Aug 23, 2017

View reviewed changes

stampycode added 2 commits September 29, 2017 14:12

renamed 'findandmodify' to 'findAndModify'

7d06a2e

replaced insert with upsert

946b1ef

jmikola self-assigned this Oct 10, 2017

alcaeus mentioned this pull request Oct 22, 2017

Enable a custom starting id #1661

Merged

alcaeus removed this from the 1.2 milestone Oct 22, 2017

alcaeus closed this Oct 22, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable a custom starting id #1634

Enable a custom starting id #1634

stampycode commented Aug 22, 2017

alcaeus left a comment

alcaeus Aug 22, 2017

stampycode Aug 22, 2017

alcaeus Aug 22, 2017 •

edited

Loading

stampycode Aug 22, 2017

alcaeus Aug 23, 2017

stampycode Aug 23, 2017

alcaeus Aug 23, 2017

jmikola Aug 23, 2017

stampycode Sep 29, 2017

jmikola Aug 23, 2017

jmikola Aug 23, 2017

jmikola Aug 23, 2017

alcaeus commented Sep 27, 2017

stampycode commented Oct 20, 2017

alcaeus commented Oct 22, 2017

jmikola commented Oct 23, 2017

Enable a custom starting id #1634

Enable a custom starting id #1634

Conversation

stampycode commented Aug 22, 2017

alcaeus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alcaeus Aug 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alcaeus commented Sep 27, 2017

stampycode commented Oct 20, 2017

alcaeus commented Oct 22, 2017

jmikola commented Oct 23, 2017

alcaeus Aug 22, 2017 •

edited

Loading