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

Upsert in bundled app - MongoError: The _id field cannot be changed #2278

Closed
janishorsts opened this Issue Jul 7, 2014 · 6 comments

Comments

Projects
None yet
4 participants
@janishorsts

janishorsts commented Jul 7, 2014

App built with "meteor bundle" fails with "Exception while invoking method 'saveDoc' MongoError: The _id field cannot be changed from {_id: "5SWzAFjndCKZxn42b"} to {_id: "KkQ5JmFKwSHqffLoa"}" when upserting full document. Works fine when not bundled.

Code sample to replicate:
git clone https://github.com/ianhorst/meteor-bundled-upsert-bug.git

Error:

Exception while invoking method 'saveDoc' MongoError: The _id field cannot be changed from {_id: "5SWzAFjndCKZxn42b"} to {_id: "KkQ5JmFKwSHqffLoa"}.
at Object.Future.wait (/Users/user/study/bundle/programs/server/node_modules/fibers/future.js:326:15)
at null.<anonymous> (packages/meteor/helpers.js:111)
at MongoConnection.(anonymous function) [as update] (packages/mongo-livedata/mongo_driver.js:607)
at Meteor.Collection.(anonymous function) [as update] (packages/mongo-livedata/collection.js:476)
at Meteor.Collection.upsert (packages/mongo-livedata/collection.js:501)
at Meteor.methods.saveDoc (app/idFail.js:15:19)
at maybeAuditArgumentChecks (packages/livedata/livedata_server.js:1487)
at packages/livedata/livedata_server.js:643
at _.extend.withValue (packages/meteor/dynamics_nodejs.js:56)
at packages/livedata/livedata_server.js:642
- - - - -
  at Object.toError (/Users/user/study/bundle/programs/server/npm/mongo-livedata/main/node_modules/mongodb/lib/mongodb/utils.js:110:11)
at /Users/user/study/bundle/programs/server/npm/mongo-livedata/main/node_modules/mongodb/lib/mongodb/collection/core.js:542:27
at /Users/user/study/bundle/programs/server/npm/mongo-livedata/main/node_modules/mongodb/lib/mongodb/db.js:1129:7
at /Users/user/study/bundle/programs/server/npm/mongo-livedata/main/node_modules/mongodb/lib/mongodb/db.js:1843:9
at Server.Base._callHandler (/Users/user/study/bundle/programs/server/npm/mongo-livedata/main/node_modules/mongodb/lib/mongodb/connection/base.js:445:41)
at /Users/user/study/bundle/programs/server/npm/mongo-livedata/main/node_modules/mongodb/lib/mongodb/connection/server.js:468:18
at MongoReply.parseBody (/Users/user/study/bundle/programs/server/npm/mongo-livedata/main/node_modules/mongodb/lib/mongodb/responses/mongo_reply.js:68:5)
at null.<anonymous> (/Users/user/study/bundle/programs/server/npm/mongo-livedata/main/node_modules/mongodb/lib/mongodb/connection/server.js:426:20)
at emit (events.js:95:17)
at null.<anonymous> (/Users/user/study/bundle/programs/server/npm/mongo-livedata/main/node_modules/mongodb/lib/mongodb/connection/connection_pool.js:201:13)
@estark37

This comment has been minimized.

Contributor

estark37 commented Jul 7, 2014

Hi @ianhorst. I can't replicate this with the app you linked to (running meteor bundle foo.tgz, untarring, running main.js). Server output after loading the page is Meteor.method[saveDoc] { title: 'Title' }.

Can you please clarify how to reproduce this? Thanks!

@janishorsts

This comment has been minimized.

janishorsts commented Jul 7, 2014

I reduced code to server side only. Please git pull.

I experience the issue on

$ uname -a
Linux host 3.14-0.bpo.1-amd64 #1 SMP Debian 3.14.5-1~bpo70+1 (2014-06-05) x86_64 GNU/Linux
$ uname -a
Darwin Janis-MacBook-Pro.local 13.2.0 Darwin Kernel Version 13.2.0: Thu Apr 17 23:03:13 PDT 2014; 
root:xnu-2422.100.13~1/RELEASE_X86_64 x86_64

Steps to replicate:

$ meteor bundle bundle-upsert-bug.tgz
$ rm -rf ../bundle
$ tar zxf bundle-upsert-bug.tgz -C ../
$ cd ../bundle
$ MONGO_URL="mongodb://127.0.0.1:27017/local" PORT=3000 node main.js 
Meteor.method[saveDoc] { title: 'Title' }
Meteor.call[saveDoc] { error: { [MongoError: The _id field cannot be changed from {_id: "7Hg5sN3A92NQo3dfc"} to {_id: "dexvtArKiMGcrCta5"}.] stack: [Getter] },
  result: undefined }

