[PATCH conductor] BZ#783218 added pessimistic database locking for quota to prevent updating with old values

Richard Su rwsu at redhat.com
Thu Feb 2 04:26:39 UTC 2012


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.





More information about the aeolus-devel mailing list