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
