)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"64de790dbbe5610d17df8864b5b3088b87974d58","unresolved":false,"context_lines":[{"line_number":22,"context_line":"will also delete the internal consumer record in the placement"},{"line_number":23,"context_line":"service."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"The race is that the allocations are removed during shelve"},{"line_number":26,"context_line":"offload *after* the server status changes to SHELVED_OFFLOADED."},{"line_number":27,"context_line":"This leaves a window where unshelve is going through the"},{"line_number":28,"context_line":"scheduler and gets the existing allocations for the instance,"},{"line_number":29,"context_line":"which are non-empty and have a consumer generation. The"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_f4312df1","line":26,"range":{"start_line":25,"start_character":0,"end_line":26,"end_character":63},"updated":"2018-11-29 12:45:52.000000000","message":"Can we fix the overall problem by setting the server state to SHELVE_OFFLOADED *after* the allocations are removed from placement? That seems a more logical fix for the problem than trying to figure out in the claim_resources code that what happens and assume that it is safe to retry.","commit_id":"d2db53521bd6246629ee46a1c15ccf84732e7717"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c26c997d9bb06461733f3ff2e2e640b7a93a67c8","unresolved":false,"context_lines":[{"line_number":22,"context_line":"will also delete the internal consumer record in the placement"},{"line_number":23,"context_line":"service."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"The race is that the allocations are removed during shelve"},{"line_number":26,"context_line":"offload *after* the server status changes to SHELVED_OFFLOADED."},{"line_number":27,"context_line":"This leaves a window where unshelve is going through the"},{"line_number":28,"context_line":"scheduler and gets the existing allocations for the instance,"},{"line_number":29,"context_line":"which are non-empty and have a consumer generation. The"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_2cb56bcf","line":26,"range":{"start_line":25,"start_character":0,"end_line":26,"end_character":63},"in_reply_to":"3f79a3b5_f4312df1","updated":"2018-12-07 22:30:07.000000000","message":"https://review.openstack.org/#/c/623596/","commit_id":"d2db53521bd6246629ee46a1c15ccf84732e7717"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9530bee89c3f5b43cb5417d734c1746abb511c27","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"2. We could remove the allocations during unshelve before"},{"line_number":52,"context_line":"   updating the instance.vm_state. This would solve the race"},{"line_number":53,"context_line":"   for this particular scenario, but is really whack-a-mole"},{"line_number":54,"context_line":"   and we should handle the generation conflict / retry in the"},{"line_number":55,"context_line":"   scheduler in a generic way if possible (which we do here)."},{"line_number":56,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_467d826e","line":53,"range":{"start_line":53,"start_character":47,"end_line":53,"end_character":59},"updated":"2018-11-19 22:57:06.000000000","message":"Special-casing \u0027null\u0027 here is still whack-a-mole-y IMO. But arguably even worse, because it duct-tapes cases we haven\u0027t thought of as well as this particular one, which might not always be the right thing.","commit_id":"d2db53521bd6246629ee46a1c15ccf84732e7717"}],"nova/scheduler/client/report.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9530bee89c3f5b43cb5417d734c1746abb511c27","unresolved":false,"context_lines":[{"line_number":1868,"context_line":"                # this but we have no error codes, only error messages."},{"line_number":1869,"context_line":"                # TODO(gibi): Use more granular error codes when available"},{"line_number":1870,"context_line":"                if \u0027consumer generation conflict\u0027 in err[\u0027detail\u0027]:"},{"line_number":1871,"context_line":"                    # NOTE(mriedem): Deal with a race where the consumer"},{"line_number":1872,"context_line":"                    # was deleted between the time we got the allocations with"},{"line_number":1873,"context_line":"                    # a consumer_generation and we PUT the allocations using"},{"line_number":1874,"context_line":"                    # that consumer_generation, and placement will create a"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_46e26238","line":1871,"updated":"2018-11-19 22:57:06.000000000","message":"I\u0027m not sure why it wouldn\u0027t be okay just to retry *any* time we get the 409. If we don\u0027t think it\u0027s acceptable to retry on some random update to the allocations, why is some random deletion of the allocations acceptable? The theory being that there may be random deletions of allocations that aren\u0027t because of this specific race condition.","commit_id":"d2db53521bd6246629ee46a1c15ccf84732e7717"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1f633474c88afcc6ac49f835abfdbeca8d7a2819","unresolved":false,"context_lines":[{"line_number":1868,"context_line":"                # this but we have no error codes, only error messages."},{"line_number":1869,"context_line":"                # TODO(gibi): Use more granular error codes when available"},{"line_number":1870,"context_line":"                if \u0027consumer generation conflict\u0027 in err[\u0027detail\u0027]:"},{"line_number":1871,"context_line":"                    # NOTE(mriedem): Deal with a race where the consumer"},{"line_number":1872,"context_line":"                    # was deleted between the time we got the allocations with"},{"line_number":1873,"context_line":"                    # a consumer_generation and we PUT the allocations using"},{"line_number":1874,"context_line":"                    # that consumer_generation, and placement will create a"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_d1c4df3b","line":1871,"in_reply_to":"3f79a3b5_46e26238","updated":"2018-11-20 13:47:57.000000000","message":"Well, that\u0027s what we used to do before the change in this code to have consumer generations:\n\nhttps://review.openstack.org/#/c/583667/29/nova/scheduler/client/report.py\n\nThat change made the retry surface very specific, but missed the null consumer race scenario that we\u0027re hitting in this tempest shelve test. So in this change I was trying to continue with being specific on the retries.","commit_id":"d2db53521bd6246629ee46a1c15ccf84732e7717"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"adf84bc5a8186ba5ffbbd7b982a3082c9e877695","unresolved":false,"context_lines":[{"line_number":1874,"context_line":"                    # that consumer_generation, and placement will create a"},{"line_number":1875,"context_line":"                    # new consumer record with a null generation and that"},{"line_number":1876,"context_line":"                    # causes the conflict."},{"line_number":1877,"context_line":"                    if \u0027expected null but got\u0027 in err[\u0027detail\u0027]:"},{"line_number":1878,"context_line":"                        # In this case, we simply retry because the"},{"line_number":1879,"context_line":"                        # GET /allocations/{consumer_id} should be empty"},{"line_number":1880,"context_line":"                        # and we\u0027ll be oh so happy."}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_512d2fab","line":1877,"updated":"2018-11-20 14:06:28.000000000","message":"I hate deep nesting like this, and would prefer to check for this specific case before the general case of any other consumer generation conflict so they are on the same level. But, this works.","commit_id":"d2db53521bd6246629ee46a1c15ccf84732e7717"}]}