Re: Tests do not pass with go1.7

2016-06-08 Thread John Meinel
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

2016-06-08 Thread Nate Finch
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

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 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

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 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

2016-06-08 Thread Christian Muirhead
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

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 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

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.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

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 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

2016-06-08 Thread Torsten Baumann
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


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 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

2016-06-08 Thread Alexis Bruemmer
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

2016-06-08 Thread Katherine Cox-Buday

Hey Thodoris,

Congratulations on beginning your provider! :)

Do you have your code visible anywhere? It would help with diagnosing any 
issues.

-
Katherine

Thodoris Sotiropoulos  writes:

> 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

2016-06-08 Thread Thodoris Sotiropoulos

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

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 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

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 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

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.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

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.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

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
> 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

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 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

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. 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

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. 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