)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c094d132be70b2b9c4e8dfe302e9fe1c050ea554","unresolved":false,"context_lines":[{"line_number":16,"context_line":"* all numeric fields need to be NonNegative"},{"line_number":17,"context_line":"* all numeric fields, save \u0027total\u0027, need a default"},{"line_number":18,"context_line":"* a get_by_resource_provider_and_class methods needs to exist, to"},{"line_number":19,"context_line":"  load one single inventory for inspection and update"},{"line_number":20,"context_line":"* check allocations in save() for conflicts"},{"line_number":21,"context_line":"* move tests which use the db to functional and increase non-db"},{"line_number":22,"context_line":"  cover"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"9abb7d3a_eabb4be3","line":19,"updated":"2016-05-31 18:31:50.000000000","message":"I disagree here. The unit of work needs to be a transactional update of multiple inventory records at once using the resource_providers.generation field as a marker field for concurrent writes. Therefore this needs to be done on the InventoryList object I believe. More inline below.","commit_id":"2ae640b22cae79f03b0d6de5855fee94091d8a6c"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"b9468bf36c2b1c9d761fa4dd26f347f36706c743","unresolved":false,"context_lines":[{"line_number":16,"context_line":"* all numeric fields need to be NonNegative"},{"line_number":17,"context_line":"* all numeric fields, save \u0027total\u0027, need a default"},{"line_number":18,"context_line":"* a get_by_resource_provider_and_class methods needs to exist, to"},{"line_number":19,"context_line":"  load one single inventory for inspection and update"},{"line_number":20,"context_line":"* check allocations in save() for conflicts"},{"line_number":21,"context_line":"* move tests which use the db to functional and increase non-db"},{"line_number":22,"context_line":"  cover"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"9abb7d3a_474d2d3d","line":19,"in_reply_to":"9abb7d3a_eabb4be3","updated":"2016-05-31 19:52:55.000000000","message":"Gonna respond here, rather than on the code as this is a shorter summary.\n\nYour statement is probably central to the confusion I\u0027ve had throughout the process, and probably part of the reason why I was trying to make the rest API have only resource pool resources (because when I did, before we changed it,  I had inventory updates in a single transaction (they didn\u0027t however, use the generation)).\n\nSome questions:\n\n* If any resource pool is only going to have one inventory for any resource class, why does updating the inventory for all the classes of inventory need to be transactional?\n* Similarly, if the REST API has been designed such that it is possible to update just one Inventory of a resource pool, why is it not appropriate on the db side, to just update that one Inventory?\n* It does, however, make sense that when I write a chunk of allocations (across different resource classes) that they need to be in a transaction: if any of them burst capacity * allocation_ratio, fail, rollback.","commit_id":"2ae640b22cae79f03b0d6de5855fee94091d8a6c"}],"nova/db/sqlalchemy/models.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c094d132be70b2b9c4e8dfe302e9fe1c050ea554","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"9abb7d3a_cac00753","updated":"2016-05-31 18:31:50.000000000","message":"We need to move inventories table to the API database, right?","commit_id":"2ae640b22cae79f03b0d6de5855fee94091d8a6c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"286c91df137f67fb1c48034048d15e01999e8f51","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"9abb7d3a_1a01b838","in_reply_to":"9abb7d3a_cac00753","updated":"2016-05-31 20:32:40.000000000","message":"Already done. These models are orphans and need to be removed now.","commit_id":"2ae640b22cae79f03b0d6de5855fee94091d8a6c"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c094d132be70b2b9c4e8dfe302e9fe1c050ea554","unresolved":false,"context_lines":[{"line_number":1495,"context_line":"    resource_provider \u003d orm.relationship("},{"line_number":1496,"context_line":"        \"ResourceProvider\","},{"line_number":1497,"context_line":"        primaryjoin\u003d(\u0027and_(Inventory.resource_provider_id \u003d\u003d \u0027"},{"line_number":1498,"context_line":"                     \u0027ResourceProvider.id)\u0027),"},{"line_number":1499,"context_line":"        foreign_keys\u003dresource_provider_id,"},{"line_number":1500,"context_line":"        lazy\u003d\u0027immediate\u0027"},{"line_number":1501,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":10,"id":"9abb7d3a_2ab27306","line":1498,"updated":"2016-05-31 18:31:50.000000000","message":"Note that the above is incorrect (though I believe SQLAlchemy is smart enough to ignore the mistake). It should not include the and_().","commit_id":"2ae640b22cae79f03b0d6de5855fee94091d8a6c"}],"nova/objects/resource_provider.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c094d132be70b2b9c4e8dfe302e9fe1c050ea554","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        db_inventory_list \u003d cls._get_all_by_resource_provider(context,"},{"line_number":304,"context_line":"                                                              rp_uuid)"},{"line_number":305,"context_line":"        return base.obj_make_list(context, cls(context), objects.Inventory,"},{"line_number":306,"context_line":"                                  db_inventory_list)"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":10,"id":"9abb7d3a_8aec3ff4","line":306,"updated":"2016-05-31 18:31:50.000000000","message":"So, here is where I think we screwed things up in these object models... perhaps irrevocably. We need to treat any changes to the inventory records of a particular resource provider in an atomic and consistent fashion. We currently don\u0027t do that because we call Inventory.save() which is a remotable classmethod and by its very nature isn\u0027t transactional across multiple RPC calls.\n\nWhat is really needed here is something like this:\n\n class InventoryList(base.ObjectListBase, base.NovaObject)\n ...\n     @base.remotable_classmethod\n     def update_inventory_list(cls, inv_list, res_prov):\n         \"\"\"Updates the supplied resource provider\u0027s inventory in a\n         consistent fashion using the supplied resource provider\u0027s\n         generation as a consistent view marker.\n \n         :param inv_list: InventoryList object\n         :param res_prov: ResourceProvider object\n         \"\"\"\n         inv_table \u003d api_models.Inventory.__table__\n         res_prov_table \u003d api_models.ResourceProvider.__table__\n \n         orig_resources \u003d cls.get_by_resource_provider_id(res_prov.id)\n         # Bear with me and assume this set() stuff works like you\n         # think it might... I know that isn\u0027t the case right now.\n         new_resources \u003d set(inv_list.objects) - set(orig_resources.objects)\n         del_resources \u003d set(orig_resources.objects) - set(inv_list.objects)\n         upd_resources \u003d set(inv_list.objects) - new_resources\n \n         generation \u003d res_provider.generation\n         ctx \u003d db_api.api_context_manager.writer_context()\n         conn \u003d ctx.connection()\n         with conn.begin():\n             for inv in new_resources:\n                 ins \u003d inv_table.insert().values(\n                      resource_class_id\u003dinv.resource_class.index(),\n                      resource_provider_id\u003dres_prov.id,\n                      total\u003dinv.total,\n                      reserved\u003dinv.reserved,\n                      min_unit\u003dinv.min_unit,\n                      max_unit\u003dinv.max_unit,\n                      step_size\u003dinv.step_size,\n                      allocation_ratio\u003dinv.allocation_ratio,\n                 )\n                 conn.execute(ins)\n             for inv in del_resources:\n                 del \u003d inv_table.delete().where(id\u003dinv.id)\n                 conn.execute(del)\n             for inv in upd_resources:\n                 upd \u003d inv_table.update().where(id\u003dinv.id).values(\n                      total\u003dinv.total,\n                      ...\n                 )\n                 conn.execute(upd)\n \n             # Now we do the final \"consistent view update\"...\n             upd \u003d res_prov_table.update().where(and_(id \u003d\u003d res_prov.id,\n                                                      generation \u003d\u003d generation)\n                                                 ).values(\n                       generation\u003dgeneration + 1\n             )\n             res \u003d conn.execute(upd)\n             if res.rowcount !\u003d 1:\n                 # Raising here will automatically rollback all the\n                 # inserts, updates and deletes from above.\n                 raise ConcurrentInventoryUpdate(...)\n \n             conn.commit()\n\nAnd the compute node\u0027s resource tracker will need to call the above in a loop that handles the ConcurrentInventoryUpdate exception that may be returned if there are multiple writers to a compute node\u0027s inventory (shouldn\u0027t happen, but best to be safe).","commit_id":"2ae640b22cae79f03b0d6de5855fee94091d8a6c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"286c91df137f67fb1c48034048d15e01999e8f51","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        db_inventory_list \u003d cls._get_all_by_resource_provider(context,"},{"line_number":304,"context_line":"                                                              rp_uuid)"},{"line_number":305,"context_line":"        return base.obj_make_list(context, cls(context), objects.Inventory,"},{"line_number":306,"context_line":"                                  db_inventory_list)"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":10,"id":"9abb7d3a_5a7ce0b4","line":306,"in_reply_to":"9abb7d3a_8aec3ff4","updated":"2016-05-31 20:32:40.000000000","message":"Well, right now we have access to the object but I expect we won\u0027t once we call the placement API from the compute node.\n\nHowever, if what you mean above is:\n\n * Do that set-based update on InventoryList\n * Compute node calls some inventory update REST API method in a \n   loop until it succeeds\n\n, then yes, agree :) Definitely agree with the above approach despite the mechanics though.","commit_id":"2ae640b22cae79f03b0d6de5855fee94091d8a6c"}]}