Re: A cautionary tale - mgo asserts

2016-06-09 Thread roger peppe
On 9 June 2016 at 10:15, James Tunnicliffe wrote: > OK, how about this as an argument (that I am not necessarily saying is > right, but we want a well justified change): Mongo changed in the > past. Ordering isn't important to us, only key: value pairs. Given > these, we can happily say that being

Re: A cautionary tale - mgo asserts

2016-06-09 Thread James Tunnicliffe
OK, how about this as an argument (that I am not necessarily saying is right, but we want a well justified change): Mongo changed in the past. Ordering isn't important to us, only key: value pairs. Given these, we can happily say that being explicitly unordered on the Juju level or even the mgo/txn

Re: A cautionary tale - mgo asserts

2016-06-09 Thread roger peppe
In fact it seems that MongoDB (as of version 2.6) *does* make some guarantees about field order: >From >https://docs.mongodb.com/master/release-notes/2.6/#insert-and-update-improvements: : MongoDB preserves the order of the document fields following write operations except for the following case

Re: A cautionary tale - mgo asserts

2016-06-09 Thread James Tunnicliffe
Surely we want to remove any ordering from the txn logic if Mongo makes no guarantees about keeping ordering? Being explicitly unordered at both ends seems right. James On Thu, Jun 9, 2016 at 8:35 AM, roger peppe wrote: > On 9 June 2016 at 01:20, Menno Smits wrote: >> On 8 June 2016 at 22:36, J

Re: A cautionary tale - mgo asserts

2016-06-09 Thread roger peppe
On 9 June 2016 at 01:20, Menno Smits wrote: > On 8 June 2016 at 22:36, John Meinel wrote: >>> >>> ... >> >> >>> >>> >>> ops := []txn.Op{{ >>> C: "collection", >>> Id: ..., >>> Assert: bson.M{ >>> "some-field.A": "foo", >>> "some-field.B": 99, >>> }, >

Re: A cautionary tale - mgo asserts

2016-06-08 Thread John Meinel
tl dr; I'm in favor of fixing mgo but I think we should still avoid them. Note the link that Roger mentioned: https://jeremywsherman.com/blog/2013/04/23/key-reordering-ruins-mongodb/ It appears that mongo itself does 2 things that do *not* play nicely together, regardless of what we do: 1.

Re: A cautionary tale - mgo asserts

2016-06-08 Thread Menno Smits
On 8 June 2016 at 22:36, John Meinel wrote: > ... >> > > >> >> ops := []txn.Op{{ >> C: "collection", >> Id: ..., >> Assert: bson.M{ >> "some-field.A": "foo", >> "some-field.B": 99, >> }, >> Update: ... >> } >> >> ... >> >> > If loading into a

Re: A cautionary tale - mgo asserts

2016-06-08 Thread Menno Smits
On 9 June 2016 at 03:44, Gustavo Niemeyer wrote: > Is it mgo/txn that is internally unmarahalling onto that? > > Let's get that fixed at its heart. > That would be ideal. The root of the problem is that the Assert, Insert and Update fields of txn.Op are of type interface{} and the bson unmarshal

Re: A cautionary tale - mgo asserts

2016-06-08 Thread roger peppe
On 8 June 2016 at 16:44, Gustavo Niemeyer wrote: > Is it mgo/txn that is internally unmarahalling onto that? > > Let's get that fixed at its heart. Yes, good plan. > On Jun 8, 2016 12:27 PM, "roger peppe" wrote: >> >> The Assert field in mgo/txn.Op is an interface{}, so >> when it's marshaled a

Re: A cautionary tale - mgo asserts

2016-06-08 Thread Gustavo Niemeyer
Is it mgo/txn that is internally unmarahalling onto that? Let's get that fixed at its heart. On Jun 8, 2016 12:27 PM, "roger peppe" wrote: > 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.

Re: A cautionary tale - mgo asserts

2016-06-08 Thread roger peppe
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 wrote: > Is it mgo itself tha

Re: A cautionary tale - mgo asserts

2016-06-08 Thread Gustavo Niemeyer
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 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 mo

Re: A cautionary tale - mgo asserts

2016-06-08 Thread David Cheney
You had me at "ruins mongodb", actually just "ruins'. On Wed, Jun 8, 2016 at 9:04 PM, roger peppe 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 ma

Re: A cautionary tale - mgo asserts

2016-06-08 Thread roger peppe
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 wrote: > > > On 8 June 2016 at 21:05, Tim Penhey wrote: >> >> Hi fol

Re: A cautionary tale - mgo asserts

2016-06-08 Thread roger peppe
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.f

Re: A cautionary tale - mgo asserts

2016-06-08 Thread roger peppe
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.f

Re: A cautionary tale - mgo asserts

2016-06-08 Thread John Meinel
> > ... > > > ops := []txn.Op{{ > C: "collection", > Id: ..., > Assert: bson.M{ > "some-field.A": "foo", > "some-field.B": 99, > }, > Update: ... > } > > ... > > > [1] If transaction operations are loaded and used from the DB (more likely > u

Re: A cautionary tale - mgo asserts

2016-06-08 Thread Menno Smits
On 8 June 2016 at 21:05, Tim Penhey 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

Re: A cautionary tale - mgo asserts

2016-06-08 Thread roger peppe
On 8 June 2016 at 10:05, Tim Penhey wrote: > Hi folks, > > tl;dr: not use structs in transaction asserts > > I have spent the last two days looking at this bug: > https://bugs.launchpad.net/juju-core/+bug/1537585 > > It was one where the instance poller got itself in a knot, and couldn't get > out

Re: A cautionary tale - mgo asserts

2016-06-08 Thread roger peppe
On 8 June 2016 at 10:05, Tim Penhey wrote: > Hi folks, > > tl;dr: not use structs in transaction asserts > > I have spent the last two days looking at this bug: > https://bugs.launchpad.net/juju-core/+bug/1537585 > > It was one where the instance poller got itself in a knot, and couldn't get > out

A cautionary tale - mgo asserts

2016-06-08 Thread Tim Penhey
Hi folks, tl;dr: not use structs in transaction asserts I have spent the last two days looking at this bug: https://bugs.launchpad.net/juju-core/+bug/1537585 It was one where the instance poller got itself in a knot, and couldn't get out. Transaction asserts were failing, and often hard to rep