The Assert field in mgo/txn.Op is an interface{}, so
when it's marshaled and unmarshaled, the order
can change because unmarshaling unmarshals as bson.M
which does not preserve key order.https://play.golang.org/p/_1ZPl7iMyn On 8 June 2016 at 15:55, Gustavo Niemeyer <gustavo.nieme...@canonical.com> wrote: > Is it mgo itself that is changing the order internally? > > It should not do that. > > On Wed, Jun 8, 2016 at 8:00 AM, roger peppe <rogpe...@gmail.com> wrote: >> >> OK, I understand now, I think. >> >> The underlying problem is that subdocument searches in MongoDB >> are order-sensitive. >> >> For example, I just tried this in a mongo shell: >> >> > db.foo.insert({_id: "one", x: {a: 1, b: 2}}) >> > db.foo.find({x: {a: 1, b: 2}}) >> { "_id" : "one", "x" : { "a" : 1, "b" : 2 } } >> > db.foo.find({x: {b: 2, a: 1}}) >> > >> >> The second find doesn't return anything even though it contains >> the same fields with the same values as the first. >> >> Urk. I did not know about that. What a gotcha! >> >> So it *could* technically be OK if the fields in the struct (and >> any bson.D) are lexically ordered to match the bson Marshaler, >> but well worth avoiding. >> >> I think things would be considerably improved if mgo/bson preserved >> order by default (by using bson.D) when unmarshaling. >> Then at least you'd know that the assertion you specify >> is exactly the one that gets executed. >> >> cheers, >> rog. >> >> >> >> >> On 8 June 2016 at 10:42, Menno Smits <menno.sm...@canonical.com> wrote: >> > >> > >> > On 8 June 2016 at 21:05, Tim Penhey <tim.pen...@canonical.com> wrote: >> >> >> >> Hi folks, >> >> >> >> tl;dr: not use structs in transaction asserts >> >> >> >> ... >> >> >> >> The solution is to not use a field struct equality, even though it is >> >> easy >> >> to write, but to use the dotted field notation to check the embedded >> >> field >> >> values. >> > >> > >> > >> > To give a more concrete example, asserting on a embedded document field >> > like >> > this is problematic: >> > >> > ops := []txn.Op{{ >> > C: "collection", >> > Id: ..., >> > Assert: bson.D{{"some-field", Thing{A: "foo", B: 99}}}, >> > Update: ... >> > } >> > >> > Due to the way mgo works[1], the document the transaction operation is >> > asserting against may have been written with A and B in reverse order, >> > or >> > the Thing struct in the Assert may have A and B swapped by the time it's >> > used. Either way, the assertion will fail randomly. >> > >> > The correct approach is to express the assertion like this: >> > >> > ops := []txn.Op{{ >> > C: "collection", >> > Id: ..., >> > Assert: bson.D{ >> > {"some-field.A", "foo"}, >> > {"some-field.B", 99}, >> > }, >> > Update: ... >> > } >> > >> > or this: >> > >> > ops := []txn.Op{{ >> > C: "collection", >> > Id: ..., >> > Assert: bson.M{ >> > "some-field.A": "foo", >> > "some-field.B": 99, >> > }, >> > Update: ... >> > } >> > >> >> >> >> Yet another thing to add to the list of things to check when doing >> >> reviews. >> > >> > >> > I think we can go a bit further and error on attempts to use structs for >> > comparison in txn.Op asserts in Juju's txn layers in state. Just as we >> > already do some munging and checking of database operations to ensure >> > correct multi-model behaviour, we should be able to do this same for >> > this >> > issue and prevent it from happening again. >> > >> > - Menno >> > >> > [1] If transaction operations are loaded and used from the DB (more >> > likely >> > under load when multiple runners are acting concurrently), the Insert, >> > Update and Assert fields are loaded as bson.M (this is what the bson >> > Unmarshaller does for interface{} typed fields). Once this happens field >> > ordering is lost. >> > >> > >> > >> > >> > -- >> > Juju-dev mailing list >> > Juju-dev@lists.ubuntu.com >> > Modify settings or unsubscribe at: >> > https://lists.ubuntu.com/mailman/listinfo/juju-dev >> > >> >> -- >> Juju-dev mailing list >> Juju-dev@lists.ubuntu.com >> Modify settings or unsubscribe at: >> https://lists.ubuntu.com/mailman/listinfo/juju-dev > > > > > -- > gustavo @ http://niemeyer.net -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
