Re: Tests do not pass with go1.7
Given the error message, I think this is our "prevent us from contacting real websites in the test run" failing. This is one of those "global state is bad" cases because we had to patch global state during the run of the tests but prob 1.7 changes what global state we need to touch. John =:-> On Jun 9, 2016 6:34 AM, "Nate Finch"wrote: > Just FYI, in case anyone was like me and decided they wanted to jump on > the faster compile times in 1.7... some of our tests do not pass in go 1.7: > > https://bugs.launchpad.net/juju-core/+bug/1589350 > > -- > 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
Tests do not pass with go1.7
Just FYI, in case anyone was like me and decided they wanted to jump on the faster compile times in 1.7... some of our tests do not pass in go 1.7: https://bugs.launchpad.net/juju-core/+bug/1589350 -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: A cautionary tale - mgo asserts
On 8 June 2016 at 22:36, John Meinelwrote: > ... >> > > >> >> ops := []txn.Op{{ >> C: "collection", >> Id: ..., >> Assert: bson.M{ >> "some-field.A": "foo", >> "some-field.B": 99, >> }, >> Update: ... >> } >> >> ... >> >> > If loading into a bson.M is the problem, wouldn't using a bson.M to start > with also be a problem? > No this is fine. The assert above defines that each field should match the values given. Each field is checked separately - order doesn't matter. This would be a problem though: ops := []txn.Op{{ C: "collection", Id: ..., Assert: bson.M{"some-field": bson.M{ "A": "foo", "B": 99, }, Update: ... } > In this case, mgo is being asked to assert that some-field is an embedded document equal to a document defined by the bson.M{"A": "foo", "B": 99} map. This is what's happening now when you provide a struct value to compare against a field because the struct gets round-tripped through bson.M. That bson.M eventually gets converts to actual bson and sent to mongodb but you have no control of the field ordering that will ultimately be used. - Menno -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: A cautionary tale - mgo asserts
On 9 June 2016 at 03:44, Gustavo Niemeyerwrote: > 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 unmarshalling uses bson.M for these. This means when a transaction is loaded from the txns collection the contents of these fields are loaded into bson.M and field ordering is lost. It looks trivial to change the bson unmarshalling code to default to bson.D but naively changing this will likely break existing users of the bson package. That's probably not the right solution here. Perhaps transactions which are written to/loaded from the database by mgo/txn should use a private txn.Op analogue where Assert, Insert and Update are bson.D instead of interface{}? - Menno -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: The CI build time continue to rise alarmingly
Hi Torsten - That fix shouldn't have increased build times unless we also changed the test run to be against Mongo 3.2. If builds are still against 2.4 then the change will have made them slightly faster (because we only drop and recreate the database at the suite level). I don't think we've switched runs to be against 3.2 because there were other problems that were unrelated to the slow state tests - these two that I know about: https://bugs.launchpad.net/juju-core/+bug/1588784 https://bugs.launchpad.net/juju-core/+bug/1573286 Cheers, Christian On Wed, 8 Jun 2016, 16:06 Torsten Baumann,wrote: > Hi David, > > Thanks for raising the inefficiency. > > From what I understand there was a change introduced in and around June > 1st for https://bugs.launchpad.net/juju-core/+bug/1573294 that may have > increased the time again. :-( As regrettable as this is we did review this > with the tech board and it was accepted as part of the fix. > > > I discussed with a few people and there is some time we can shave off on > the tarball assembly (2-4 minutes) if we spent a few days work there. > > I also believe we can save time if we ran the tests in LXC containers but > there may be some reliability issues there. we can always switch the jobs > to use lxc and see what happens? > > Other than that I am open to seeing who else on this list has ideas as to > what we can do to reduce the time? I would rather we go after the most > important ones first if we can identify them. > > thanks everyone, > > Torsten > > > > -- Forwarded message -- > From: David Cheney > Date: Thu, Jun 2, 2016 at 9:49 PM > Subject: The CI build time continue to rise alarmingly > To: "juju-dev@lists.ubuntu.com" > > > CI build times are now an average of 36 minutes. That means in a > typical 8 hour work day, assuming doing nothing other than landing > branches, less than 16 commits can be landed. > > While bugs can be worked on and reviewed in parallel, landing is a > sequential action that blocks everyone, and given the landing bot is > batting less than 0.500, this limits the practical number of changes > that can be landed in a day, a sprint, a iteration, or a development > cycle. > > I cannot make it any clearer than this, the speed of CI limits the > velocity of this team. > > Dave > > -- > 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 > -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: A cautionary tale - mgo asserts
On 8 June 2016 at 16:44, Gustavo Niemeyerwrote: > 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 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 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 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 wrote: >> >> > >> >> > >> >> > 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 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 >>
Re: A cautionary tale - mgo asserts
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.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 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 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 wrote: > >> > > >> > > >> > 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 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
Re: A cautionary tale - mgo asserts
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 Niemeyerwrote: > 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 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 wrote: >> > >> > >> > 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 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
Re: The CI build time continue to rise alarmingly
Hi David, Thanks for raising the inefficiency. >From what I understand there was a change introduced in and around June 1st for https://bugs.launchpad.net/juju-core/+bug/1573294 that may have increased the time again. :-( As regrettable as this is we did review this with the tech board and it was accepted as part of the fix. I discussed with a few people and there is some time we can shave off on the tarball assembly (2-4 minutes) if we spent a few days work there. I also believe we can save time if we ran the tests in LXC containers but there may be some reliability issues there. we can always switch the jobs to use lxc and see what happens? Other than that I am open to seeing who else on this list has ideas as to what we can do to reduce the time? I would rather we go after the most important ones first if we can identify them. thanks everyone, Torsten -- Forwarded message -- From: David CheneyDate: Thu, Jun 2, 2016 at 9:49 PM Subject: The CI build time continue to rise alarmingly To: "juju-dev@lists.ubuntu.com" CI build times are now an average of 36 minutes. That means in a typical 8 hour work day, assuming doing nothing other than landing branches, less than 16 commits can be landed. While bugs can be worked on and reviewed in parallel, landing is a sequential action that blocks everyone, and given the landing bot is batting less than 0.500, this limits the practical number of changes that can be landed in a day, a sprint, a iteration, or a development cycle. I cannot make it any clearer than this, the speed of CI limits the velocity of this team. Dave -- 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
Re: A cautionary tale - mgo asserts
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 peppewrote: > 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 wrote: > > > > > > 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 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
Re: Regarding Synnefo environment provider for Juju
Thodoris, Also feel free to join #juju-dev on freenode (IRC) for dynamic assistance. You can reach out to me directly (alexisb) and I will ensure someone provides some help. We look forward to working with you on getting a Juju provider for Synnefo! Alexis On Wed, Jun 8, 2016 at 4:28 AM, Thodoris Sotiropoulos < theo...@admin.grnet.gr> wrote: > Hi all, > > You may remember previous e-mails sent by my partner Stavros Sachtouris > regarding > the case of implementing a Juju environment provider for our open source > IaaS called > Synnefo. > > We have started implementation of the basics (configuration schema, > instance creation, > instance queries, preparation of environment, etc). Our goal is to make a > proof > of concept implementation of the bootstrap command and that's why we have > ignored > networking and storage configuration, a.k.a mocked. > > So far, we have managed to create and communicate with a machine instance. > However, > during the last step of bootstrap process (insertion of the admin model to > database) > we are facing an unexpected problem (method `NewModel` of > `state/model.go`). > > Here is the corresponding log file: > https://pithos.okeanos.grnet.gr/public/8EHM5jpEm2W7bSwly9wFG > > I tried to investigate the problem but I cannot figure out why I get the > `model %s for %q already exists`. What am I missing? I would appreciate any > help or guidance.» > > Thank you in advance > > Thodoris Sotiropoulos > Developer @ GRNET > theo...@grnet.gr > > -- > Juju-dev mailing list > Juju-dev@lists.ubuntu.com > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/juju-dev > -- Alexis Bruemmer Juju Core Manager, Canonical Ltd. (503) 686-5018 alexis.bruem...@canonical.com -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Regarding Synnefo environment provider for Juju
Hey Thodoris, Congratulations on beginning your provider! :) Do you have your code visible anywhere? It would help with diagnosing any issues. - Katherine Thodoris Sotiropouloswrites: > Hi all, > > You may remember previous e-mails sent by my partner Stavros > Sachtouris regarding the case of implementing a Juju environment > provider for our open source IaaS called Synnefo. > > We have started implementation of the basics (configuration schema, > instance creation, instance queries, preparation of environment, > etc). Our goal is to make a proof of concept implementation of the > bootstrap command and that's why we have ignored networking and > storage configuration, a.k.a mocked. > > So far, we have managed to create and communicate with a machine > instance. However, during the last step of bootstrap process > (insertion of the admin model to database) we are facing an unexpected > problem (method `NewModel` of `state/model.go`). > > Here is the corresponding log file: > https://pithos.okeanos.grnet.gr/public/8EHM5jpEm2W7bSwly9wFG > > I tried to investigate the problem but I cannot figure out why I get > the `model %s for %q already exists`. What am I missing? I would > appreciate any help or guidance.» > > Thank you in advance > > Thodoris Sotiropoulos Developer @ GRNET theo...@grnet.gr -- Katherine -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Regarding Synnefo environment provider for Juju
Hi all, You may remember previous e-mails sent by my partner Stavros Sachtouris regarding the case of implementing a Juju environment provider for our open source IaaS called Synnefo. We have started implementation of the basics (configuration schema, instance creation, instance queries, preparation of environment, etc). Our goal is to make a proof of concept implementation of the bootstrap command and that's why we have ignored networking and storage configuration, a.k.a mocked. So far, we have managed to create and communicate with a machine instance. However, during the last step of bootstrap process (insertion of the admin model to database) we are facing an unexpected problem (method `NewModel` of `state/model.go`). Here is the corresponding log file: https://pithos.okeanos.grnet.gr/public/8EHM5jpEm2W7bSwly9wFG I tried to investigate the problem but I cannot figure out why I get the `model %s for %q already exists`. What am I missing? I would appreciate any help or guidance.» Thank you in advance Thodoris Sotiropoulos Developer @ GRNET theo...@grnet.gr -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: A cautionary tale - mgo asserts
You had me at "ruins mongodb", actually just "ruins'. On Wed, Jun 8, 2016 at 9:04 PM, roger peppewrote: > 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 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 -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: A cautionary tale - mgo asserts
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 Smitswrote: > > > 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 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
Re: A cautionary tale - mgo asserts
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 Smitswrote: > > > 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 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
Re: A cautionary tale - mgo asserts
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 Smitswrote: > > > 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 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
Re: A cautionary tale - mgo asserts
> > ... > > > 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 > 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. > > If loading into a bson.M is the problem, wouldn't using a bson.M to start with also be a problem? John =:-> -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: A cautionary tale - mgo asserts
On 8 June 2016 at 21:05, Tim Penheywrote: > 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
Re: A cautionary tale - mgo asserts
On 8 June 2016 at 10:05, Tim Penheywrote: > 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 reproduce. > > Nate spent last week on it, and I pulled Menno in for his mgo expertise > yesterday and today because I was getting stumped. > > Here is what was happening: > > The machine document was being read, and we were wanting to update the > preferred public and private addresses. An assert was added to make sure > that the value was either what we expected it to be (from reading the doc), > or unset. Now, really, it wouldn't be unset, but that is a different story. > > The problem is that the assert was failing. Logging was added to check what > the db had, and what the asserts were transformed to with the multi model > layer. All the asserts matched. The data was there, everything was good, > except it wasn't. > > This morning, I pulled the data out with a bson.D, and it became readily > apparent that the address structure ordering was different in the times it > failed. Now normally we are guaranteed ordering. When the struct is > marshalled to bson, it is as a bson.D with the order defined in by the order > of the members. However when using the struct in the assert, it was > matching different orders, so the assert failed. This is interesting and not something I was aware of. I had been under the impression that bson.D could almost always be used interchangeably with bson.M or structs and that key order wasn't relevant when comparing objects. Could you provide a simple example (preferably not involving mgo/txn) where the results are different depending on member ordering? cheers, rog. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: A cautionary tale - mgo asserts
On 8 June 2016 at 10:05, Tim Penheywrote: > 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 reproduce. > > Nate spent last week on it, and I pulled Menno in for his mgo expertise > yesterday and today because I was getting stumped. > > Here is what was happening: > > The machine document was being read, and we were wanting to update the > preferred public and private addresses. An assert was added to make sure > that the value was either what we expected it to be (from reading the doc), > or unset. Now, really, it wouldn't be unset, but that is a different story. > > The problem is that the assert was failing. Logging was added to check what > the db had, and what the asserts were transformed to with the multi model > layer. All the asserts matched. The data was there, everything was good, > except it wasn't. > > This morning, I pulled the data out with a bson.D, and it became readily > apparent that the address structure ordering was different in the times it > failed. Now normally we are guaranteed ordering. When the struct is > marshalled to bson, it is as a bson.D with the order defined in by the order > of the members. However when using the struct in the assert, it was > matching different orders, so the assert failed. This is interesting and not something I was aware of. I had been under the impression that bson.D could almost always be used interchangeably with bson.M or structs and that key order wasn't relevant when comparing objects. Could you provide a simple example (preferably not involving mgo/txn) where the results are different depending on member ordering? cheers, rog. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev