Re: [openstack-dev] [Nova] Grant FFE to "Host-state level locking" BP

2016-03-04 Thread Nikola Đipanov 
On 03/04/2016 02:06 PM, John Garbutt wrote:
> tl;dr
> As on IRC, I don't think this should get an FFE this cycle.
> 
> On 4 March 2016 at 10:56, Nikola Đipanov  wrote:
>> Hi,
>>
>> The actual BP that links to the approved spec is here: [1] and 2
>> outstanding patches are [2][3].
>>
>> Apart from the usual empathy-inspired reasons to allow this (code's been
>> up for a while, yet only had real review on the last day etc.) which are
>> not related to the technical merit of the work, there is also the fact
>> that two initial patches that add locking around updates of the
>> in-memory host map ([4] and [5]) have already been merged.
>>
>> They add the overhead of locking to the scheduler, but without the final
>> work they don't provide any benefits (races will not be detected,
>> without [2]).
> 
> We could land a patch to drop the synchronized decorators, but it
> seemed like it might still help (the possibly theoretical issue?) of
> two greenlets competing decrementing the same resource counts.
> 
>> I don't have any numbers on this but the result is likely that we made
>> things worse, for the sake of adhering to random and made-up dates.
> 
> For details on the reasons behind our process, please see:
> http://docs.openstack.org/developer/nova/process.html
> 
>> With
>> this in mind I think it only makes sense to do our best to merge the 2
>> outstanding patches.
> 
> Looking at the feature freeze exception criteria:
> https://wiki.openstack.org/wiki/FeatureFreeze
> 
> The code is not ready to merge right now, so its hard to asses the
> risk of merging it, and hard to asses how long it will take to merge.
> It seems medium-ish risk, given the existing patches.
> 
> We have had 2 FFEs, just for things that were +Wed but merged when we
> cut mitaka-3. They are all merged now.
> 
> Time is much tighter this cycle than usual. We also seem to have less
> reviewers doing reviews than normal for this point in the cycle, and a
> much bigger backlog of bug fixes to review. We only have about 7 more
> working days between now and tagging RC1, at which point master opens
> for Newton, and these patches are free to merge again.
> 
> While this is useful, its not a regression. It would help us detect
> races in the scheduler sooner. It does not feel release critical.
> 

Thanks for the response John,

If we take "release critical" to mean "Nova not able to start VMs if we
don't have this", then no - it's not release critical.

But it does mean that people consuming releases will not get to use this
and consequently find and report bugs for another 6 months.

On a more personal note - this is the second thing that I was involved
with this cycle that got accepted, only to get half merged over a random
deadline. The other one being [1], which was just integration work that
would make a lot of other work that went in this cycle (in both Nova and
Neutron) usable. Again - the result is, we have the code in tree, but no
one can use it and test it.

Even if I try to keep my personal feelings out of this - I still feel
that this is a massive waste we are happy to accept for practically 0 gain.

N.

[1]
https://blueprints.launchpad.net/openstack/?searchtext=sriov-pf-passthrough-neutron-port

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [Nova] Grant FFE to "Host-state level locking" BP

2016-03-04 Thread John Garbutt 
tl;dr
As on IRC, I don't think this should get an FFE this cycle.

On 4 March 2016 at 10:56, Nikola Đipanov  wrote:
> Hi,
>
> The actual BP that links to the approved spec is here: [1] and 2
> outstanding patches are [2][3].
>
> Apart from the usual empathy-inspired reasons to allow this (code's been
> up for a while, yet only had real review on the last day etc.) which are
> not related to the technical merit of the work, there is also the fact
> that two initial patches that add locking around updates of the
> in-memory host map ([4] and [5]) have already been merged.
>
> They add the overhead of locking to the scheduler, but without the final
> work they don't provide any benefits (races will not be detected,
> without [2]).

We could land a patch to drop the synchronized decorators, but it
seemed like it might still help (the possibly theoretical issue?) of
two greenlets competing decrementing the same resource counts.

> I don't have any numbers on this but the result is likely that we made
> things worse, for the sake of adhering to random and made-up dates.

For details on the reasons behind our process, please see:
http://docs.openstack.org/developer/nova/process.html

>With
> this in mind I think it only makes sense to do our best to merge the 2
> outstanding patches.

Looking at the feature freeze exception criteria:
https://wiki.openstack.org/wiki/FeatureFreeze

The code is not ready to merge right now, so its hard to asses the
risk of merging it, and hard to asses how long it will take to merge.
It seems medium-ish risk, given the existing patches.

We have had 2 FFEs, just for things that were +Wed but merged when we
cut mitaka-3. They are all merged now.

Time is much tighter this cycle than usual. We also seem to have less
reviewers doing reviews than normal for this point in the cycle, and a
much bigger backlog of bug fixes to review. We only have about 7 more
working days between now and tagging RC1, at which point master opens
for Newton, and these patches are free to merge again.

While this is useful, its not a regression. It would help us detect
races in the scheduler sooner. It does not feel release critical.

As such, I don't think this it should get an exception. Lets keep
focus on the lower risk, high value bug fixes sitting in our review
backlog.

Thanks,
johnthetubaguy

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [Nova] Grant FFE to "Host-state level locking" BP

2016-03-04 Thread Cheng, Yingxin 
I'll make sure to deliver the patches if FFE is granted.

Regards,
-Yingxin

On Friday, March 4, 2016 6:56 PM Nikola Đipanov wrote:
> 
> Hi,
> 
> The actual BP that links to the approved spec is here: [1] and 2 outstanding
> patches are [2][3].
> 
> Apart from the usual empathy-inspired reasons to allow this (code's been up 
> for
> a while, yet only had real review on the last day etc.) which are not related 
> to
> the technical merit of the work, there is also the fact that two initial 
> patches
> that add locking around updates of the in-memory host map ([4] and [5]) have
> already been merged.
> 
> They add the overhead of locking to the scheduler, but without the final work
> they don't provide any benefits (races will not be detected, without [2]).
> 
> I don't have any numbers on this but the result is likely that we made things
> worse, for the sake of adhering to random and made-up dates. With this in mind
> I think it only makes sense to do our best to merge the 2 outstanding patches.
> 
> Cheers,
> N.
> 
> [1]
> https://blueprints.launchpad.net/openstack/?searchtext=host-state-level-
> locking
> [2] https://review.openstack.org/#/c/262938/
> [3] https://review.openstack.org/#/c/262939/
> 
> [4] https://review.openstack.org/#/c/259891/
> [5] https://review.openstack.org/#/c/259892/
> 

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

[openstack-dev] [Nova] Grant FFE to "Host-state level locking" BP

2016-03-04 Thread Nikola Đipanov 
Hi,

The actual BP that links to the approved spec is here: [1] and 2
outstanding patches are [2][3].

Apart from the usual empathy-inspired reasons to allow this (code's been
up for a while, yet only had real review on the last day etc.) which are
not related to the technical merit of the work, there is also the fact
that two initial patches that add locking around updates of the
in-memory host map ([4] and [5]) have already been merged.

They add the overhead of locking to the scheduler, but without the final
work they don't provide any benefits (races will not be detected,
without [2]).

I don't have any numbers on this but the result is likely that we made
things worse, for the sake of adhering to random and made-up dates. With
this in mind I think it only makes sense to do our best to merge the 2
outstanding patches.

Cheers,
N.

[1]
https://blueprints.launchpad.net/openstack/?searchtext=host-state-level-locking
[2] https://review.openstack.org/#/c/262938/
[3] https://review.openstack.org/#/c/262939/

[4] https://review.openstack.org/#/c/259891/
[5] https://review.openstack.org/#/c/259892/

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev