On 02/02/2012 12:11 AM, Jan Provaznik wrote:
On 02/02/2012 05:26 AM, Richard Su wrote:
> On 01/31/2012 08:08 AM, Tomas Sedovic wrote:
>> On 01/30/2012 01:53 PM, Tomáš Hrčka wrote:
>>> ---
>>> src/app/models/instance_observer.rb | 20 ++++++++++++++------
>>> 1 files changed, 14 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/src/app/models/instance_observer.rb
>>> b/src/app/models/instance_observer.rb
>>> index 9d13fb0..06ba2ef 100644
>>> --- a/src/app/models/instance_observer.rb
>>> +++ b/src/app/models/instance_observer.rb
>>> @@ -65,13 +65,21 @@ class InstanceObserver< ActiveRecord::Observer
>>> elsif RUNNING_STATES.include?(state_from)&&
>>> !RUNNING_STATES.include?(state_to)
>>> quota.running_instances -= 1
>>> end
>>> -
>>> - if !ACTIVE_STATES.include?(state_from)&&
>>> ACTIVE_STATES.include?(state_to)
>>> - quota.total_instances += 1
>>> - elsif ACTIVE_STATES.include?(state_from)&&
>>> !ACTIVE_STATES.include?(state_to)
>>> - quota.total_instances -= 1
>>> - end
>>> - quota.save!
>>> + quota.transaction do
>>> + quota.lock!
>>> + if !RUNNING_STATES.include?(state_from)&&
>>> RUNNING_STATES.include?(state_to)
>>> + quota.running_instances += 1
>>> + elsif RUNNING_STATES.include?(state_from)&&
>>> !RUNNING_STATES.include?(state_to)
>>> + quota.running_instances -= 1
>>> + end
>>> +
>>> + if !ACTIVE_STATES.include?(state_from)&&
>>> ACTIVE_STATES.include?(state_to)
>>> + quota.total_instances += 1
>>> + elsif ACTIVE_STATES.include?(state_from)&&
>>> !ACTIVE_STATES.include?(state_to)
>>> + quota.total_instances -= 1
>>> + end
>>> + quota.save!
>>> + end
>>> end
>>> end
>>> end
>>
>> I'm willing to ACK this as the code seems to be correct to me.
>>
>> However, I'm having trouble reproducing the original issue. If it
>> fixes it for you, go ahead and push this. We'll see what the person
>> who reported the bug has to say about it.
>>
>> Or if there's anyone else who can repro the bug, do feel free to try
>> it out.
>
> Ack. I'm also not able to reproduce the original issue. Launches work
> with this patch, I've tried 50 mock instances in a deployment.
>
> I'd say push it and let's get more feedback from the performance testing
> lab.
>
>
Reproduction nit:
What happens here is that both dbomatic and conductor touches quota in
the same time, it's probably easier to reproduce on RHEVM because
start process is slower than for example for mock, so there is higher
chance that a user hits dbomatic 60s refresh cycle.
Because dbomatic forks separate processes for each account, same
conflict can be achieved on mock driver by defining 2-3 providers,
then launch big defployment on each of them at the same time (or try
to stop them). At least this worked for me.
Jan
Pushed. Dave Wilson was able to reproduce it in the lab and this patch
fixes the problem.