$ mongo local
> db.docs.count()
0
@estark37

This comment has been minimized.

Contributor

estark37 commented Jul 7, 2014

Still can't reproduce. What version of MongoDB are you running?

@janishorsts

This comment has been minimized.

janishorsts commented Jul 7, 2014

$ mongo --version
MongoDB shell version: 2.6.3
@estark37

This comment has been minimized.

Contributor

estark37 commented Jul 8, 2014

Okay, I see, looks like this is something that changed from MongoDB 2.4 to 2.6.

I'm going to look into this a bit, but we've been generally postponing issues with MongoDB 2.6 for the moment.

@glasser glasser referenced this issue Jul 17, 2014

Closed

MongoDB 2.6 issues #2036

12 of 14 tasks complete
@dgreensp

This comment has been minimized.

Contributor

dgreensp commented Nov 24, 2014

I'm not sure why this would happen, but it is probably due to some detail of our "fake upsert" implementation, and there is probably some relatively simple fix, though no core devs have time to look at this code at this particular moment. Mongo 2.6 support is important, and we will get to it.

Actually, I believe we can dispense with "fake upserts" if we can assume Mongo 2.6 (not sure if we can or not).

Why did we simulate upserts in the first place in Meteor? It took me a minute to remember, so I want to capture it here.

Under Mongo 2.4, you can't specify the _id for upserted documents. If you upsert {foo: 1} and a document is created, the mongo server chooses the _id, and it will be in mongo's ID format. This limitation affected not just Meteor or even Node users, but any application that generated its own IDs or wanted to use IDs that were, say, short strings or small numbers. It's all the more strange because it's considered best practice for the application to generate IDs when inserting documents, but when you go to upsert, the ID is generated internal to the server, end of story. Anyway, we found a work-around using a careful combination of update and insert to avoid race conditions. You can see people discussing the issue with mongo on StackOverflow: http://stackoverflow.com/questions/8844397/upserts-in-mongodb-when-using-custom-id-values The accepted answer isn't correct, but our solution is along those lines, with an extra step or two that makes it correct.

However, in Mongo 2.6, you can use $setOnInsert to set the _id in an upsert! See: https://jira.mongodb.org/browse/SERVER-9958 I also confirmed this in the mongo shell.

There's a second limitation we ran into, in the node mongo driver this time, which has now been fixed in v2.0 (we use a fork of v1.4.1). Suppose you let the mongo server generate the ID for an upsert. There was no way to retrieve it from node! In newer versions of the driver, there is. This is almost irrelevant given the above, but in theory we could update Meteor's version of the node mongo driver in order to enable real upsert in the case where Meteor is in "Mongo ID" mode (where it generates _ids that fit mongo's scheme). However, we should probably gear new code towards the Mongo 2.6 case, not the 2.4 case.

glasser added a commit that referenced this issue Feb 5, 2015

upsert: Detect "can't change _id" in Mongo 2.6
This fixes the "upsert error parse" test and changes the behavior of
other failing upsert tests to be failing with "Upsert failed after 3
tries" errors instead of failing with a "can't change _id"-style error.

First step of addressing #2278.

glasser added a commit that referenced this issue Feb 5, 2015

Make our upserts match 2.6 semantics
Mongo 2.4 had the following behavior: if

  c.update({_id: 'x'}, {y: 1}, {upsert: 1})

resulted in an insert, it would get a random _id instead of 'x'.  (This
only happened when the update was a "replacement" rather than a
"modifier".) We dutifully implemented this behavior in various parts of
Meteor and added tests to prove that it occured.

However, this was actually a bug which got fixed in Mongo 2.6:
https://jira.mongodb.org/browse/SERVER-5289

Since this was regarded by Mongo as a bug (not a backwards-incompatible
change deemed worthy of mention in the Mongo 2.6 release notes), we
should just make Meteor always behave the Mongo 2.6 way, no matter what
version we're on (at least in the aspects where Meteor has its own code
to control this).  In other parts of Meteor where the queries are just
getting processed by Mongo, the behavior of this special case will match
Mongo's behavior.  If you care strongly about consistent behavior in
this strange edge case, upgrade to Mongo 2.6!

Specifically:

 - The minimongo implementation of upsert should pull its _id from the
   selector if it got removed by applying the replacement "modifier".
 - Mongo.Collection.update should only generate a random _id as an
   insertedId if the selector doesn't mention one.
 - The case of a selector mentioning _id and a *replacement* modifier
   not mentioning _id should still count as "known ID" in the
   lower-level MongoConnection.update, which means that there's no
   reason to use simulated upsert and that we should include it as
   insertedId in the return object.
 - Various tests of the previous behavior should be changed.

(Note that if this commit is cherry-picked onto 1.0.3.1 (ie, run against
2.4), it would fail, because some of the tested cases end up going to
unsimulated Mongo upsert and assume the 2.6 behavior. That's OK, as
described above.)

Fixes #2278.

glasser added a commit that referenced this issue Feb 5, 2015

upsert: Detect "can't change _id" in Mongo 2.6
This fixes the "upsert error parse" test and changes the behavior of
other failing upsert tests to be failing with "Upsert failed after 3
tries" errors instead of failing with a "can't change _id"-style error.

First step of addressing #2278.

glasser added a commit that referenced this issue Feb 5, 2015

Make our upserts match 2.6 semantics
Mongo 2.4 had the following behavior: if

  c.update({_id: 'x'}, {y: 1}, {upsert: 1})

resulted in an insert, it would get a random _id instead of 'x'.  (This
only happened when the update was a "replacement" rather than a
"modifier".) We dutifully implemented this behavior in various parts of
Meteor and added tests to prove that it occured.

However, this was actually a bug which got fixed in Mongo 2.6:
https://jira.mongodb.org/browse/SERVER-5289

Since this was regarded by Mongo as a bug (not a backwards-incompatible
change deemed worthy of mention in the Mongo 2.6 release notes), we
should just make Meteor always behave the Mongo 2.6 way, no matter what
version we're on (at least in the aspects where Meteor has its own code
to control this).  In other parts of Meteor where the queries are just
getting processed by Mongo, the behavior of this special case will match
Mongo's behavior.  If you care strongly about consistent behavior in
this strange edge case, upgrade to Mongo 2.6!

Specifically:

 - The minimongo implementation of upsert should pull its _id from the
   selector if it got removed by applying the replacement "modifier".
 - Mongo.Collection.update should only generate a random _id as an
   insertedId if the selector doesn't mention one.
 - The case of a selector mentioning _id and a *replacement* modifier
   not mentioning _id should still count as "known ID" in the
   lower-level MongoConnection.update, which means that there's no
   reason to use simulated upsert and that we should include it as
   insertedId in the return object.
 - Various tests of the previous behavior should be changed.

(Note that if this commit is cherry-picked onto 1.0.3.1 (ie, run against
2.4), it would fail, because some of the tested cases end up going to
unsimulated Mongo upsert and assume the 2.6 behavior. That's OK, as
described above.)

Fixes #2278.

glasser added a commit that referenced this issue Feb 5, 2015

upsert: Detect "can't change _id" in Mongo 2.6
This fixes the "upsert error parse" test and changes the behavior of
other failing upsert tests to be failing with "Upsert failed after 3
tries" errors instead of failing with a "can't change _id"-style error.

First step of addressing #2278.

glasser added a commit that referenced this issue Feb 5, 2015

Make our upserts match 2.6 semantics
Mongo 2.4 had the following behavior: if

  c.update({_id: 'x'}, {y: 1}, {upsert: 1})

resulted in an insert, it would get a random _id instead of 'x'.  (This
only happened when the update was a "replacement" rather than a
"modifier".) We dutifully implemented this behavior in various parts of
Meteor and added tests to prove that it occured.

However, this was actually a bug which got fixed in Mongo 2.6:
https://jira.mongodb.org/browse/SERVER-5289

Since this was regarded by Mongo as a bug (not a backwards-incompatible
change deemed worthy of mention in the Mongo 2.6 release notes), we
should just make Meteor always behave the Mongo 2.6 way, no matter what
version we're on (at least in the aspects where Meteor has its own code
to control this).  In other parts of Meteor where the queries are just
getting processed by Mongo, the behavior of this special case will match
Mongo's behavior.  If you care strongly about consistent behavior in
this strange edge case, upgrade to Mongo 2.6!

Specifically:

 - The minimongo implementation of upsert should pull its _id from the
   selector if it got removed by applying the replacement "modifier".
 - Mongo.Collection.update should only generate a random _id as an
   insertedId if the selector doesn't mention one.
 - The case of a selector mentioning _id and a *replacement* modifier
   not mentioning _id should still count as "known ID" in the
   lower-level MongoConnection.update, which means that there's no
   reason to use simulated upsert and that we should include it as
   insertedId in the return object.
 - Various tests of the previous behavior should be changed.

(Note that if this commit is cherry-picked onto 1.0.3.1 (ie, run against
2.4), it would fail, because some of the tested cases end up going to
unsimulated Mongo upsert and assume the 2.6 behavior. That's OK, as
described above.)

Fixes #2278.

