A cautionary tale - mgo asserts

[David Cheney]

You had me at "ruins mongodb", actually just "ruins'.

On Wed, Jun 8, 2016 at 9:04 PM, roger peppe <roger.peppe at canonical.com> wrote:
> This is also relevant (but probably only for larger documents):
>
> https://jeremywsherman.com/blog/2013/04/23/key-reordering-ruins-mongodb/
>
> Another reason to avoid entire-subdocument matches.
>
> On 8 June 2016 at 10:42, Menno Smits <menno.smits at canonical.com> wrote:
>>
>>
>> On 8 June 2016 at 21:05, Tim Penhey <tim.penhey at 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 at lists.ubuntu.com
>> Modify settings or unsubscribe at:
>> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>>
>
> --
> Juju-dev mailing list
> Juju-dev at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev