[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