glasser added a commit that referenced this issue Feb 6, 2015

upsert: Detect "can't change _id" in Mongo 2.6
This fixes the "upsert error parse" test and changes the behavior of
other failing upsert tests to be failing with "Upsert failed after 3
tries" errors instead of failing with a "can't change _id"-style error.

First step of addressing #2278.

glasser added a commit that referenced this issue Feb 6, 2015

Make our upserts match 2.6 semantics
Mongo 2.4 had the following behavior: if

  c.update({_id: 'x'}, {y: 1}, {upsert: 1})

resulted in an insert, it would get a random _id instead of 'x'.  (This
only happened when the update was a "replacement" rather than a
"modifier".) We dutifully implemented this behavior in various parts of
Meteor and added tests to prove that it occured.

However, this was actually a bug which got fixed in Mongo 2.6:
https://jira.mongodb.org/browse/SERVER-5289

Since this was regarded by Mongo as a bug (not a backwards-incompatible
change deemed worthy of mention in the Mongo 2.6 release notes), we
should just make Meteor always behave the Mongo 2.6 way, no matter what
version we're on (at least in the aspects where Meteor has its own code
to control this).  In other parts of Meteor where the queries are just
getting processed by Mongo, the behavior of this special case will match
Mongo's behavior.  If you care strongly about consistent behavior in
this strange edge case, upgrade to Mongo 2.6!

Specifically:

 - The minimongo implementation of modifiers loses its special-case code
   which drops _id from the modified document during an upsert replace.
 - Mongo.Collection.update should only generate a random _id as an
   insertedId if the selector doesn't mention one.
 - The case of a selector mentioning _id and a *replacement* modifier
   not mentioning _id should still count as "known ID" in the
   lower-level MongoConnection.update, which means that there's no
   reason to use simulated upsert and that we should include it as
   insertedId in the return object.
 - Various tests of the previous behavior should be changed.

(Note that if this commit is cherry-picked onto 1.0.3.1 (ie, run against
2.4), it would fail, because some of the tested cases end up going to
unsimulated Mongo upsert and assume the 2.6 behavior. That's OK, as
described above.)

Fixes #2278.

glasser added a commit that referenced this issue Feb 6, 2015

upsert: Detect "can't change _id" in Mongo 2.6
This fixes the "upsert error parse" test and changes the behavior of
other failing upsert tests to be failing with "Upsert failed after 3
tries" errors instead of failing with a "can't change _id"-style error.

First step of addressing #2278.

glasser added a commit that referenced this issue Feb 6, 2015

Make our upserts match 2.6 semantics
Mongo 2.4 had the following behavior: if

  c.update({_id: 'x'}, {y: 1}, {upsert: 1})

resulted in an insert, it would get a random _id instead of 'x'.  (This
only happened when the update was a "replacement" rather than a
"modifier".) We dutifully implemented this behavior in various parts of
Meteor and added tests to prove that it occured.

However, this was actually a bug which got fixed in Mongo 2.6:
https://jira.mongodb.org/browse/SERVER-5289

Since this was regarded by Mongo as a bug (not a backwards-incompatible
change deemed worthy of mention in the Mongo 2.6 release notes), we
should just make Meteor always behave the Mongo 2.6 way, no matter what
version we're on (at least in the aspects where Meteor has its own code
to control this).  In other parts of Meteor where the queries are just
getting processed by Mongo, the behavior of this special case will match
Mongo's behavior.  If you care strongly about consistent behavior in
this strange edge case, upgrade to Mongo 2.6!

Specifically:

 - The minimongo implementation of modifiers loses its special-case code
   which drops _id from the modified document during an upsert replace.
 - Mongo.Collection.update should only generate a random _id as an
   insertedId if the selector doesn't mention one.
 - The case of a selector mentioning _id and a *replacement* modifier
   not mentioning _id should still count as "known ID" in the
   lower-level MongoConnection.update, which means that there's no
   reason to use simulated upsert and that we should include it as
   insertedId in the return object.
 - Various tests of the previous behavior should be changed.

(Note that if this commit is cherry-picked onto 1.0.3.1 (ie, run against
2.4), it would fail, because some of the tested cases end up going to
unsimulated Mongo upsert and assume the 2.6 behavior. That's OK, as
described above.)

Fixes #2278.

glasser added a commit that referenced this issue Feb 6, 2015

upsert: Detect "can't change _id" in Mongo 2.6
This fixes the "upsert error parse" test and changes the behavior of
other failing upsert tests to be failing with "Upsert failed after 3
tries" errors instead of failing with a "can't change _id"-style error.

First step of addressing #2278.

@glasser glasser closed this in a83008e Feb 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment