Open Stack

On Thu, 2014-07-24 at 14:04 +0800, Chen CH Ji wrote:
> According to bug [1], there are some possibilities that concurrent
> operations on keypair/security rules can exceed quota
> Found that we have 3 kinds of resources in quotas.py:
>  ReservableResource/AbsoluteResource/CountableResource
> 
> curious about CountableResource because it's can't be thread safe due
> to its logic:
> 
> count = QUOTAS.count(context, 'security_group_rules', id)
> try:
>             projected = count + len(vals)
>             QUOTAS.limit_check(context,
> security_group_rules=projected)
> 
> was it designed by purpose to be different to ReservableResource? If
> set it to ReservableResource just like RAM/CPU, what kind of side
> effect it might lead to ?
> 
> Also, is it possible to consider a solution like 'hold a write lock in
> db layer, check the count of resource and raise exception if it exceed
> quota'? 

First, some background: the difference between a ReservableResource and
a CountableResource comes down to the method used to count the in-use
resources.  For ReservableResource, the count is based on the number of
database objects matching the process ID, whereas with
CountableResource, a counting function has to be specified; otherwise,
the classes are identical and designed to be used identically—indeed,
CountableResource extends ReservableResource and only overrides the
constructor.  The only two CountableResource quota objects declared in
Nova are security_group_rules, where the counting function counts the
rules per group; and key_pairs, where the counting function counts the
key pairs per user.

Now, the code you paste in your email is the wrong code to use with a
ReservableResource or CountableResource; indeed, the limit_check()
docstring indicates that it's for those quotas for which there is no
usage synchronization function…meaning an AbsoluteResource.  For
ReservableResource or CountableResource, the code should be using the
reserve() method.  The reserve() method performs its checks safely, and
creates a reservation to prevent cross-process allocation that would
result in quota exceeded.  The downside is that reserve() must create a
reservation that must later be committed or rolled back, which is
probably why that code is using the limit_check() inappropriately.

It may be worthwhile to add some sanity-checking to limit_check() and
reserve() that ensure that they only work with the correct resource
type(s), in order to prevent this sort of problem from occurring in the
future.
-- 
Kevin L. Mitchell <kevin.mitchell at rackspace.com>
Rackspace