commit cedcef7ecb9673530ca0241c2976dfd862c4f75a
Author: Chris Dent <email address hidden>
Date: Wed Dec 20 18:35:53 2017 +0000
Do not set allocation.id in AllocationList.create_all()
The _set_allocations method used by AllocationList.create_all is
side-effecty: it sets the 'id' attribute on the list of Allocation
objects that is passed to it.
At the start of the method the incoming Allocation objects are
checked to see if they have already been created, by checking
for an 'id' field.
Meanwhile, _set_allocations is also configured to retry on db
deadlock. The deadlock can happen for a variety of reasons within
the transaction. The original theory, discussed in the original
fix, I2c276dc0125b5b9f7a54a1cd431b1b2f5239e93a, is that it is
during resource provider generation checks. In the associated bug it
looks like it may happen sometimes while inserting allocations.
In either case, if we have gone through the 'for alloc in allocs'
loop at least once, the contents of the 'allocs' list has been
modified to have at least one of the alloc.id fields set. Upon
retry, the 'id' field check at the start of the method will fail,
leading to an ObjectActionError and an eventual 500 at the placement
API level.
This change takes the simplest approach and simply removes the setting
of the 'id' attribute on the allocations in the 'allocs' list. There are
other ways to deal with this, this is the least intrusive. It works
because:
* create_all is only called from the allocation handler in placement,
and the objects are not used (the response bodies are empty)
* other than the 'id' change, the alloc members in the allocs list
are otherwise unchanged
* this kind of side-effecty business is dangerous, so let's not
rely on it
Tests which were relying on the side-effecty business have been adjusted
accordingly.
NOTE(mriedem): The resource_provider.py conflict is due to not
having 31751a7149a3bb4b51087aecdc3b14c061c60d9b in Pike. The
test_resource_provider.py conflict is due to not having
5a1ef8fa862321e50a9063f22b27dd5fd93446e7 in Pike. Furthermore,
the functional test had to be adjusted because of not having
db4d6799f1a220f69aa804754a0913065ff9436a in Pike.
Change-Id: I3c7aea7d8959a20c3c404bc6616b47336ff40b67
Closes-Bug: #1739453
(cherry picked from commit 3491f3d6f2334b80c364deeb21d65004262c2846)
Reviewed: https:/ /review. openstack. org/530794 /git.openstack. org/cgit/ openstack/ nova/commit/ ?id=cedcef7ecb9 673530ca0241c29 76dfd862c4f75a
Committed: https:/
Submitter: Zuul
Branch: stable/pike
commit cedcef7ecb96735 30ca0241c2976df d862c4f75a
Author: Chris Dent <email address hidden>
Date: Wed Dec 20 18:35:53 2017 +0000
Do not set allocation.id in AllocationList. create_ all()
The _set_allocations method used by AllocationList. create_ all is
side-effecty: it sets the 'id' attribute on the list of Allocation
objects that is passed to it.
At the start of the method the incoming Allocation objects are
checked to see if they have already been created, by checking
for an 'id' field.
Meanwhile, _set_allocations is also configured to retry on db 9f7a54a1cd431b1 b2f5239e93a, is that it is
deadlock. The deadlock can happen for a variety of reasons within
the transaction. The original theory, discussed in the original
fix, I2c276dc0125b5b
during resource provider generation checks. In the associated bug it
looks like it may happen sometimes while inserting allocations.
In either case, if we have gone through the 'for alloc in allocs'
loop at least once, the contents of the 'allocs' list has been
modified to have at least one of the alloc.id fields set. Upon
retry, the 'id' field check at the start of the method will fail,
leading to an ObjectActionError and an eventual 500 at the placement
API level.
This change takes the simplest approach and simply removes the setting
of the 'id' attribute on the allocations in the 'allocs' list. There are
other ways to deal with this, this is the least intrusive. It works
because:
* create_all is only called from the allocation handler in placement,
and the objects are not used (the response bodies are empty)
* other than the 'id' change, the alloc members in the allocs list
are otherwise unchanged
* this kind of side-effecty business is dangerous, so let's not
rely on it
Tests which were relying on the side-effecty business have been adjusted
accordingly.
Conflicts:
nova/ objects/ resource_ provider. py
nova/ tests/unit/ objects/ test_resource_ provider. py
NOTE(mriedem): The resource_ provider. py conflict is due to not b51087aecdc3b14 c061c60d9b in Pike. The resource_ provider. py conflict is due to not having 321e50a9063f22b 27dd5fd93446e7 in Pike. Furthermore, 220f69aa804754a 0913065ff9436a in Pike.
having 31751a7149a3bb4
test_
5a1ef8fa862
the functional test had to be adjusted because of not having
db4d6799f1a
Change-Id: I3c7aea7d8959a2 0c3c404bc6616b4 7336ff40b67 0c364deeb21d650 04262c2846)
Closes-Bug: #1739453
(cherry picked from commit 3491f3d6f2334b8