)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"94df3286bdc18ceeb25f6f703f02b8d127d21bb7","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This counts instance mappings for counting quota usage for instances"},{"line_number":10,"context_line":"and adds calls to placement for counting quota usage for cores and ram."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"During an upgrade, if any un-migrated instance mappings are found (with"},{"line_number":13,"context_line":"NULL user_id), we will fall back on the legacy counting method."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Because placement does not yet support the ability to partition"},{"line_number":16,"context_line":"allocations from multiple Nova deployments, we also provide a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":22,"id":"5fc1f717_1f632544","line":13,"range":{"start_line":12,"start_character":26,"end_line":13,"end_character":13},"updated":"2019-03-25 12:27:02.000000000","message":"as mentioned in my previous patchset, we should do the same for NULL queued_for_delete IMO,","commit_id":"eca06d5054e27ca68d7cf6cb9a6b124b63658ea6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"eef52f30ae8a2ab3bfa199940746bf9fbbe7d412","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This counts instance mappings for counting quota usage for instances"},{"line_number":10,"context_line":"and adds calls to placement for counting quota usage for cores and ram."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"During an upgrade, if any un-migrated instance mappings are found (with"},{"line_number":13,"context_line":"NULL user_id), we will fall back on the legacy counting method."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Because placement does not yet support the ability to partition"},{"line_number":16,"context_line":"allocations from multiple Nova deployments, we also provide a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":22,"id":"5fc1f717_da1e0b5e","line":13,"range":{"start_line":12,"start_character":26,"end_line":13,"end_character":13},"in_reply_to":"5fc1f717_1f632544","updated":"2019-03-25 12:33:07.000000000","message":"we could add extra documentation saying unless the whole deployment hasn\u0027t migrated the instance_mapping.user_id and queued_for_delete this method of counting from placement is not recommended since it would fall back to legacy by introducing a bunch of unwanted workflow through the instance_mappings.","commit_id":"eca06d5054e27ca68d7cf6cb9a6b124b63658ea6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b06ecac20df04770b902eb5ae4cf23fcebb45851","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This counts instance mappings for counting quota usage for instances"},{"line_number":10,"context_line":"and adds calls to placement for counting quota usage for cores and ram."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"During an upgrade, if any un-migrated instance mappings are found (with"},{"line_number":13,"context_line":"NULL user_id), we will fall back on the legacy counting method."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Because placement does not yet support the ability to partition"},{"line_number":16,"context_line":"allocations from multiple Nova deployments, we also provide a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":22,"id":"5fc1f717_0fd40e47","line":13,"range":{"start_line":12,"start_character":26,"end_line":13,"end_character":13},"in_reply_to":"5fc1f717_4fb69640","updated":"2019-03-27 19:19:47.000000000","message":"Quota usage counting from placement isn\u0027t opt-in, you have to opt-out if you don\u0027t want it:\n\nhttps://review.openstack.org/#/c/638073/22/nova/conf/workarounds.py@254\n\nAnd this is the cost of detecting whether migration is done:\n\nhttps://review.openstack.org/#/c/638073/22/nova/objects/instance_mapping.py@246\n\nThis is different than the full counting flow that happens if no unmigrated records are detected, but indeed it\u0027s still a cost.\n\nWhere do you suggest mentioning more detail? In the release note? (Which reminds me, this patch needs a release note).","commit_id":"eca06d5054e27ca68d7cf6cb9a6b124b63658ea6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"c84184f1c5e18e5088a0ae24b12e6e2a81c9def7","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This counts instance mappings for counting quota usage for instances"},{"line_number":10,"context_line":"and adds calls to placement for counting quota usage for cores and ram."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"During an upgrade, if any un-migrated instance mappings are found (with"},{"line_number":13,"context_line":"NULL user_id), we will fall back on the legacy counting method."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Because placement does not yet support the ability to partition"},{"line_number":16,"context_line":"allocations from multiple Nova deployments, we also provide a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":22,"id":"5fc1f717_4fb69640","line":13,"range":{"start_line":12,"start_character":26,"end_line":13,"end_character":13},"in_reply_to":"5fc1f717_ac0aa822","updated":"2019-03-27 18:49:50.000000000","message":"\u003e Can you explain a bit more what you mean by \"bunch of unwanted\n \u003e workflow through the instance mappings\"? Do you just mean the COUNT\n \u003e sql query for detecting unmigrated records?\n\nSorry if it came out wrong, what I meant was, if the operator hasn\u0027t run either the user_id or queued_for_delete migrations yet, they should not opt into this method of counting quota since it would first go through instance_mappings and then fall back to legacy while they could just to straight to using legacy if they haven\u0027t done the migrations yet.","commit_id":"eca06d5054e27ca68d7cf6cb9a6b124b63658ea6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f912b474bcedb7988196d3b1c6084332f92bdcce","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This counts instance mappings for counting quota usage for instances"},{"line_number":10,"context_line":"and adds calls to placement for counting quota usage for cores and ram."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"During an upgrade, if any un-migrated instance mappings are found (with"},{"line_number":13,"context_line":"NULL user_id), we will fall back on the legacy counting method."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Because placement does not yet support the ability to partition"},{"line_number":16,"context_line":"allocations from multiple Nova deployments, we also provide a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":22,"id":"5fc1f717_ac0aa822","line":13,"range":{"start_line":12,"start_character":26,"end_line":13,"end_character":13},"in_reply_to":"5fc1f717_da1e0b5e","updated":"2019-03-27 18:37:09.000000000","message":"Can you explain a bit more what you mean by \"bunch of unwanted workflow through the instance mappings\"? Do you just mean the COUNT sql query for detecting unmigrated records?","commit_id":"eca06d5054e27ca68d7cf6cb9a6b124b63658ea6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This counts instance mappings for counting quota usage for instances"},{"line_number":10,"context_line":"and adds calls to placement for counting quota usage for cores and ram."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"During an upgrade, if any un-migrated instance mappings are found (with"},{"line_number":13,"context_line":"NULL user_id), we will fall back on the legacy counting method."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Because placement does not yet support the ability to partition"},{"line_number":16,"context_line":"allocations from multiple Nova deployments, we also provide a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":24,"id":"5fc1f717_dc923d4b","line":13,"range":{"start_line":12,"start_character":67,"end_line":13,"end_character":12},"updated":"2019-04-10 20:13:55.000000000","message":"or queued_for_delete\u003dNone?\n\nhttps://review.openstack.org/#/c/638072/14/nova/objects/instance_mapping.py@386","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This counts instance mappings for counting quota usage for instances"},{"line_number":10,"context_line":"and adds calls to placement for counting quota usage for cores and ram."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"During an upgrade, if any un-migrated instance mappings are found (with"},{"line_number":13,"context_line":"NULL user_id), we will fall back on the legacy counting method."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Because placement does not yet support the ability to partition"},{"line_number":16,"context_line":"allocations from multiple Nova deployments, we also provide a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":24,"id":"3fce034c_2f71a5e1","line":13,"range":{"start_line":12,"start_character":67,"end_line":13,"end_character":12},"in_reply_to":"5fc1f717_dc923d4b","updated":"2019-04-10 20:53:58.000000000","message":"Good catch, I need to add \"or NULL queued_for_delete\"","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f5e4e36bc388046389087034a0e24cb4838d79ac","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"  * Though beneficial for multi-cell deployments to be resilient to"},{"line_number":20,"context_line":"    down cells, the vast majority of deployments are single cell and"},{"line_number":21,"context_line":"    will not be able to realize this benefit."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"  * Usage for resizes will reflect resources being held on both the"},{"line_number":24,"context_line":"    source and destination until the resize is confirmed or reverted."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"dfbec78f_5d837b57","line":21,"range":{"start_line":21,"start_character":32,"end_line":21,"end_character":36},"updated":"2019-05-02 18:18:56.000000000","message":"s/this/the down cells resiliency/","commit_id":"e34a1ee797ba3b3016bf27664c3a0fe985dcf247"},{"author":{"_account_id":12408,"name":"Maxim Nestratov","email":"mnestratov@virtuozzo.com","username":"mnestratov"},"change_message_id":"0ff15f6b4cf4381df3863a109cd9e35a30ad241d","unresolved":false,"context_lines":[{"line_number":34,"context_line":"  * Usage for unscheduled instances in ERROR state will not reflect"},{"line_number":35,"context_line":"    resource consumption for cores and ram because the instance has no"},{"line_number":36,"context_line":"    placement allocations."},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"Part of blueprint count-quota-usage-from-placement"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"Change-Id: Ie22b0acb5824a41da327abdcf9848d02fc9a92f5"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"dfbec78f_d202fa3e","line":37,"updated":"2019-05-02 18:03:44.000000000","message":"If SHELVED_OFFLOADED servers don\u0027t have placement allocation could you please mention this explicitly then in the commit message as this spec is supersedes the proposed spec https://review.opendev.org/#/c/656806/.","commit_id":"e34a1ee797ba3b3016bf27664c3a0fe985dcf247"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f5e4e36bc388046389087034a0e24cb4838d79ac","unresolved":false,"context_lines":[{"line_number":34,"context_line":"  * Usage for unscheduled instances in ERROR state will not reflect"},{"line_number":35,"context_line":"    resource consumption for cores and ram because the instance has no"},{"line_number":36,"context_line":"    placement allocations."},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"Part of blueprint count-quota-usage-from-placement"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"Change-Id: Ie22b0acb5824a41da327abdcf9848d02fc9a92f5"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"dfbec78f_9d49f361","line":37,"in_reply_to":"dfbec78f_d202fa3e","updated":"2019-05-02 18:18:56.000000000","message":"Yes, will do. I wrote myself a TODO to update this commit message, release note, and docs patch to include info about SHELVED_OFFLOADED behavior when operators opt-in to this change.","commit_id":"e34a1ee797ba3b3016bf27664c3a0fe985dcf247"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"e25e234eb5dda7473d6229241042a6a6b7e6594d","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    aggregates all Nova deployments together. Such environments should"},{"line_number":33,"context_line":"    not enable counting from placement."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"  * Usage for unscheduled instances in ERROR state will not reflect"},{"line_number":36,"context_line":"    resource consumption for cores and ram because the instance has no"},{"line_number":37,"context_line":"    placement allocations."},{"line_number":38,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":30,"id":"dfbec78f_2c2000ac","line":35,"updated":"2019-05-16 14:01:58.000000000","message":"I understand this is a behavior change, but I think its a good thing :), what would be scary is when we start to count the number of instances from placement (after consumer types) in which case we stop counting ERROR instances in our quota usage that means users might never delete these vms (making cell0 an operator headache).","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"444e8876ce7391e52775b1843d858b0c592562ce","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    aggregates all Nova deployments together. Such environments should"},{"line_number":33,"context_line":"    not enable counting from placement."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"  * Usage for unscheduled instances in ERROR state will not reflect"},{"line_number":36,"context_line":"    resource consumption for cores and ram because the instance has no"},{"line_number":37,"context_line":"    placement allocations."},{"line_number":38,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":30,"id":"bfb3d3c7_fc453f16","line":35,"in_reply_to":"dfbec78f_2c2000ac","updated":"2019-05-22 11:53:51.000000000","message":"+1 your concern.\n\nBecause they will count for the server count, I am less worried overall.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"e25e234eb5dda7473d6229241042a6a6b7e6594d","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"  * Usage for instances in SHELVED_OFFLOADED state will not reflect"},{"line_number":40,"context_line":"    resource consumption for cores and ram because the instance has no"},{"line_number":41,"context_line":"    placement allocations. Note that because of this, it will be possible for a"},{"line_number":42,"context_line":"    request to unshelve a server to be rejected if the user does not have"},{"line_number":43,"context_line":"    enough quota available to support the cores and ram needed by the server to"},{"line_number":44,"context_line":"    be unshelved."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":30,"id":"dfbec78f_67ea93d8","line":41,"updated":"2019-05-16 14:01:58.000000000","message":"[note to self on this behaviour change] so in the legacy way we were counting quota for this always and so we would be sure of being able to unshelve and now this is dependent on the fly ? hmm I guess its not that bad considering the concept of shelving since this way we don\u0027t block quota on being able to create newer vms while something is shelved.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"444e8876ce7391e52775b1843d858b0c592562ce","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"  * Usage for instances in SHELVED_OFFLOADED state will not reflect"},{"line_number":40,"context_line":"    resource consumption for cores and ram because the instance has no"},{"line_number":41,"context_line":"    placement allocations. Note that because of this, it will be possible for a"},{"line_number":42,"context_line":"    request to unshelve a server to be rejected if the user does not have"},{"line_number":43,"context_line":"    enough quota available to support the cores and ram needed by the server to"},{"line_number":44,"context_line":"    be unshelved."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":30,"id":"bfb3d3c7_3c5d17e9","line":41,"in_reply_to":"dfbec78f_67ea93d8","updated":"2019-05-22 11:53:51.000000000","message":"There are a bunch of things that fail when you try to unshelve, this is just one more thing.\n\nI think this new behaviour is nice, as it matches what people were going to do with billing, i.e. only charge for things like floating ips that happen to be attached, etc.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"e25e234eb5dda7473d6229241042a6a6b7e6594d","unresolved":false,"context_lines":[{"line_number":42,"context_line":"    request to unshelve a server to be rejected if the user does not have"},{"line_number":43,"context_line":"    enough quota available to support the cores and ram needed by the server to"},{"line_number":44,"context_line":"    be unshelved."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"Part of blueprint count-quota-usage-from-placement"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"Change-Id: Ie22b0acb5824a41da327abdcf9848d02fc9a92f5"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":30,"id":"dfbec78f_c7557ffb","line":45,"updated":"2019-05-16 14:01:58.000000000","message":"nicely detailed commit message, ++","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e18d943135bf8444d196a8a01dcb1312cf319c35","unresolved":false,"context_lines":[{"line_number":21,"context_line":"    will not be able to realize a down cells resiliency benefit and may"},{"line_number":22,"context_line":"    prefer to keep legacy quota usage counting."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"  * Usage for resizes will reflect resources being held on both the"},{"line_number":25,"context_line":"    source and destination until the resize is confirmed or reverted."},{"line_number":26,"context_line":"    Operators may not want to enable counting from placement based on"},{"line_number":27,"context_line":"    whether the behavior change is problematic for them."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"bfb3d3c7_0fa18946","line":24,"updated":"2019-05-29 15:56:04.000000000","message":"To refresh my memory, before this we\u0027re counting cores/ram based on the fields in the instance record which are either the old or new flavor depending on when you count and at what point along the resize operation we are, i.e. if we\u0027re in VERIFY_RESIZE status then the new flavor values are used.\n\nAnd if we want to eventually get out of the double-counting situation with placement, we need to have consumer types or something to be able to say we only want to count usage based on the instance (new_flavor) allocation rather than the migration (old_flavor) allocation.\n\nI can\u0027t remember off-hand what you said the behavior was pre-Pike and counting quotas during a resize, but I thought you said we\u0027d reserve space for a revert in case the new flavor was smaller than the old flavor, which we would have regressed with counting quotas in Pike (but apparently no one has noticed).","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"db1a4afb5378d351fad66962aaa098ed59546223","unresolved":false,"context_lines":[{"line_number":21,"context_line":"    will not be able to realize a down cells resiliency benefit and may"},{"line_number":22,"context_line":"    prefer to keep legacy quota usage counting."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"  * Usage for resizes will reflect resources being held on both the"},{"line_number":25,"context_line":"    source and destination until the resize is confirmed or reverted."},{"line_number":26,"context_line":"    Operators may not want to enable counting from placement based on"},{"line_number":27,"context_line":"    whether the behavior change is problematic for them."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"bfb3d3c7_8ac48441","line":24,"in_reply_to":"bfb3d3c7_0fa18946","updated":"2019-05-29 17:06:54.000000000","message":"\u003e To refresh my memory, before this we\u0027re counting cores/ram based on\n \u003e the fields in the instance record which are either the old or new\n \u003e flavor depending on when you count and at what point along the\n \u003e resize operation we are, i.e. if we\u0027re in VERIFY_RESIZE status then\n \u003e the new flavor values are used.\n\nThat\u0027s right, depends on whether the new_flavor has been saved to instance.vcpus and instance.memory_mb yet.\n\n \u003e And if we want to eventually get out of the double-counting\n \u003e situation with placement, we need to have consumer types or\n \u003e something to be able to say we only want to count usage based on\n \u003e the instance (new_flavor) allocation rather than the migration\n \u003e (old_flavor) allocation.\n\nRight.\n\n \u003e I can\u0027t remember off-hand what you said the behavior was pre-Pike\n \u003e and counting quotas during a resize, but I thought you said we\u0027d\n \u003e reserve space for a revert in case the new flavor was smaller than\n \u003e the old flavor, which we would have regressed with counting quotas\n \u003e in Pike (but apparently no one has noticed).\n\nThe behavior pre-Pike was that space was reserved in the case of a downsize, to leave room for a revert (no chance to go over quota during a revert). And yes this was regressed with counting quotas in Pike and either no one noticed or no one minded, that we\u0027re aware of.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e18d943135bf8444d196a8a01dcb1312cf319c35","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    aggregates all Nova deployments together. Such environments should"},{"line_number":33,"context_line":"    not enable counting from placement."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"  * Usage for unscheduled instances in ERROR state will not reflect"},{"line_number":36,"context_line":"    resource consumption for cores and ram because the instance has no"},{"line_number":37,"context_line":"    placement allocations."},{"line_number":38,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"bfb3d3c7_cf5db147","line":35,"updated":"2019-05-29 15:56:04.000000000","message":"I also forgot, is this the behavior pre-Pike and counting quotas? If so, then this (counting from placement) is actually correcting that regression, but again no one has been complaining about this as far as I know. I agree that we shouldn\u0027t be counting cores/ram usage for instances that were never scheduled and thus not on a host (although we also count cores/ram usage for SHELVE_OFFLOADED instances even though they aren\u0027t on a host, which is weird and I know some operators would like to change the billing model around that).","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"db1a4afb5378d351fad66962aaa098ed59546223","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    aggregates all Nova deployments together. Such environments should"},{"line_number":33,"context_line":"    not enable counting from placement."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"  * Usage for unscheduled instances in ERROR state will not reflect"},{"line_number":36,"context_line":"    resource consumption for cores and ram because the instance has no"},{"line_number":37,"context_line":"    placement allocations."},{"line_number":38,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"bfb3d3c7_eace6058","line":35,"in_reply_to":"bfb3d3c7_cf5db147","updated":"2019-05-29 17:06:54.000000000","message":"The behavior pre-Pike was that ERROR instances, cores, and ram count as part of your quota usage. This patch series will be the first time ever that ERROR instance cores and ram will not count toward quota usage.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e18d943135bf8444d196a8a01dcb1312cf319c35","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    resource consumption for cores and ram because the instance has no"},{"line_number":37,"context_line":"    placement allocations."},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"  * Usage for instances in SHELVED_OFFLOADED state will not reflect"},{"line_number":40,"context_line":"    resource consumption for cores and ram because the instance has no"},{"line_number":41,"context_line":"    placement allocations. Note that because of this, it will be possible for a"},{"line_number":42,"context_line":"    request to unshelve a server to be rejected if the user does not have"},{"line_number":43,"context_line":"    enough quota available to support the cores and ram needed by the server to"},{"line_number":44,"context_line":"    be unshelved."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"bfb3d3c7_6f584555","line":41,"range":{"start_line":39,"start_character":4,"end_line":41,"end_character":26},"updated":"2019-05-29 15:56:04.000000000","message":"Aha, yeah, just what I was saying above.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e18d943135bf8444d196a8a01dcb1312cf319c35","unresolved":false,"context_lines":[{"line_number":39,"context_line":"  * Usage for instances in SHELVED_OFFLOADED state will not reflect"},{"line_number":40,"context_line":"    resource consumption for cores and ram because the instance has no"},{"line_number":41,"context_line":"    placement allocations. Note that because of this, it will be possible for a"},{"line_number":42,"context_line":"    request to unshelve a server to be rejected if the user does not have"},{"line_number":43,"context_line":"    enough quota available to support the cores and ram needed by the server to"},{"line_number":44,"context_line":"    be unshelved."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"Part of blueprint count-quota-usage-from-placement"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"bfb3d3c7_1a0b0524","line":44,"range":{"start_line":42,"start_character":4,"end_line":44,"end_character":17},"updated":"2019-05-29 15:56:04.000000000","message":"Yeah I think this is probably fine, and I think John mentioned in an earlier patch set that there are many reasons unshelve could fail because things have changed since the server was offloaded, so it\u0027s a crapshoot. I could see someone down the road saying they would like to be able to unshelve with a new flavor to get around this (since you can\u0027t resize an offloaded server), but let\u0027s assume it won\u0027t be that big of a problem (the unshelve and no quota thing).","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"}],"nova/conf/quota.py":[{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"e25e234eb5dda7473d6229241042a6a6b7e6594d","unresolved":false,"context_lines":[{"line_number":325,"context_line":"server in ERROR state that has never been scheduled to a compute host will"},{"line_number":326,"context_line":"not have placement allocations, so it will not consume quota usage for cores"},{"line_number":327,"context_line":"and ram."},{"line_number":328,"context_line":"\"\"\"),"},{"line_number":329,"context_line":"]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_277c9b81","line":328,"updated":"2019-05-16 14:01:58.000000000","message":"do you think we should put the SHELVED_OFFLOADED and the unshelving behavior change here as well ?","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"07ba290e3bb1a37c71e5f7759cbb891be2710068","unresolved":false,"context_lines":[{"line_number":325,"context_line":"server in ERROR state that has never been scheduled to a compute host will"},{"line_number":326,"context_line":"not have placement allocations, so it will not consume quota usage for cores"},{"line_number":327,"context_line":"and ram."},{"line_number":328,"context_line":"\"\"\"),"},{"line_number":329,"context_line":"]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"bfb3d3c7_c0dba2ea","line":328,"in_reply_to":"bfb3d3c7_9c6d239a","updated":"2019-05-22 23:29:20.000000000","message":"I think I just missed adding the paragraph here as well. I didn\u0027t intentionally omit it. Will add in the next PS.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"444e8876ce7391e52775b1843d858b0c592562ce","unresolved":false,"context_lines":[{"line_number":325,"context_line":"server in ERROR state that has never been scheduled to a compute host will"},{"line_number":326,"context_line":"not have placement allocations, so it will not consume quota usage for cores"},{"line_number":327,"context_line":"and ram."},{"line_number":328,"context_line":"\"\"\"),"},{"line_number":329,"context_line":"]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"bfb3d3c7_9c6d239a","line":328,"in_reply_to":"dfbec78f_277c9b81","updated":"2019-05-22 11:53:51.000000000","message":"It is a good point... maybe we should just link to the admin docs here, and have a simple statement noting there are subtable behaviour changes?","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"}],"nova/conf/workarounds.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2631a05c8cf46b38838df324619e89e2150e4c42","unresolved":false,"context_lines":[{"line_number":251,"context_line":""},{"line_number":252,"context_line":"Starting in Stein, quota usage for instances, cores, and ram is counted from"},{"line_number":253,"context_line":"the placement service instead of counting it locally in cell databases. This"},{"line_number":254,"context_line":"works well if there is only one Nova service running per placement service."},{"line_number":255,"context_line":"However, if operators are running more than one Nova service per placement"},{"line_number":256,"context_line":"service, they will need to set this workaround option to True because"},{"line_number":257,"context_line":"currently, the placement service has no way to partition resource allocations"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_bcbf2cce","line":254,"range":{"start_line":254,"start_character":28,"end_line":254,"end_character":44},"updated":"2019-02-28 21:42:16.000000000","message":"one conductor? one compute? ...","commit_id":"28c83029dddd88c1687e909731bb636a58d814d7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f941f110312b377cd4dee1cba93e5f2cee12f133","unresolved":false,"context_lines":[{"line_number":251,"context_line":""},{"line_number":252,"context_line":"Starting in Stein, quota usage for instances, cores, and ram is counted from"},{"line_number":253,"context_line":"the placement service instead of counting it locally in cell databases. This"},{"line_number":254,"context_line":"works well if there is only one Nova service running per placement service."},{"line_number":255,"context_line":"However, if operators are running more than one Nova service per placement"},{"line_number":256,"context_line":"service, they will need to set this workaround option to True because"},{"line_number":257,"context_line":"currently, the placement service has no way to partition resource allocations"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_b5ec1f29","line":254,"range":{"start_line":254,"start_character":28,"end_line":254,"end_character":44},"in_reply_to":"9fdfeff1_327285e4","updated":"2019-03-01 00:17:23.000000000","message":"That is true, only a problem if they share project/user IDs, specifically for the problem of quota usage counting. In general, I believe you can GET /usages without filtering, but we always filter for quota usage counting.","commit_id":"28c83029dddd88c1687e909731bb636a58d814d7"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"4f228ea72f93f01d80b8500cabe6b57558d6d639","unresolved":false,"context_lines":[{"line_number":251,"context_line":""},{"line_number":252,"context_line":"Starting in Stein, quota usage for instances, cores, and ram is counted from"},{"line_number":253,"context_line":"the placement service instead of counting it locally in cell databases. This"},{"line_number":254,"context_line":"works well if there is only one Nova service running per placement service."},{"line_number":255,"context_line":"However, if operators are running more than one Nova service per placement"},{"line_number":256,"context_line":"service, they will need to set this workaround option to True because"},{"line_number":257,"context_line":"currently, the placement service has no way to partition resource allocations"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_327285e4","line":254,"range":{"start_line":254,"start_character":28,"end_line":254,"end_character":44},"in_reply_to":"9fdfeff1_7c9ea406","updated":"2019-03-01 00:03:49.000000000","message":"Okay, I think I understand now, but I didn\u0027t before. I think changing to \"Nova deployment\" is clearer.\n\nAlso, it\u0027s only a problem for multiple deployments if they share project/user IDs, right?","commit_id":"28c83029dddd88c1687e909731bb636a58d814d7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e39a326d103b1c5e646a2622ff378f136cba3ebb","unresolved":false,"context_lines":[{"line_number":251,"context_line":""},{"line_number":252,"context_line":"Starting in Stein, quota usage for instances, cores, and ram is counted from"},{"line_number":253,"context_line":"the placement service instead of counting it locally in cell databases. This"},{"line_number":254,"context_line":"works well if there is only one Nova service running per placement service."},{"line_number":255,"context_line":"However, if operators are running more than one Nova service per placement"},{"line_number":256,"context_line":"service, they will need to set this workaround option to True because"},{"line_number":257,"context_line":"currently, the placement service has no way to partition resource allocations"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_5ee35007","line":254,"range":{"start_line":254,"start_character":28,"end_line":254,"end_character":44},"in_reply_to":"9fdfeff1_8d0a0677","updated":"2019-03-05 07:54:15.000000000","message":"Correct.","commit_id":"28c83029dddd88c1687e909731bb636a58d814d7"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8cbc2da8842e92ab1024893b0a013db2200d316b","unresolved":false,"context_lines":[{"line_number":251,"context_line":""},{"line_number":252,"context_line":"Starting in Stein, quota usage for instances, cores, and ram is counted from"},{"line_number":253,"context_line":"the placement service instead of counting it locally in cell databases. This"},{"line_number":254,"context_line":"works well if there is only one Nova service running per placement service."},{"line_number":255,"context_line":"However, if operators are running more than one Nova service per placement"},{"line_number":256,"context_line":"service, they will need to set this workaround option to True because"},{"line_number":257,"context_line":"currently, the placement service has no way to partition resource allocations"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_8d0a0677","line":254,"range":{"start_line":254,"start_character":28,"end_line":254,"end_character":44},"in_reply_to":"9fdfeff1_b5ec1f29","updated":"2019-03-04 12:33:10.000000000","message":"Just to add more noise to this particular concern: It seems likely that in a situation where one placement is managing more than one nova, it is likely there could also be one keystone managing more than one nova.\n\nIn which case project uuid\u0027s might be shared.","commit_id":"28c83029dddd88c1687e909731bb636a58d814d7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7bfd7b9c49b0df9510c3bc4ad178cc386417e1d8","unresolved":false,"context_lines":[{"line_number":251,"context_line":""},{"line_number":252,"context_line":"Starting in Stein, quota usage for instances, cores, and ram is counted from"},{"line_number":253,"context_line":"the placement service instead of counting it locally in cell databases. This"},{"line_number":254,"context_line":"works well if there is only one Nova service running per placement service."},{"line_number":255,"context_line":"However, if operators are running more than one Nova service per placement"},{"line_number":256,"context_line":"service, they will need to set this workaround option to True because"},{"line_number":257,"context_line":"currently, the placement service has no way to partition resource allocations"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_7c9ea406","line":254,"range":{"start_line":254,"start_character":28,"end_line":254,"end_character":44},"in_reply_to":"9fdfeff1_bcbf2cce","updated":"2019-02-28 22:02:20.000000000","message":"Hm, maybe I should say \"Nova deployment\". I\u0027m trying to describe that if there is more than one Nova deployment talking to one placement deployment, a GET /usages from placement for VCPU would include resources for all Nova deployments PUT\u0027ing allocations into placement.","commit_id":"28c83029dddd88c1687e909731bb636a58d814d7"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"a5358d6797c8a5cd893c6371358372f9666e5e9c","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"Starting in Stein, quota usage for instances, cores, and ram is counted from"},{"line_number":260,"context_line":"the placement service instead of counting it locally in cell databases. This"},{"line_number":261,"context_line":"works well if there is only one Nova deployment running per placement"},{"line_number":262,"context_line":"deployment."}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_5eaa9385","line":259,"range":{"start_line":259,"start_character":35,"end_line":259,"end_character":44},"updated":"2019-03-06 09:30:53.000000000","message":"probably doesn\u0027t matter, but this is not counted from placement","commit_id":"6dfc30d0020f3b75cf30c60d85e351994cb34669"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9253af62dc7823eaf680f8bd91317e82a5ca805e","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"Starting in Stein, quota usage for instances, cores, and ram is counted from"},{"line_number":260,"context_line":"the placement service instead of counting it locally in cell databases. This"},{"line_number":261,"context_line":"works well if there is only one Nova deployment running per placement"},{"line_number":262,"context_line":"deployment."}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_0037b4b2","line":259,"range":{"start_line":259,"start_character":35,"end_line":259,"end_character":44},"in_reply_to":"5fc1f717_5eaa9385","updated":"2019-03-06 23:18:42.000000000","message":"True, I\u0027ll update this to avoid confusion.","commit_id":"6dfc30d0020f3b75cf30c60d85e351994cb34669"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"723e4a0664122130a06bee0ef812940b2970541b","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"Starting in Stein, quota usage for instances, cores, and ram is counted from"},{"line_number":260,"context_line":"the placement service instead of counting it locally in cell databases. This"},{"line_number":261,"context_line":"works well if there is only one Nova deployment running per placement"},{"line_number":262,"context_line":"deployment."}],"source_content_type":"text/x-python","patch_set":19,"id":"5fc1f717_c098cc6b","line":259,"range":{"start_line":259,"start_character":35,"end_line":259,"end_character":44},"updated":"2019-03-06 23:33:57.000000000","message":"TODO: remove this","commit_id":"d132c15418ff600097ca83d528c0b374467e7bef"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c90978be7431eb8b73fe6f8dac0b8bca86fc28ba","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"Starting in Stein, quota usage for cores and ram is counted from the placement"},{"line_number":260,"context_line":"service instead of counting it locally in cell databases. This works well if"},{"line_number":261,"context_line":"there is only one Nova deployment running per placement deployment. However,"},{"line_number":262,"context_line":"if an operator is running more than one Nova deployment per placement"}],"source_content_type":"text/x-python","patch_set":23,"id":"5fc1f717_3c33b415","line":259,"updated":"2019-04-04 17:25:09.000000000","message":"Is it train now? I\u0027m unclear how much functionality merged.","commit_id":"f140b079481989aeea3c15109da953564abca4f2"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d573ba0182f4b8ebdad8788d6aa9126ad208323","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"Starting in Stein, quota usage for cores and ram is counted from the placement"},{"line_number":260,"context_line":"service instead of counting it locally in cell databases. This works well if"},{"line_number":261,"context_line":"there is only one Nova deployment running per placement deployment. However,"},{"line_number":262,"context_line":"if an operator is running more than one Nova deployment per placement"}],"source_content_type":"text/x-python","patch_set":23,"id":"5fc1f717_942cc199","line":259,"in_reply_to":"5fc1f717_3c33b415","updated":"2019-04-09 01:54:22.000000000","message":"Yes, thanks. The data migration pieces merged in stein but not the counting part. I need to update this.","commit_id":"f140b079481989aeea3c15109da953564abca4f2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":251,"context_line":"\"\"\"),"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    cfg.BoolOpt("},{"line_number":254,"context_line":"        \u0027disable_quota_usage_from_placement\u0027,"},{"line_number":255,"context_line":"        default\u003dFalse,"},{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_2ca9b3de","line":254,"updated":"2019-04-10 20:13:55.000000000","message":"I guess I didn\u0027t think about this during the spec review, or didn\u0027t say anything, but workarounds are mostly for known issues that eventually go away once something is fixed, but this doesn\u0027t really seem to fit that, meaning this could have gone under the [quotas] section. I realize it\u0027s here because of the case for multiple novas using the same placement / edge site thing, so I guess the long-term answer is \"grow sharding support in placement\" and that\u0027s why it\u0027s in [workarounds], so OK, I\u0027m just not sure if anyone is planning on working on that support in placement. IOW, when do we plan on ever dropping the workaround or will we just leave it as a permanent wrinkle in the config?\n\nI could see this being a useful backdoor in case someone runs into problems with placement counts being wrong and they need to go back to hitting the cell database, e.g. they aren\u0027t using multi-cell and there are some weird orphaned or missing allocations or something and the operator is more comfortable using the old way rather than placement. I\u0027m thinking about stuff like this:\n\nhttps://bugs.launchpad.net/nova/+bug/1793569\n\nTo be sure if we\u0027re leaking allocations or somehow not properly tracking allocations those are bugs that we need to fix, I\u0027m just sure we have some gaps in error handling where we just throw up our hands and never cleanup, e.g.:\n\nhttps://bugs.launchpad.net/nova/+bug/1821594","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":251,"context_line":"\"\"\"),"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    cfg.BoolOpt("},{"line_number":254,"context_line":"        \u0027disable_quota_usage_from_placement\u0027,"},{"line_number":255,"context_line":"        default\u003dFalse,"},{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_2f3f65d7","line":254,"in_reply_to":"3fce034c_2ca9b3de","updated":"2019-04-10 20:53:58.000000000","message":"I think this is a fair point. I put it in workarounds with the assumption that placement is going to need to be able to shard itself (haha) eventually, but it\u0027s true I don\u0027t know when that will be. Maybe it wouldn\u0027t be for a very long time.\n\nAnd then yeah, for the case of leaking allocations and such, having a backdoor. I dunno, maybe it\u0027s worth moving to the [quotas] section for all these reasons.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a602d33f033aecc8dc55c225b707a18ed90bb6aa","unresolved":false,"context_lines":[{"line_number":251,"context_line":"\"\"\"),"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    cfg.BoolOpt("},{"line_number":254,"context_line":"        \u0027disable_quota_usage_from_placement\u0027,"},{"line_number":255,"context_line":"        default\u003dFalse,"},{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_58394a3f","line":254,"in_reply_to":"3fce034c_2f3f65d7","updated":"2019-04-11 14:00:06.000000000","message":"Another thing I was thinking about is should this be the default to avoid the upgrade impact and just mark this as a new feature? I know CERN needs this, but that\u0027s because they have multiple cells and want to avoid the down cell case, but the majority of deployments are going to be single-cell and counting quotas from the cell DB should be fine - unless that goes down, but if that went down they can\u0027t create servers anyway.\n\nSomething to think about but it seems like the more I think about this it\u0027s really for multi-cell deployments which is the vast majority at this point.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"09b657bc06125568f6fb900358ca4020a7c63faa","unresolved":false,"context_lines":[{"line_number":251,"context_line":"\"\"\"),"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    cfg.BoolOpt("},{"line_number":254,"context_line":"        \u0027disable_quota_usage_from_placement\u0027,"},{"line_number":255,"context_line":"        default\u003dFalse,"},{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_edef13ae","line":254,"in_reply_to":"3fce034c_4daae785","updated":"2019-04-11 18:42:11.000000000","message":"\u003e The quota usage behavior change for instances that are resized is another factor that I\u0027ve just thought about since reviewing the spec. I\u0027m not sure if that\u0027s a big enough reason though, but it\u0027s a change in behavior.\n\nHaving said that, I don\u0027t know if we regressed that behavior in Pike with counting quotas or not, i.e. pre-counting did a resize create a reservation on the target host and then commit it after the resize was confirmed so that both resource usage from the ResourceTracker and GET /limits + reserved\u003d1 would reflect the same resource usage. If so, then this would actually get us back to that behavior. ^ would require testing in an ocata deployment to find out what the old behavior was.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b09d6fc6eaba47890c33b013ec058203a5a1612f","unresolved":false,"context_lines":[{"line_number":251,"context_line":"\"\"\"),"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    cfg.BoolOpt("},{"line_number":254,"context_line":"        \u0027disable_quota_usage_from_placement\u0027,"},{"line_number":255,"context_line":"        default\u003dFalse,"},{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_87266eb7","line":254,"in_reply_to":"3fce034c_58394a3f","updated":"2019-04-11 16:50:02.000000000","message":"I don\u0027t know about changing the default to disabled. I view this as a step toward the future where eventually all resource usage data should come from placement. I understand where you\u0027re coming from though. If consensus thinks we should be more conservative and not take this step for most deployments at this time, I will change the default.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"00961009a5926c5e7d37dee727dd7347a8b22252","unresolved":false,"context_lines":[{"line_number":251,"context_line":"\"\"\"),"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    cfg.BoolOpt("},{"line_number":254,"context_line":"        \u0027disable_quota_usage_from_placement\u0027,"},{"line_number":255,"context_line":"        default\u003dFalse,"},{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_4daae785","line":254,"in_reply_to":"3fce034c_87266eb7","updated":"2019-04-11 18:34:53.000000000","message":"\u003e I don\u0027t know about changing the default to disabled. I view this as\n \u003e a step toward the future where eventually all resource usage data\n \u003e should come from placement. I understand where you\u0027re coming from\n \u003e though. If consensus thinks we should be more conservative and not\n \u003e take this step for most deployments at this time, I will change the\n \u003e default.\n\nThe quota usage behavior change for instances that are resized is another factor that I\u0027ve just thought about since reviewing the spec. I\u0027m not sure if that\u0027s a big enough reason though, but it\u0027s a change in behavior.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"cd1e609f2248309f0bddc91fd469ab038502b9c2","unresolved":false,"context_lines":[{"line_number":251,"context_line":"\"\"\"),"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    cfg.BoolOpt("},{"line_number":254,"context_line":"        \u0027disable_quota_usage_from_placement\u0027,"},{"line_number":255,"context_line":"        default\u003dFalse,"},{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_78d0bf51","line":254,"in_reply_to":"3fce034c_edef13ae","updated":"2019-04-11 19:15:43.000000000","message":"\u003e \u003e The quota usage behavior change for instances that are resized is\n \u003e another factor that I\u0027ve just thought about since reviewing the\n \u003e spec. I\u0027m not sure if that\u0027s a big enough reason though, but it\u0027s a\n \u003e change in behavior.\n \u003e \n \u003e Having said that, I don\u0027t know if we regressed that behavior in\n \u003e Pike with counting quotas or not, i.e. pre-counting did a resize\n \u003e create a reservation on the target host and then commit it after\n \u003e the resize was confirmed so that both resource usage from the\n \u003e ResourceTracker and GET /limits + reserved\u003d1 would reflect the same\n \u003e resource usage. If so, then this would actually get us back to that\n \u003e behavior. ^ would require testing in an ocata deployment to find\n \u003e out what the old behavior was.\n\nI don\u0027t think we regressed quota usage behavior from pre-Pike. Looking at the code, I see that at the beginning of a resize, it reserved the delta between old and new flavor, for an upsize:\n\nhttps://github.com/openstack/nova/blob/stable/ocata/nova/compute/api.py#L3351\n\n(Reminder, a reservation is a before quota is \"committed\". When a quota reservation gets committed, the reservation is added to the quota_usages table and then it\u0027s deleted from the reservations table. And total quota usage calculation is reservations + quota_usages).\n\nSo the quota usage pre-Pike would have been: upsize_delta + current usage (old flavor) \u003d new flavor.\n\nAnd the quota reservation was committed at finish_resize time, after setting the Instance.vcpus and Instance.memory_mb attributes with the new flavor:\n\nhttps://github.com/openstack/nova/blob/stable/ocata/nova/compute/manager.py#L4078\n\nBased on this, I think pre-counting and post-counting, the quota usage for an instance in VERIFY_RESIZE is new flavor.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    cfg.BoolOpt("},{"line_number":254,"context_line":"        \u0027disable_quota_usage_from_placement\u0027,"},{"line_number":255,"context_line":"        default\u003dFalse,"},{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_0f838906","line":255,"updated":"2019-04-10 20:13:55.000000000","message":"The spec said we\u0027d set this to True in one of our gate jobs:\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/train/approved/count-quota-usage-from-placement.html#testing\n\nBut I don\u0027t see that happening - it could be added to either this change or a following change in the series since this is already pretty big.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    cfg.BoolOpt("},{"line_number":254,"context_line":"        \u0027disable_quota_usage_from_placement\u0027,"},{"line_number":255,"context_line":"        default\u003dFalse,"},{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_8f2d39aa","line":255,"in_reply_to":"3fce034c_0f838906","updated":"2019-04-10 20:53:58.000000000","message":"Yes, it slipped my mind. Separate change is probably best.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0b1b304681101e954779dcecf70f2f003410a8d9","unresolved":false,"context_lines":[{"line_number":254,"context_line":"        \u0027disable_quota_usage_from_placement\u0027,"},{"line_number":255,"context_line":"        default\u003dFalse,"},{"line_number":256,"context_line":"        help\u003d\"\"\""},{"line_number":257,"context_line":"Disable the counting of quota usage from the placement service."},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"Starting in Train, quota usage for cores and ram is counted from the placement"},{"line_number":260,"context_line":"service instead of counting it locally in cell databases. This works well if"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_bcfd7fb2","line":257,"updated":"2019-04-15 23:32:10.000000000","message":"I just wanted to add a note here that based on further discussion on IRC [1], I\u0027m leaning toward making counting from placement opt-in, under the [quota] config section and will try it out in the next PS.\n\n[1] http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-04-11.log.html#t2019-04-11T21:28:34","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":261,"context_line":"there is only one Nova deployment running per placement deployment. However,"},{"line_number":262,"context_line":"if an operator is running more than one Nova deployment per placement"},{"line_number":263,"context_line":"deployment, they will need to set this workaround option to True because"},{"line_number":264,"context_line":"currently, the placement service has no way to partition resource allocations"},{"line_number":265,"context_line":"per Nova deployment. When this workaround option is set to True, Nova will fall"},{"line_number":266,"context_line":"back on the legacy counting method to count quota usage for instances, cores,"},{"line_number":267,"context_line":"and ram from its cell databases."}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_ecd88b80","line":264,"range":{"start_line":264,"start_character":9,"end_line":264,"end_character":10},"updated":"2019-04-10 20:13:55.000000000","message":"nit: I think you could nix this.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":263,"context_line":"deployment, they will need to set this workaround option to True because"},{"line_number":264,"context_line":"currently, the placement service has no way to partition resource allocations"},{"line_number":265,"context_line":"per Nova deployment. When this workaround option is set to True, Nova will fall"},{"line_number":266,"context_line":"back on the legacy counting method to count quota usage for instances, cores,"},{"line_number":267,"context_line":"and ram from its cell databases."},{"line_number":268,"context_line":"\"\"\"),"},{"line_number":269,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_ccc4af95","line":266,"range":{"start_line":266,"start_character":60,"end_line":266,"end_character":69},"updated":"2019-04-10 20:13:55.000000000","message":"nit: the first sentence only mentions cores and ram but does this option also apply to counting instances using the instance mapping count for the instances quota? Or will we always hit the API DB for that count regardless of this option?","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b09d6fc6eaba47890c33b013ec058203a5a1612f","unresolved":false,"context_lines":[{"line_number":263,"context_line":"deployment, they will need to set this workaround option to True because"},{"line_number":264,"context_line":"currently, the placement service has no way to partition resource allocations"},{"line_number":265,"context_line":"per Nova deployment. When this workaround option is set to True, Nova will fall"},{"line_number":266,"context_line":"back on the legacy counting method to count quota usage for instances, cores,"},{"line_number":267,"context_line":"and ram from its cell databases."},{"line_number":268,"context_line":"\"\"\"),"},{"line_number":269,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_c751d610","line":266,"range":{"start_line":266,"start_character":60,"end_line":266,"end_character":69},"in_reply_to":"3fce034c_380d5e90","updated":"2019-04-11 16:50:02.000000000","message":"\u003e \u003e And try moving it under [quotas].\n \u003e \n \u003e OK if you do that you should probably remember to update the spec\n \u003e later as well.\n\nIt\u0027s my intention to remember to update the spec. I know my memory isn\u0027t great, but I\u0027m trying.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":263,"context_line":"deployment, they will need to set this workaround option to True because"},{"line_number":264,"context_line":"currently, the placement service has no way to partition resource allocations"},{"line_number":265,"context_line":"per Nova deployment. When this workaround option is set to True, Nova will fall"},{"line_number":266,"context_line":"back on the legacy counting method to count quota usage for instances, cores,"},{"line_number":267,"context_line":"and ram from its cell databases."},{"line_number":268,"context_line":"\"\"\"),"},{"line_number":269,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_d211a455","line":266,"range":{"start_line":266,"start_character":60,"end_line":266,"end_character":69},"in_reply_to":"3fce034c_ccc4af95","updated":"2019-04-10 20:53:58.000000000","message":"The first sentence used to say \"instances, cores, ram\" and then I changed it based on an earlier review comment that instances are not counted from placement. I should have added another sentence about instance mappings in the API database being for the instances count.\n\nWe will not always hit the API DB for counting instances. If the option for disabling counting from placement is being used, we\u0027ll use cell databases for instances, cores, and ram (as they are all counted in the same database query together).\n\nI\u0027ll add clarification to this. And try moving it under [quotas].","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a602d33f033aecc8dc55c225b707a18ed90bb6aa","unresolved":false,"context_lines":[{"line_number":263,"context_line":"deployment, they will need to set this workaround option to True because"},{"line_number":264,"context_line":"currently, the placement service has no way to partition resource allocations"},{"line_number":265,"context_line":"per Nova deployment. When this workaround option is set to True, Nova will fall"},{"line_number":266,"context_line":"back on the legacy counting method to count quota usage for instances, cores,"},{"line_number":267,"context_line":"and ram from its cell databases."},{"line_number":268,"context_line":"\"\"\"),"},{"line_number":269,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_380d5e90","line":266,"range":{"start_line":266,"start_character":60,"end_line":266,"end_character":69},"in_reply_to":"3fce034c_d211a455","updated":"2019-04-11 14:00:06.000000000","message":"\u003e And try moving it under [quotas].\n\nOK if you do that you should probably remember to update the spec later as well.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"}],"nova/objects/instance_mapping.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b06ecac20df04770b902eb5ae4cf23fcebb45851","unresolved":false,"context_lines":[{"line_number":243,"context_line":"    :returns: True if user_id is set for all non-deleted instances, else False"},{"line_number":244,"context_line":"    \"\"\""},{"line_number":245,"context_line":"    query \u003d context.session.query("},{"line_number":246,"context_line":"        func.count(api_models.InstanceMapping.id)).\\"},{"line_number":247,"context_line":"        filter_by(user_id\u003dNone).\\"},{"line_number":248,"context_line":"        filter_by(queued_for_delete\u003dFalse)"},{"line_number":249,"context_line":"    if project_id:"}],"source_content_type":"text/x-python","patch_set":22,"id":"5fc1f717_2f30ea55","line":246,"range":{"start_line":246,"start_character":13,"end_line":246,"end_character":18},"updated":"2019-03-27 19:19:47.000000000","message":"I think this could be made much more efficient by using \u0027exists\u0027 instead.","commit_id":"eca06d5054e27ca68d7cf6cb9a6b124b63658ea6"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c90978be7431eb8b73fe6f8dac0b8bca86fc28ba","unresolved":false,"context_lines":[{"line_number":245,"context_line":"    SOFT_DELETED instances do not count against quota limits)."},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"    We also want to fall back to the legacy counting method if we detect any"},{"line_number":248,"context_line":"    records that have no yet populated the queued_for_delete field. We do this"},{"line_number":249,"context_line":"    instead of counting queued_for_delete\u003dNone records since that might not"},{"line_number":250,"context_line":"    accurately reflect the project or project user\u0027s quota usage."},{"line_number":251,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"5fc1f717_17fed304","line":248,"range":{"start_line":248,"start_character":22,"end_line":248,"end_character":24},"updated":"2019-04-04 17:25:09.000000000","message":"nit: not","commit_id":"f140b079481989aeea3c15109da953564abca4f2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":226,"context_line":""},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"@db_api.api_context_manager.reader"},{"line_number":229,"context_line":"def user_id_queued_for_delete_populated(context, project_id\u003dNone):"},{"line_number":230,"context_line":"    \"\"\"Determine whether user_id and queued_for_delete are set."},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    This will be used to determine whether we need to fall back on"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_ef795d2f","line":229,"range":{"start_line":229,"start_character":49,"end_line":229,"end_character":64},"updated":"2019-04-10 20:13:55.000000000","message":"I\u0027m not sure why this is optional - a project_id is always passed to this method right?","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":226,"context_line":""},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"@db_api.api_context_manager.reader"},{"line_number":229,"context_line":"def user_id_queued_for_delete_populated(context, project_id\u003dNone):"},{"line_number":230,"context_line":"    \"\"\"Determine whether user_id and queued_for_delete are set."},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    This will be used to determine whether we need to fall back on"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_f23700e3","line":229,"range":{"start_line":229,"start_character":49,"end_line":229,"end_character":64},"in_reply_to":"3fce034c_ef795d2f","updated":"2019-04-10 20:53:58.000000000","message":"No, in the last patch in the series, counting server group members from the API database using instance mappings, we\u0027re not scoped to a project and will check for unmigrated records without a project_id.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    This will be used to determine whether we need to fall back on"},{"line_number":233,"context_line":"    the legacy quota counting method (if we cannot rely on counting"},{"line_number":234,"context_line":"    instance mappings for the instance count). If any records with user_id\u003dNone"},{"line_number":235,"context_line":"    and queued_for_delete\u003dFalse are found, we need to fall back to the legacy"},{"line_number":236,"context_line":"    counting method. If any records with queued_for_delete\u003dNone are found, we"},{"line_number":237,"context_line":"    need to fall back to the legacy counting method."},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    Note that this check specifies queued_for_deleted\u003dFalse, which excludes"},{"line_number":240,"context_line":"    deleted and SOFT_DELETED instances. The \u0027populate_user_id\u0027 data migration"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_0c839706","line":237,"range":{"start_line":234,"start_character":47,"end_line":237,"end_character":52},"updated":"2019-04-10 20:13:55.000000000","message":"So if user_id is None but queued_for_delete is True, then we consider that record \u0027migrated\u0027 because of the reasoning in the next paragraph, right? Meaning this method (user_id_queued_for_delete_populated) is tightly coupled only to the instances count check and nothing else, which is why it gets to be a bit biased in what it considers migrated or not.\n\nMaybe this should live closer to that code if so...but I also haven\u0027t gotten to the code that uses this yet.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    This will be used to determine whether we need to fall back on"},{"line_number":233,"context_line":"    the legacy quota counting method (if we cannot rely on counting"},{"line_number":234,"context_line":"    instance mappings for the instance count). If any records with user_id\u003dNone"},{"line_number":235,"context_line":"    and queued_for_delete\u003dFalse are found, we need to fall back to the legacy"},{"line_number":236,"context_line":"    counting method. If any records with queued_for_delete\u003dNone are found, we"},{"line_number":237,"context_line":"    need to fall back to the legacy counting method."},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    Note that this check specifies queued_for_deleted\u003dFalse, which excludes"},{"line_number":240,"context_line":"    deleted and SOFT_DELETED instances. The \u0027populate_user_id\u0027 data migration"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_d268c4bb","line":237,"range":{"start_line":234,"start_character":47,"end_line":237,"end_character":52},"in_reply_to":"3fce034c_0c839706","updated":"2019-04-10 20:53:58.000000000","message":"No, any record with user_id None is considered unmigrated. I should just nix the \"and queued_for_delete\u003dFalse\" part, it\u0027s not the salient point. I\u0027m not sure why I wrote it like that in the first place.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b09d6fc6eaba47890c33b013ec058203a5a1612f","unresolved":false,"context_lines":[{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    This will be used to determine whether we need to fall back on"},{"line_number":233,"context_line":"    the legacy quota counting method (if we cannot rely on counting"},{"line_number":234,"context_line":"    instance mappings for the instance count). If any records with user_id\u003dNone"},{"line_number":235,"context_line":"    and queued_for_delete\u003dFalse are found, we need to fall back to the legacy"},{"line_number":236,"context_line":"    counting method. If any records with queued_for_delete\u003dNone are found, we"},{"line_number":237,"context_line":"    need to fall back to the legacy counting method."},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    Note that this check specifies queued_for_deleted\u003dFalse, which excludes"},{"line_number":240,"context_line":"    deleted and SOFT_DELETED instances. The \u0027populate_user_id\u0027 data migration"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_c7ed5626","line":237,"range":{"start_line":234,"start_character":47,"end_line":237,"end_character":52},"in_reply_to":"3fce034c_1824220d","updated":"2019-04-11 16:50:02.000000000","message":"Yeah, sorry. What you said in the first comment here is correct, this method is biased in what it considers migrated, because it only exists to deal with quota counting. With that in mind, I see what you mean about moving it. I would put it in the nova/quota.py file, that would probably be more appropriate.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"00961009a5926c5e7d37dee727dd7347a8b22252","unresolved":false,"context_lines":[{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    This will be used to determine whether we need to fall back on"},{"line_number":233,"context_line":"    the legacy quota counting method (if we cannot rely on counting"},{"line_number":234,"context_line":"    instance mappings for the instance count). If any records with user_id\u003dNone"},{"line_number":235,"context_line":"    and queued_for_delete\u003dFalse are found, we need to fall back to the legacy"},{"line_number":236,"context_line":"    counting method. If any records with queued_for_delete\u003dNone are found, we"},{"line_number":237,"context_line":"    need to fall back to the legacy counting method."},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    Note that this check specifies queued_for_deleted\u003dFalse, which excludes"},{"line_number":240,"context_line":"    deleted and SOFT_DELETED instances. The \u0027populate_user_id\u0027 data migration"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_edc5d35c","line":237,"range":{"start_line":234,"start_character":47,"end_line":237,"end_character":52},"in_reply_to":"3fce034c_c7ed5626","updated":"2019-04-11 18:34:53.000000000","message":"\u003e I would put it in the nova/quota.py file, that would probably be more appropriate.\n\nAgree that\u0027s where I was thinking as well.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a602d33f033aecc8dc55c225b707a18ed90bb6aa","unresolved":false,"context_lines":[{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    This will be used to determine whether we need to fall back on"},{"line_number":233,"context_line":"    the legacy quota counting method (if we cannot rely on counting"},{"line_number":234,"context_line":"    instance mappings for the instance count). If any records with user_id\u003dNone"},{"line_number":235,"context_line":"    and queued_for_delete\u003dFalse are found, we need to fall back to the legacy"},{"line_number":236,"context_line":"    counting method. If any records with queued_for_delete\u003dNone are found, we"},{"line_number":237,"context_line":"    need to fall back to the legacy counting method."},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    Note that this check specifies queued_for_deleted\u003dFalse, which excludes"},{"line_number":240,"context_line":"    deleted and SOFT_DELETED instances. The \u0027populate_user_id\u0027 data migration"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_1824220d","line":237,"range":{"start_line":234,"start_character":47,"end_line":237,"end_character":52},"in_reply_to":"3fce034c_d268c4bb","updated":"2019-04-11 14:00:06.000000000","message":"\u003e No, any record with user_id None is considered unmigrated. \n\nNot if I\u0027m reading this query filter correctly:\n\nand_(\n        api_models.InstanceMapping.user_id \u003d\u003d null(),\n        api_models.InstanceMapping.queued_for_delete \u003d\u003d false())","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":253,"context_line":"    :returns: True if user_id is set for all non-deleted instances and"},{"line_number":254,"context_line":"              queued_for_delete is set for all instances, else False"},{"line_number":255,"context_line":"    \"\"\""},{"line_number":256,"context_line":"    user_id_populated \u003d and_("},{"line_number":257,"context_line":"        api_models.InstanceMapping.user_id \u003d\u003d null(),"},{"line_number":258,"context_line":"        api_models.InstanceMapping.queued_for_delete \u003d\u003d false())"},{"line_number":259,"context_line":"    # If either queued_for_delete or user_id are unmigrated, we will return"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_cce58f0c","line":256,"range":{"start_line":256,"start_character":4,"end_line":256,"end_character":21},"updated":"2019-04-10 20:13:55.000000000","message":"user_id_not_populated?","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":253,"context_line":"    :returns: True if user_id is set for all non-deleted instances and"},{"line_number":254,"context_line":"              queued_for_delete is set for all instances, else False"},{"line_number":255,"context_line":"    \"\"\""},{"line_number":256,"context_line":"    user_id_populated \u003d and_("},{"line_number":257,"context_line":"        api_models.InstanceMapping.user_id \u003d\u003d null(),"},{"line_number":258,"context_line":"        api_models.InstanceMapping.queued_for_delete \u003d\u003d false())"},{"line_number":259,"context_line":"    # If either queued_for_delete or user_id are unmigrated, we will return"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_52837457","line":256,"range":{"start_line":256,"start_character":4,"end_line":256,"end_character":21},"in_reply_to":"3fce034c_cce58f0c","updated":"2019-04-10 20:53:58.000000000","message":"Argh, yeah. Oops.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"}],"nova/quota.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8b6707f3d84b0c4a8e9f09737919ef3ef55c3788","unresolved":false,"context_lines":[{"line_number":1194,"context_line":"                                          user_id\u003duser_id or \u0027N/A\u0027)"},{"line_number":1195,"context_line":""},{"line_number":1196,"context_line":""},{"line_number":1197,"context_line":"def _cores_ram_count_placement(context, project_id, user_id\u003dNone):"},{"line_number":1198,"context_line":"    total_counts \u003d {\u0027project\u0027: {}}"},{"line_number":1199,"context_line":"    global PLACEMENT_CLIENT"},{"line_number":1200,"context_line":"    if PLACEMENT_CLIENT is None:"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_804c643f","line":1197,"range":{"start_line":1197,"start_character":4,"end_line":1197,"end_character":30},"updated":"2019-02-27 17:44:09.000000000","message":"This needs to go into SchedulerReportClient itself.\n\nIn fact, we should (separately) make SchedulerReportClient.get/put/post/delete private to emphasize that pattern.","commit_id":"7b502a8ce395044fdcee9efcc5998aec836f24e1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"19ca5f4d6891f6bb0eeeb2f1d9a8488c8ea59623","unresolved":false,"context_lines":[{"line_number":1194,"context_line":"                                          user_id\u003duser_id or \u0027N/A\u0027)"},{"line_number":1195,"context_line":""},{"line_number":1196,"context_line":""},{"line_number":1197,"context_line":"def _cores_ram_count_placement(context, project_id, user_id\u003dNone):"},{"line_number":1198,"context_line":"    total_counts \u003d {\u0027project\u0027: {}}"},{"line_number":1199,"context_line":"    global PLACEMENT_CLIENT"},{"line_number":1200,"context_line":"    if PLACEMENT_CLIENT is None:"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_a00e4838","line":1197,"range":{"start_line":1197,"start_character":4,"end_line":1197,"end_character":30},"in_reply_to":"9fdfeff1_804c643f","updated":"2019-02-27 18:11:14.000000000","message":"Thanks for the feedback. I\u0027ll move it there.","commit_id":"7b502a8ce395044fdcee9efcc5998aec836f24e1"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2631a05c8cf46b38838df324619e89e2150e4c42","unresolved":false,"context_lines":[{"line_number":1207,"context_line":"        LOG.debug(\u0027Falling back to legacy quota counting method\u0027)"},{"line_number":1208,"context_line":"        return _instances_cores_ram_count_legacy(context, project_id,"},{"line_number":1209,"context_line":"                                                 user_id\u003duser_id)"},{"line_number":1210,"context_line":"    else:"},{"line_number":1211,"context_line":"        # Will return in format {\u0027project\u0027: {\u0027instances\u0027: M},"},{"line_number":1212,"context_line":"        #                        \u0027user\u0027: {\u0027instances\u0027: N}}"},{"line_number":1213,"context_line":"        # where the \u0027user\u0027 key is optional."}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_9c2ee840","line":1210,"range":{"start_line":1210,"start_character":4,"end_line":1210,"end_character":8},"updated":"2019-02-28 21:42:16.000000000","message":"nit: redundant","commit_id":"28c83029dddd88c1687e909731bb636a58d814d7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7bfd7b9c49b0df9510c3bc4ad178cc386417e1d8","unresolved":false,"context_lines":[{"line_number":1207,"context_line":"        LOG.debug(\u0027Falling back to legacy quota counting method\u0027)"},{"line_number":1208,"context_line":"        return _instances_cores_ram_count_legacy(context, project_id,"},{"line_number":1209,"context_line":"                                                 user_id\u003duser_id)"},{"line_number":1210,"context_line":"    else:"},{"line_number":1211,"context_line":"        # Will return in format {\u0027project\u0027: {\u0027instances\u0027: M},"},{"line_number":1212,"context_line":"        #                        \u0027user\u0027: {\u0027instances\u0027: N}}"},{"line_number":1213,"context_line":"        # where the \u0027user\u0027 key is optional."}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_dc8eb0cf","line":1210,"range":{"start_line":1210,"start_character":4,"end_line":1210,"end_character":8},"in_reply_to":"9fdfeff1_9c2ee840","updated":"2019-02-28 22:02:20.000000000","message":"I thought it looked nice. Maybe it doesn\u0027t.","commit_id":"28c83029dddd88c1687e909731bb636a58d814d7"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"4f228ea72f93f01d80b8500cabe6b57558d6d639","unresolved":false,"context_lines":[{"line_number":1207,"context_line":"        LOG.debug(\u0027Falling back to legacy quota counting method\u0027)"},{"line_number":1208,"context_line":"        return _instances_cores_ram_count_legacy(context, project_id,"},{"line_number":1209,"context_line":"                                                 user_id\u003duser_id)"},{"line_number":1210,"context_line":"    else:"},{"line_number":1211,"context_line":"        # Will return in format {\u0027project\u0027: {\u0027instances\u0027: M},"},{"line_number":1212,"context_line":"        #                        \u0027user\u0027: {\u0027instances\u0027: N}}"},{"line_number":1213,"context_line":"        # where the \u0027user\u0027 key is optional."}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_32ada530","line":1210,"range":{"start_line":1210,"start_character":4,"end_line":1210,"end_character":8},"in_reply_to":"9fdfeff1_dc8eb0cf","updated":"2019-03-01 00:03:49.000000000","message":"It\u0027s really just a style preference, feel free to ignore.","commit_id":"28c83029dddd88c1687e909731bb636a58d814d7"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"a5358d6797c8a5cd893c6371358372f9666e5e9c","unresolved":false,"context_lines":[{"line_number":1122,"context_line":""},{"line_number":1123,"context_line":""},{"line_number":1124,"context_line":"def _instances_cores_ram_count(context, project_id, user_id\u003dNone):"},{"line_number":1125,"context_line":"    \"\"\"Get the counts of instances, cores, and ram in the database."},{"line_number":1126,"context_line":""},{"line_number":1127,"context_line":"    :param context: The request context for database access"},{"line_number":1128,"context_line":"    :param project_id: The project_id to count across"},{"line_number":1129,"context_line":"    :param user_id: The user_id to count across"},{"line_number":1130,"context_line":"    :returns: A dict containing the project-scoped counts and user-scoped"},{"line_number":1131,"context_line":"              counts if user_id is specified. For example:"},{"line_number":1132,"context_line":""},{"line_number":1133,"context_line":"                {\u0027project\u0027: {\u0027instances\u0027: \u003ccount across project\u003e,"},{"line_number":1134,"context_line":"                             \u0027cores\u0027: \u003ccount across project\u003e,"},{"line_number":1135,"context_line":"                             \u0027ram\u0027: \u003ccount across project\u003e},"},{"line_number":1136,"context_line":"                 \u0027user\u0027: {\u0027instances\u0027: \u003ccount across user\u003e,"},{"line_number":1137,"context_line":"                          \u0027cores\u0027: \u003ccount across user\u003e,"},{"line_number":1138,"context_line":"                          \u0027ram\u0027: \u003ccount across user\u003e}}"},{"line_number":1139,"context_line":"    \"\"\""},{"line_number":1140,"context_line":"    # TODO(melwitt): Counting across cells for instances means we will miss"},{"line_number":1141,"context_line":"    # counting resources if a cell is down. In the future, we should query"},{"line_number":1142,"context_line":"    # placement for cores/ram and InstanceMappings for instances (once we are"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_3e7a2724","side":"PARENT","line":1139,"range":{"start_line":1125,"start_character":0,"end_line":1139,"end_character":7},"updated":"2019-03-06 09:30:53.000000000","message":"why are we removing this nice doc string?","commit_id":"d07d177d42751fe2b911f371e8d121f5e6ab4887"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9253af62dc7823eaf680f8bd91317e82a5ca805e","unresolved":false,"context_lines":[{"line_number":1122,"context_line":""},{"line_number":1123,"context_line":""},{"line_number":1124,"context_line":"def _instances_cores_ram_count(context, project_id, user_id\u003dNone):"},{"line_number":1125,"context_line":"    \"\"\"Get the counts of instances, cores, and ram in the database."},{"line_number":1126,"context_line":""},{"line_number":1127,"context_line":"    :param context: The request context for database access"},{"line_number":1128,"context_line":"    :param project_id: The project_id to count across"},{"line_number":1129,"context_line":"    :param user_id: The user_id to count across"},{"line_number":1130,"context_line":"    :returns: A dict containing the project-scoped counts and user-scoped"},{"line_number":1131,"context_line":"              counts if user_id is specified. For example:"},{"line_number":1132,"context_line":""},{"line_number":1133,"context_line":"                {\u0027project\u0027: {\u0027instances\u0027: \u003ccount across project\u003e,"},{"line_number":1134,"context_line":"                             \u0027cores\u0027: \u003ccount across project\u003e,"},{"line_number":1135,"context_line":"                             \u0027ram\u0027: \u003ccount across project\u003e},"},{"line_number":1136,"context_line":"                 \u0027user\u0027: {\u0027instances\u0027: \u003ccount across user\u003e,"},{"line_number":1137,"context_line":"                          \u0027cores\u0027: \u003ccount across user\u003e,"},{"line_number":1138,"context_line":"                          \u0027ram\u0027: \u003ccount across user\u003e}}"},{"line_number":1139,"context_line":"    \"\"\""},{"line_number":1140,"context_line":"    # TODO(melwitt): Counting across cells for instances means we will miss"},{"line_number":1141,"context_line":"    # counting resources if a cell is down. In the future, we should query"},{"line_number":1142,"context_line":"    # placement for cores/ram and InstanceMappings for instances (once we are"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_4054dcca","side":"PARENT","line":1139,"range":{"start_line":1125,"start_character":0,"end_line":1139,"end_character":7},"in_reply_to":"5fc1f717_0514a69f","updated":"2019-03-06 23:18:42.000000000","message":"I don\u0027t consider the docstring moved to the non-legacy method. It\u0027s staying on _instances_cores_ram_count, the same method. It\u0027s just that same method has become a wrapper that either calls the new _legacy method or placement. I didn\u0027t think it helpful to duplicate the docstring to the legacy method, but I can if you think it would be helpful.","commit_id":"d07d177d42751fe2b911f371e8d121f5e6ab4887"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9174d534f7698244bab8719c085ac5658ea1334f","unresolved":false,"context_lines":[{"line_number":1122,"context_line":""},{"line_number":1123,"context_line":""},{"line_number":1124,"context_line":"def _instances_cores_ram_count(context, project_id, user_id\u003dNone):"},{"line_number":1125,"context_line":"    \"\"\"Get the counts of instances, cores, and ram in the database."},{"line_number":1126,"context_line":""},{"line_number":1127,"context_line":"    :param context: The request context for database access"},{"line_number":1128,"context_line":"    :param project_id: The project_id to count across"},{"line_number":1129,"context_line":"    :param user_id: The user_id to count across"},{"line_number":1130,"context_line":"    :returns: A dict containing the project-scoped counts and user-scoped"},{"line_number":1131,"context_line":"              counts if user_id is specified. For example:"},{"line_number":1132,"context_line":""},{"line_number":1133,"context_line":"                {\u0027project\u0027: {\u0027instances\u0027: \u003ccount across project\u003e,"},{"line_number":1134,"context_line":"                             \u0027cores\u0027: \u003ccount across project\u003e,"},{"line_number":1135,"context_line":"                             \u0027ram\u0027: \u003ccount across project\u003e},"},{"line_number":1136,"context_line":"                 \u0027user\u0027: {\u0027instances\u0027: \u003ccount across user\u003e,"},{"line_number":1137,"context_line":"                          \u0027cores\u0027: \u003ccount across user\u003e,"},{"line_number":1138,"context_line":"                          \u0027ram\u0027: \u003ccount across user\u003e}}"},{"line_number":1139,"context_line":"    \"\"\""},{"line_number":1140,"context_line":"    # TODO(melwitt): Counting across cells for instances means we will miss"},{"line_number":1141,"context_line":"    # counting resources if a cell is down. In the future, we should query"},{"line_number":1142,"context_line":"    # placement for cores/ram and InstanceMappings for instances (once we are"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_0514a69f","side":"PARENT","line":1139,"range":{"start_line":1125,"start_character":0,"end_line":1139,"end_character":7},"in_reply_to":"5fc1f717_3e7a2724","updated":"2019-03-06 22:44:42.000000000","message":"It\u0027s moved to the new (non-legacy) method. I kind of agree it would be nice to preserve it, but also don\u0027t know that it should be duplicated. Perhaps just replace it with\n\n \"\"\"See the _instances_cores_ram_count docstring.\"\"\"","commit_id":"d07d177d42751fe2b911f371e8d121f5e6ab4887"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c90978be7431eb8b73fe6f8dac0b8bca86fc28ba","unresolved":false,"context_lines":[{"line_number":1154,"context_line":"def _cores_ram_count_placement(context, project_id, user_id\u003dNone):"},{"line_number":1155,"context_line":"    global PLACEMENT_CLIENT"},{"line_number":1156,"context_line":"    if PLACEMENT_CLIENT is None:"},{"line_number":1157,"context_line":"        PLACEMENT_CLIENT \u003d report.SchedulerReportClient()"},{"line_number":1158,"context_line":"    return PLACEMENT_CLIENT.get_usages_counts_for_quota(context, project_id,"},{"line_number":1159,"context_line":"                                                        user_id\u003duser_id)"},{"line_number":1160,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"5fc1f717_97ade3eb","line":1157,"updated":"2019-04-04 17:25:09.000000000","message":"Is there a cost associated with creating the client that needs to be avoided? or some state that it is important to manage? I guess ksa lookups might be a concern.\n\nIdeally the report client would be an easy one shot kind of thing.","commit_id":"f140b079481989aeea3c15109da953564abca4f2"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d573ba0182f4b8ebdad8788d6aa9126ad208323","unresolved":false,"context_lines":[{"line_number":1154,"context_line":"def _cores_ram_count_placement(context, project_id, user_id\u003dNone):"},{"line_number":1155,"context_line":"    global PLACEMENT_CLIENT"},{"line_number":1156,"context_line":"    if PLACEMENT_CLIENT is None:"},{"line_number":1157,"context_line":"        PLACEMENT_CLIENT \u003d report.SchedulerReportClient()"},{"line_number":1158,"context_line":"    return PLACEMENT_CLIENT.get_usages_counts_for_quota(context, project_id,"},{"line_number":1159,"context_line":"                                                        user_id\u003duser_id)"},{"line_number":1160,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"5fc1f717_34a06df3","line":1157,"in_reply_to":"5fc1f717_97ade3eb","updated":"2019-04-09 01:54:22.000000000","message":"I\u0027m actually not 100% sure. I was basing it on existing usage of SchedulerReportClient, for example:\n\nhttps://github.com/openstack/nova/blob/fb1fee6772bb101eac83845bac9022df77113aaa/nova/compute/api.py#L2208\n\nI can ask efried if ksa things could be a concern.","commit_id":"f140b079481989aeea3c15109da953564abca4f2"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"20d40deb4ce180f3aadee6143f11785e9b6aedf7","unresolved":false,"context_lines":[{"line_number":1177,"context_line":"    \"\"\""},{"line_number":1178,"context_line":"    if (CONF.workarounds.disable_quota_usage_from_placement or"},{"line_number":1179,"context_line":"            not instance_mapping_obj.user_id_queued_for_delete_populated("},{"line_number":1180,"context_line":"                context, project_id)):"},{"line_number":1181,"context_line":"        LOG.debug(\u0027Falling back to legacy quota counting method\u0027)"},{"line_number":1182,"context_line":"        return _instances_cores_ram_count_legacy(context, project_id,"},{"line_number":1183,"context_line":"                                                 user_id\u003duser_id)"}],"source_content_type":"text/x-python","patch_set":23,"id":"5fc1f717_71233557","line":1180,"updated":"2019-03-28 01:36:57.000000000","message":"Note to self: I think I\u0027m missing test coverage for this logic. Need to write tests where the counting methods are mocked and assert the right ones are called depending on the config option and *populated method results.","commit_id":"f140b079481989aeea3c15109da953564abca4f2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":1122,"context_line":""},{"line_number":1123,"context_line":""},{"line_number":1124,"context_line":"def _instances_cores_ram_count(context, project_id, user_id\u003dNone):"},{"line_number":1125,"context_line":"    \"\"\"Get the counts of instances, cores, and ram in the database."},{"line_number":1126,"context_line":""},{"line_number":1127,"context_line":"    :param context: The request context for database access"},{"line_number":1128,"context_line":"    :param project_id: The project_id to count across"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_6c0a5b8b","side":"PARENT","line":1125,"updated":"2019-04-10 20:13:55.000000000","message":"This docstring didn\u0027t really need to go away did it? It could have been updated to note what legacy means.","commit_id":"7d4abaa8b5d83fd3d095c431e1d3cef0827b6d4e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":1122,"context_line":""},{"line_number":1123,"context_line":""},{"line_number":1124,"context_line":"def _instances_cores_ram_count(context, project_id, user_id\u003dNone):"},{"line_number":1125,"context_line":"    \"\"\"Get the counts of instances, cores, and ram in the database."},{"line_number":1126,"context_line":""},{"line_number":1127,"context_line":"    :param context: The request context for database access"},{"line_number":1128,"context_line":"    :param project_id: The project_id to count across"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_9290fc97","side":"PARENT","line":1125,"in_reply_to":"3fce034c_6c0a5b8b","updated":"2019-04-10 20:53:58.000000000","message":"It didn\u0027t go away, it stays on the _instances_cores_ram_count method. But everyone is getting confused by how it looks in the diff. It\u0027s valid for both the legacy count and the placement count, so it should be on the main method that chooses between them. Is there value in having it in two places? Based on the repeated comments, I guess so. So I\u0027ll just have it be mostly copied in both places.","commit_id":"7d4abaa8b5d83fd3d095c431e1d3cef0827b6d4e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":1122,"context_line":""},{"line_number":1123,"context_line":""},{"line_number":1124,"context_line":"def _instances_cores_ram_count_legacy(context, project_id, user_id\u003dNone):"},{"line_number":1125,"context_line":"    # TODO(melwitt): Counting across cells for instances means we will miss"},{"line_number":1126,"context_line":"    # counting resources if a cell is down. In the future, we should query"},{"line_number":1127,"context_line":"    # placement for cores/ram and InstanceMappings for instances (once we are"},{"line_number":1128,"context_line":"    # deleting InstanceMappings when we delete instances)."},{"line_number":1129,"context_line":"    # NOTE(tssurya): We only go into those cells in which the tenant has"},{"line_number":1130,"context_line":"    # instances. We could optimize this to avoid the CellMappingList query"},{"line_number":1131,"context_line":"    # for single-cell deployments by checking the cell cache and only doing"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_eccc0b63","line":1128,"range":{"start_line":1125,"start_character":4,"end_line":1128,"end_character":58},"updated":"2019-04-10 20:13:55.000000000","message":"Seems this should be removed or updated.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":1122,"context_line":""},{"line_number":1123,"context_line":""},{"line_number":1124,"context_line":"def _instances_cores_ram_count_legacy(context, project_id, user_id\u003dNone):"},{"line_number":1125,"context_line":"    # TODO(melwitt): Counting across cells for instances means we will miss"},{"line_number":1126,"context_line":"    # counting resources if a cell is down. In the future, we should query"},{"line_number":1127,"context_line":"    # placement for cores/ram and InstanceMappings for instances (once we are"},{"line_number":1128,"context_line":"    # deleting InstanceMappings when we delete instances)."},{"line_number":1129,"context_line":"    # NOTE(tssurya): We only go into those cells in which the tenant has"},{"line_number":1130,"context_line":"    # instances. We could optimize this to avoid the CellMappingList query"},{"line_number":1131,"context_line":"    # for single-cell deployments by checking the cell cache and only doing"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_12f2cc07","line":1128,"range":{"start_line":1125,"start_character":4,"end_line":1128,"end_character":58},"in_reply_to":"3fce034c_eccc0b63","updated":"2019-04-10 20:53:58.000000000","message":"Yeah, I\u0027ll update it. I think it should still mention down cell limitation in some way.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":1151,"context_line":""},{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"def _cores_ram_count_placement(context, project_id, user_id\u003dNone):"},{"line_number":1154,"context_line":"    placementclient \u003d report.SchedulerReportClient()"},{"line_number":1155,"context_line":"    return placementclient.get_usages_counts_for_quota(context, project_id,"},{"line_number":1156,"context_line":"                                                       user_id\u003duser_id)"},{"line_number":1157,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_8c7547fd","line":1154,"range":{"start_line":1154,"start_character":4,"end_line":1154,"end_character":52},"updated":"2019-04-10 20:13:55.000000000","message":"Kind of sucks that we have to construct this object for every quota check during server create or resize with the new flow, this thing isn\u0027t exactly cheap since it constructs a ProviderTree object and KSA adapter. The former we don\u0027t care about the latter we definitely do. Could we just make this a singleton in this module?","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"6367c7e585632a3b1b90fed07fb4c9a2ff987ace","unresolved":false,"context_lines":[{"line_number":1151,"context_line":""},{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"def _cores_ram_count_placement(context, project_id, user_id\u003dNone):"},{"line_number":1154,"context_line":"    placementclient \u003d report.SchedulerReportClient()"},{"line_number":1155,"context_line":"    return placementclient.get_usages_counts_for_quota(context, project_id,"},{"line_number":1156,"context_line":"                                                       user_id\u003duser_id)"},{"line_number":1157,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_ed7a9e86","line":1154,"range":{"start_line":1154,"start_character":4,"end_line":1154,"end_character":52},"in_reply_to":"3fce034c_12cbaca5","updated":"2019-04-12 16:48:18.000000000","message":"\u003e I had this as a singleton earlier but cdent suggested it might be\n \u003e inexpensive enough to create each time. I can change it back,\n \u003e except with a lock around it to prevent racing API workers creating\n \u003e more than one.\n\nSorry, I was probably a bit misleading in my comment. I wasn\u0027t saying \"it is inexpensive\" but rather asking \"is it expensive? if not, may as well not bother\". That it is expensive (apparently) remains sad to me. A client, in and of itself, should not carry state, it should manipulate state.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a602d33f033aecc8dc55c225b707a18ed90bb6aa","unresolved":false,"context_lines":[{"line_number":1151,"context_line":""},{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"def _cores_ram_count_placement(context, project_id, user_id\u003dNone):"},{"line_number":1154,"context_line":"    placementclient \u003d report.SchedulerReportClient()"},{"line_number":1155,"context_line":"    return placementclient.get_usages_counts_for_quota(context, project_id,"},{"line_number":1156,"context_line":"                                                       user_id\u003duser_id)"},{"line_number":1157,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_18698224","line":1154,"range":{"start_line":1154,"start_character":4,"end_line":1154,"end_character":52},"in_reply_to":"3fce034c_12cbaca5","updated":"2019-04-11 14:00:06.000000000","message":"All we care about in the report client is the KSA adapter, so I really don\u0027t think we need to be constructing this every time, along with the provider tree and all that. Singleton should be fine, and I don\u0027t even think a lock matters since we don\u0027t care about the state of the object, but if you want to add one that\u0027s fine.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b09d6fc6eaba47890c33b013ec058203a5a1612f","unresolved":false,"context_lines":[{"line_number":1151,"context_line":""},{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"def _cores_ram_count_placement(context, project_id, user_id\u003dNone):"},{"line_number":1154,"context_line":"    placementclient \u003d report.SchedulerReportClient()"},{"line_number":1155,"context_line":"    return placementclient.get_usages_counts_for_quota(context, project_id,"},{"line_number":1156,"context_line":"                                                       user_id\u003duser_id)"},{"line_number":1157,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_473de6b9","line":1154,"range":{"start_line":1154,"start_character":4,"end_line":1154,"end_character":52},"in_reply_to":"3fce034c_18698224","updated":"2019-04-11 16:50:02.000000000","message":"OK, I was thinking about leaking clients with multiple created, but I guess that doesn\u0027t matter in python-land.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0501e0e28825449801553b87388fad4ec3f4c52d","unresolved":false,"context_lines":[{"line_number":1151,"context_line":""},{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"def _cores_ram_count_placement(context, project_id, user_id\u003dNone):"},{"line_number":1154,"context_line":"    placementclient \u003d report.SchedulerReportClient()"},{"line_number":1155,"context_line":"    return placementclient.get_usages_counts_for_quota(context, project_id,"},{"line_number":1156,"context_line":"                                                       user_id\u003duser_id)"},{"line_number":1157,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_66f80bc4","line":1154,"range":{"start_line":1154,"start_character":4,"end_line":1154,"end_character":52},"in_reply_to":"3fce034c_6d7a2e25","updated":"2019-04-12 20:30:52.000000000","message":"We don\u0027t build the ProviderTree except to initialize the object, which is very cheap. We only go out and fetch all the things if one of the _ensure methods is called, which doesn\u0027t happen here.\n\nSo the expense is just building the ksa adapter. The cost of this can vary depending how the admin set up their conf options. I\u0027m pretty sure the recommended/typical setup will go query endpoints for version documents and whatnot. That\u0027s not super expensive, but it\u0027s calls over the wire, which aren\u0027t free.\n\nBut whether it\u0027s expensive or not, I would advocate using a singleton. In fact, I\u0027d like to make that happen in report.py itself so *everyone* (in the same process) gets the same instance. (I see I\u0027m not the only one who feels that way [1], though others may disagree [2].)\n\n[1] https://github.com/openstack/nova/blob/03322bb517925a9f5a04ebdb41c3fd31e7962440/nova/scheduler/client/report.py#L178-L181\n[2] https://review.openstack.org/#/c/631242/","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":1151,"context_line":""},{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"def _cores_ram_count_placement(context, project_id, user_id\u003dNone):"},{"line_number":1154,"context_line":"    placementclient \u003d report.SchedulerReportClient()"},{"line_number":1155,"context_line":"    return placementclient.get_usages_counts_for_quota(context, project_id,"},{"line_number":1156,"context_line":"                                                       user_id\u003duser_id)"},{"line_number":1157,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_12cbaca5","line":1154,"range":{"start_line":1154,"start_character":4,"end_line":1154,"end_character":52},"in_reply_to":"3fce034c_8c7547fd","updated":"2019-04-10 20:53:58.000000000","message":"I had this as a singleton earlier but cdent suggested it might be inexpensive enough to create each time. I can change it back, except with a lock around it to prevent racing API workers creating more than one.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f3b2b1b807efab4c88a6c860373a5b787242ecb","unresolved":false,"context_lines":[{"line_number":1151,"context_line":""},{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"def _cores_ram_count_placement(context, project_id, user_id\u003dNone):"},{"line_number":1154,"context_line":"    placementclient \u003d report.SchedulerReportClient()"},{"line_number":1155,"context_line":"    return placementclient.get_usages_counts_for_quota(context, project_id,"},{"line_number":1156,"context_line":"                                                       user_id\u003duser_id)"},{"line_number":1157,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_6d7a2e25","line":1154,"range":{"start_line":1154,"start_character":4,"end_line":1154,"end_character":52},"in_reply_to":"3fce034c_ed7a9e86","updated":"2019-04-12 17:15:31.000000000","message":"\u003e \u003e I had this as a singleton earlier but cdent suggested it might be\n \u003e \u003e inexpensive enough to create each time. I can change it back,\n \u003e \u003e except with a lock around it to prevent racing API workers\n \u003e creating\n \u003e \u003e more than one.\n \u003e \n \u003e Sorry, I was probably a bit misleading in my comment. I wasn\u0027t\n \u003e saying \"it is inexpensive\" but rather asking \"is it expensive? if\n \u003e not, may as well not bother\". That it is expensive (apparently)\n \u003e remains sad to me. A client, in and of itself, should not carry\n \u003e state, it should manipulate state.\n\nIt\u0027s not you, it\u0027s me, I\u0027m sure. I think I misunderstood what you meant.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":1157,"context_line":""},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":"def _instances_cores_ram_count(context, project_id, user_id\u003dNone):"},{"line_number":1160,"context_line":"    \"\"\"Get the counts of instances, cores, and ram in the database."},{"line_number":1161,"context_line":""},{"line_number":1162,"context_line":"    :param context: The request context for database access"},{"line_number":1163,"context_line":"    :param project_id: The project_id to count across"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_0cff1768","line":1160,"range":{"start_line":1160,"start_character":51,"end_line":1160,"end_character":67},"updated":"2019-04-10 20:13:55.000000000","message":"This was copied but is not entirely true, seems the docstring should be updated.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":1157,"context_line":""},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":"def _instances_cores_ram_count(context, project_id, user_id\u003dNone):"},{"line_number":1160,"context_line":"    \"\"\"Get the counts of instances, cores, and ram in the database."},{"line_number":1161,"context_line":""},{"line_number":1162,"context_line":"    :param context: The request context for database access"},{"line_number":1163,"context_line":"    :param project_id: The project_id to count across"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_f2c14082","line":1160,"range":{"start_line":1160,"start_character":51,"end_line":1160,"end_character":67},"in_reply_to":"3fce034c_0cff1768","updated":"2019-04-10 20:53:58.000000000","message":"True, will update.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":1173,"context_line":"                          \u0027ram\u0027: \u003ccount across user\u003e}}"},{"line_number":1174,"context_line":"    \"\"\""},{"line_number":1175,"context_line":"    if (CONF.workarounds.disable_quota_usage_from_placement or"},{"line_number":1176,"context_line":"            not instance_mapping_obj.user_id_queued_for_delete_populated("},{"line_number":1177,"context_line":"                context, project_id)):"},{"line_number":1178,"context_line":"        LOG.debug(\u0027Falling back to legacy quota counting method for \u0027"},{"line_number":1179,"context_line":"                  \u0027instances, cores, and ram\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_6c2fbbf6","line":1176,"range":{"start_line":1176,"start_character":16,"end_line":1176,"end_character":72},"updated":"2019-04-10 20:13:55.000000000","message":"According to the spec I thought we were going to cache this result:\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/train/approved/count-quota-usage-from-placement.html#upgrade-impact\n\nAt least if we detected the project is fully migrated and we don\u0027t need to check it each time. I guess that could be a TODO to follow up. But I thought the idea was that if my project is fully migrated you only need to check this once and cache it rather than check every time. If the project isn\u0027t migrated then we need to continue checking until it is.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":1173,"context_line":"                          \u0027ram\u0027: \u003ccount across user\u003e}}"},{"line_number":1174,"context_line":"    \"\"\""},{"line_number":1175,"context_line":"    if (CONF.workarounds.disable_quota_usage_from_placement or"},{"line_number":1176,"context_line":"            not instance_mapping_obj.user_id_queued_for_delete_populated("},{"line_number":1177,"context_line":"                context, project_id)):"},{"line_number":1178,"context_line":"        LOG.debug(\u0027Falling back to legacy quota counting method for \u0027"},{"line_number":1179,"context_line":"                  \u0027instances, cores, and ram\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_12408c07","line":1176,"range":{"start_line":1176,"start_character":16,"end_line":1176,"end_character":72},"in_reply_to":"3fce034c_6c2fbbf6","updated":"2019-04-10 20:53:58.000000000","message":"Oh right. That slipped my mind, sorry. I need to add that.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ec60024325e79f455553ff11b416a39de0ae1e74","unresolved":false,"context_lines":[{"line_number":1261,"context_line":"    # Will return a dict with format: {\u0027project\u0027: {\u0027instances\u0027: M},"},{"line_number":1262,"context_line":"    #                                  \u0027user\u0027: {\u0027instances\u0027: N}}"},{"line_number":1263,"context_line":"    # where the \u0027user\u0027 key is optional."},{"line_number":1264,"context_line":"    total_counts \u003d objects.InstanceMappingList.get_counts(context,"},{"line_number":1265,"context_line":"                                                          project_id,"},{"line_number":1266,"context_line":"                                                          user_id\u003duser_id)"},{"line_number":1267,"context_line":"    cores_ram_counts \u003d _cores_ram_count_placement(context, project_id,"}],"source_content_type":"text/x-python","patch_set":27,"id":"3fce034c_cedbb579","line":1264,"range":{"start_line":1264,"start_character":4,"end_line":1264,"end_character":16},"updated":"2019-04-18 13:04:33.000000000","message":"micronit: this name confused me for a second, but I have little to offer other than calling it \"instance_counts\" with a later rename, but the comment helps.","commit_id":"40a4a1abd894d11a53d596df70cd3184541f87d1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ec60024325e79f455553ff11b416a39de0ae1e74","unresolved":false,"context_lines":[{"line_number":1261,"context_line":"    # Will return a dict with format: {\u0027project\u0027: {\u0027instances\u0027: M},"},{"line_number":1262,"context_line":"    #                                  \u0027user\u0027: {\u0027instances\u0027: N}}"},{"line_number":1263,"context_line":"    # where the \u0027user\u0027 key is optional."},{"line_number":1264,"context_line":"    total_counts \u003d objects.InstanceMappingList.get_counts(context,"},{"line_number":1265,"context_line":"                                                          project_id,"},{"line_number":1266,"context_line":"                                                          user_id\u003duser_id)"},{"line_number":1267,"context_line":"    cores_ram_counts \u003d _cores_ram_count_placement(context, project_id,"},{"line_number":1268,"context_line":"                                                  user_id\u003duser_id)"},{"line_number":1269,"context_line":"    total_counts[\u0027project\u0027].update(cores_ram_counts[\u0027project\u0027])"},{"line_number":1270,"context_line":"    if \u0027user\u0027 in total_counts:"},{"line_number":1271,"context_line":"        total_counts[\u0027user\u0027].update(cores_ram_counts[\u0027user\u0027])"},{"line_number":1272,"context_line":"    return total_counts"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":""},{"line_number":1275,"context_line":"def _server_group_count(context, project_id, user_id\u003dNone):"}],"source_content_type":"text/x-python","patch_set":27,"id":"3fce034c_eea231fc","line":1272,"range":{"start_line":1264,"start_character":0,"end_line":1272,"end_character":23},"updated":"2019-04-18 13:04:33.000000000","message":"Nit...\n\nI would be tempted to push this into some helper function and invert that if statement.\n\nIt would make the test test_instances_cores_ram_count a bit simpler I suspect, and give the test only one \"reason to change\".","commit_id":"40a4a1abd894d11a53d596df70cd3184541f87d1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0d2334c5de2aeb023ea08c87aa7f21b22553109c","unresolved":false,"context_lines":[{"line_number":1261,"context_line":"    # Will return a dict with format: {\u0027project\u0027: {\u0027instances\u0027: M},"},{"line_number":1262,"context_line":"    #                                  \u0027user\u0027: {\u0027instances\u0027: N}}"},{"line_number":1263,"context_line":"    # where the \u0027user\u0027 key is optional."},{"line_number":1264,"context_line":"    total_counts \u003d objects.InstanceMappingList.get_counts(context,"},{"line_number":1265,"context_line":"                                                          project_id,"},{"line_number":1266,"context_line":"                                                          user_id\u003duser_id)"},{"line_number":1267,"context_line":"    cores_ram_counts \u003d _cores_ram_count_placement(context, project_id,"},{"line_number":1268,"context_line":"                                                  user_id\u003duser_id)"},{"line_number":1269,"context_line":"    total_counts[\u0027project\u0027].update(cores_ram_counts[\u0027project\u0027])"},{"line_number":1270,"context_line":"    if \u0027user\u0027 in total_counts:"},{"line_number":1271,"context_line":"        total_counts[\u0027user\u0027].update(cores_ram_counts[\u0027user\u0027])"},{"line_number":1272,"context_line":"    return total_counts"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":""},{"line_number":1275,"context_line":"def _server_group_count(context, project_id, user_id\u003dNone):"}],"source_content_type":"text/x-python","patch_set":27,"id":"3fce034c_c83de52b","line":1272,"range":{"start_line":1264,"start_character":0,"end_line":1272,"end_character":23},"in_reply_to":"3fce034c_eea231fc","updated":"2019-04-18 17:41:21.000000000","message":"I like your suggestion -- working on changing this around now.","commit_id":"40a4a1abd894d11a53d596df70cd3184541f87d1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f5292c95e53329d09f3fba8940b5e0ab13e82f2b","unresolved":false,"context_lines":[{"line_number":1061,"context_line":"        return 0"},{"line_number":1062,"context_line":""},{"line_number":1063,"context_line":""},{"line_number":1064,"context_line":"@db_api.api_context_manager.reader"},{"line_number":1065,"context_line":"def _user_id_queued_for_delete_populated(context, project_id\u003dNone):"},{"line_number":1066,"context_line":"    \"\"\"Determine whether user_id and queued_for_delete are set."},{"line_number":1067,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_7bc82b21","line":1064,"updated":"2019-05-13 18:39:09.000000000","message":"Nit: I would be tempted to put this on the instance mapping object (although maybe that is just because I want to use it independently from the unified limits code)","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e9b9ee23a6420c087db662395d26768c7e28717b","unresolved":false,"context_lines":[{"line_number":1061,"context_line":"        return 0"},{"line_number":1062,"context_line":""},{"line_number":1063,"context_line":""},{"line_number":1064,"context_line":"@db_api.api_context_manager.reader"},{"line_number":1065,"context_line":"def _user_id_queued_for_delete_populated(context, project_id\u003dNone):"},{"line_number":1066,"context_line":"    \"\"\"Determine whether user_id and queued_for_delete are set."},{"line_number":1067,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_a315f1e6","line":1064,"in_reply_to":"dfbec78f_7bc82b21","updated":"2019-05-15 16:28:24.000000000","message":"Heh, the instance mapping object was where this method was in earlier PS, but because its only use was in this file, we decided it would be clearer to move it in this file.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"01839ae47daab215e066975a1d654e0839d8bd3b","unresolved":false,"context_lines":[{"line_number":1061,"context_line":"        return 0"},{"line_number":1062,"context_line":""},{"line_number":1063,"context_line":""},{"line_number":1064,"context_line":"@db_api.api_context_manager.reader"},{"line_number":1065,"context_line":"def _user_id_queued_for_delete_populated(context, project_id\u003dNone):"},{"line_number":1066,"context_line":"    \"\"\"Determine whether user_id and queued_for_delete are set."},{"line_number":1067,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_67de13db","line":1064,"in_reply_to":"dfbec78f_a315f1e6","updated":"2019-05-16 14:00:27.000000000","message":"Yeah, I could go either way. I should have looked back a bit further.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"e25e234eb5dda7473d6229241042a6a6b7e6594d","unresolved":false,"context_lines":[{"line_number":1062,"context_line":""},{"line_number":1063,"context_line":""},{"line_number":1064,"context_line":"@db_api.api_context_manager.reader"},{"line_number":1065,"context_line":"def _user_id_queued_for_delete_populated(context, project_id\u003dNone):"},{"line_number":1066,"context_line":"    \"\"\"Determine whether user_id and queued_for_delete are set."},{"line_number":1067,"context_line":""},{"line_number":1068,"context_line":"    This will be used to determine whether we need to fall back on"}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_c7fedf69","line":1065,"range":{"start_line":1065,"start_character":61,"end_line":1065,"end_character":65},"updated":"2019-05-16 14:01:58.000000000","message":"when is this None?\n\n(later)\nah for the sever_groups quota.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"07ba290e3bb1a37c71e5f7759cbb891be2710068","unresolved":false,"context_lines":[{"line_number":1062,"context_line":""},{"line_number":1063,"context_line":""},{"line_number":1064,"context_line":"@db_api.api_context_manager.reader"},{"line_number":1065,"context_line":"def _user_id_queued_for_delete_populated(context, project_id\u003dNone):"},{"line_number":1066,"context_line":"    \"\"\"Determine whether user_id and queued_for_delete are set."},{"line_number":1067,"context_line":""},{"line_number":1068,"context_line":"    This will be used to determine whether we need to fall back on"}],"source_content_type":"text/x-python","patch_set":30,"id":"bfb3d3c7_80e5aaaa","line":1065,"range":{"start_line":1065,"start_character":61,"end_line":1065,"end_character":65},"in_reply_to":"dfbec78f_c7fedf69","updated":"2019-05-22 23:29:20.000000000","message":"Correct.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"e25e234eb5dda7473d6229241042a6a6b7e6594d","unresolved":false,"context_lines":[{"line_number":1089,"context_line":"    :returns: True if user_id is set for all non-deleted instances and"},{"line_number":1090,"context_line":"              queued_for_delete is set for all instances, else False"},{"line_number":1091,"context_line":"    \"\"\""},{"line_number":1092,"context_line":"    user_id_not_populated \u003d and_("},{"line_number":1093,"context_line":"        api_models.InstanceMapping.user_id \u003d\u003d null(),"},{"line_number":1094,"context_line":"        api_models.InstanceMapping.queued_for_delete \u003d\u003d false())"},{"line_number":1095,"context_line":"    # If either queued_for_delete or user_id are unmigrated, we will return"}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_8e624409","line":1092,"range":{"start_line":1092,"start_character":4,"end_line":1092,"end_character":25},"updated":"2019-05-16 14:01:58.000000000","message":"ah ok so if its qfd\u003dtrue we don\u0027t care if user_id is None since we don\u0027t have to count it","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"07ba290e3bb1a37c71e5f7759cbb891be2710068","unresolved":false,"context_lines":[{"line_number":1089,"context_line":"    :returns: True if user_id is set for all non-deleted instances and"},{"line_number":1090,"context_line":"              queued_for_delete is set for all instances, else False"},{"line_number":1091,"context_line":"    \"\"\""},{"line_number":1092,"context_line":"    user_id_not_populated \u003d and_("},{"line_number":1093,"context_line":"        api_models.InstanceMapping.user_id \u003d\u003d null(),"},{"line_number":1094,"context_line":"        api_models.InstanceMapping.queued_for_delete \u003d\u003d false())"},{"line_number":1095,"context_line":"    # If either queued_for_delete or user_id are unmigrated, we will return"}],"source_content_type":"text/x-python","patch_set":30,"id":"bfb3d3c7_e0d6e6c3","line":1092,"range":{"start_line":1092,"start_character":4,"end_line":1092,"end_character":25},"in_reply_to":"dfbec78f_8e624409","updated":"2019-05-22 23:29:20.000000000","message":"Right.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"e25e234eb5dda7473d6229241042a6a6b7e6594d","unresolved":false,"context_lines":[{"line_number":1259,"context_line":"    \"\"\""},{"line_number":1260,"context_line":"    if CONF.quota.count_usage_from_placement:"},{"line_number":1261,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1262,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1263,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1264,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("},{"line_number":1265,"context_line":"                context, project_id)"}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_d120e19d","line":1262,"range":{"start_line":1262,"start_character":30,"end_line":1262,"end_character":62},"updated":"2019-05-16 14:01:58.000000000","message":"++","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a3c744b1d8495325eb867be15b0be234b6f456b9","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"    if CONF.quota.count_usage_from_placement:"},{"line_number":1261,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1262,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1263,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1264,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("},{"line_number":1265,"context_line":"                context, project_id)"},{"line_number":1266,"context_line":"            if uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_7e349493","line":1263,"updated":"2019-05-13 21:43:52.000000000","message":"Now we are in stein, can we block upgrade on the migration being complete?","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"e25e234eb5dda7473d6229241042a6a6b7e6594d","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"    if CONF.quota.count_usage_from_placement:"},{"line_number":1261,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1262,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1263,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1264,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("},{"line_number":1265,"context_line":"                context, project_id)"},{"line_number":1266,"context_line":"            if uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_4c58d454","line":1263,"range":{"start_line":1263,"start_character":29,"end_line":1263,"end_character":63},"updated":"2019-05-16 14:01:58.000000000","message":"Tested the cache, once the project_id gets into it it seems consistent to me, but I still have troubles in the beginning because I would expect that after the first try for a particular project/user it should be in the cache (like in the unit tests) but for some reason it doesn\u0027t reflect fast or something. Like I can see in my logs that for the recheck_quota part it queries again instead of using the cache.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b23b7e7d6813acdea57b149c70cccc5030f3cf2b","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"    if CONF.quota.count_usage_from_placement:"},{"line_number":1261,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1262,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1263,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1264,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("},{"line_number":1265,"context_line":"                context, project_id)"},{"line_number":1266,"context_line":"            if uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":30,"id":"bfb3d3c7_275cb1eb","line":1263,"range":{"start_line":1263,"start_character":29,"end_line":1263,"end_character":63},"in_reply_to":"bfb3d3c7_94f04593","updated":"2019-05-29 15:43:56.000000000","message":"Ah, good point.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"118f0c70e29ca3ed5a2751e501bad9490788ade5","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"    if CONF.quota.count_usage_from_placement:"},{"line_number":1261,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1262,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1263,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1264,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("},{"line_number":1265,"context_line":"                context, project_id)"},{"line_number":1266,"context_line":"            if uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":30,"id":"bfb3d3c7_c5815f4c","line":1263,"range":{"start_line":1263,"start_character":29,"end_line":1263,"end_character":63},"in_reply_to":"bfb3d3c7_c004024d","updated":"2019-05-23 18:47:57.000000000","message":"Update: I added some debug log messages to this patch and upon testing in devstack, I did not see a call to placement for the quota recheck. It only called once and never again, because of the cache. So I was not able to reproduce the behavior you saw.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f4ef80fca2d5aae2558a556518d499f668c669c3","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"    if CONF.quota.count_usage_from_placement:"},{"line_number":1261,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1262,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1263,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1264,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("},{"line_number":1265,"context_line":"                context, project_id)"},{"line_number":1266,"context_line":"            if uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":30,"id":"bfb3d3c7_94f04593","line":1263,"range":{"start_line":1263,"start_character":29,"end_line":1263,"end_character":63},"in_reply_to":"bfb3d3c7_c5815f4c","updated":"2019-05-28 11:12:02.000000000","message":"Looking at this again, it will be a per process cache, so it might depend on your settings.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"e25e234eb5dda7473d6229241042a6a6b7e6594d","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"    if CONF.quota.count_usage_from_placement:"},{"line_number":1261,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1262,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1263,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1264,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("},{"line_number":1265,"context_line":"                context, project_id)"},{"line_number":1266,"context_line":"            if uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_91158942","line":1263,"in_reply_to":"dfbec78f_418531ce","updated":"2019-05-16 14:01:58.000000000","message":"\u003e I think you mean Train, and at this point I think that\u0027s quite\n \u003e premature given we don\u0027t even have a 3 month window from the Stein\n \u003e GA yet, which is a typical period for things like deprecations so I\n \u003e think that should also be a minimum for people doing upgrades.\n\nagreed, but we should maybe have a TODO to remove this logic in future when we have given sufficient amount of migration time.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"38a6d521fd816d0ccdfa5c661abb079f2c83d6cc","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"    if CONF.quota.count_usage_from_placement:"},{"line_number":1261,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1262,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1263,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1264,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("},{"line_number":1265,"context_line":"                context, project_id)"},{"line_number":1266,"context_line":"            if uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_8758e787","line":1263,"in_reply_to":"dfbec78f_418531ce","updated":"2019-05-16 13:57:18.000000000","message":"Oops, yes, Train.\n\nYou are right, its probably too soon. I was thinking going down the \"its off by default anyways\".\n\nI guess the only simplification here would be to check all projects once, if anything is left over just fall back to the old behaviour.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"07ba290e3bb1a37c71e5f7759cbb891be2710068","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"    if CONF.quota.count_usage_from_placement:"},{"line_number":1261,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1262,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1263,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1264,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("},{"line_number":1265,"context_line":"                context, project_id)"},{"line_number":1266,"context_line":"            if uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":30,"id":"bfb3d3c7_c004024d","line":1263,"range":{"start_line":1263,"start_character":29,"end_line":1263,"end_character":63},"in_reply_to":"dfbec78f_4c58d454","updated":"2019-05-22 23:29:20.000000000","message":"\u003e Tested the cache, once the project_id gets into it it seems\n \u003e consistent to me, but I still have troubles in the beginning\n \u003e because I would expect that after the first try for a particular\n \u003e project/user it should be in the cache (like in the unit tests) but\n \u003e for some reason it doesn\u0027t reflect fast or something. Like I can\n \u003e see in my logs that for the recheck_quota part it queries again\n \u003e instead of using the cache.\n\nReally appreciate your testing in devstack. I will try it myself and figure out what\u0027s going on. Thanks for letting me know.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a1b6110c70e1678f522fbf928e8e0759f8745552","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"    if CONF.quota.count_usage_from_placement:"},{"line_number":1261,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1262,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1263,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1264,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("},{"line_number":1265,"context_line":"                context, project_id)"},{"line_number":1266,"context_line":"            if uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_418531ce","line":1263,"in_reply_to":"dfbec78f_7e349493","updated":"2019-05-13 23:27:06.000000000","message":"I think you mean Train, and at this point I think that\u0027s quite premature given we don\u0027t even have a 3 month window from the Stein GA yet, which is a typical period for things like deprecations so I think that should also be a minimum for people doing upgrades.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"01839ae47daab215e066975a1d654e0839d8bd3b","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"    if CONF.quota.count_usage_from_placement:"},{"line_number":1261,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1262,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1263,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1264,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("},{"line_number":1265,"context_line":"                context, project_id)"},{"line_number":1266,"context_line":"            if uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_878b07c1","line":1263,"in_reply_to":"dfbec78f_8758e787","updated":"2019-05-16 14:00:27.000000000","message":"Hmm, ignore me, what you have here is way better.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"38a6d521fd816d0ccdfa5c661abb079f2c83d6cc","unresolved":false,"context_lines":[{"line_number":1268,"context_line":"        else:"},{"line_number":1269,"context_line":"            uid_qfd_populated \u003d True"},{"line_number":1270,"context_line":"        if not uid_qfd_populated:"},{"line_number":1271,"context_line":"            LOG.debug(\u0027Falling back to legacy quota counting method for \u0027"},{"line_number":1272,"context_line":"                      \u0027instances, cores, and ram\u0027)"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":"    if CONF.quota.count_usage_from_placement and uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_a753ab6a","line":1271,"updated":"2019-05-16 13:57:18.000000000","message":"You know, this should probably be a Warning, its quite surprising for an operator otherwise.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"07ba290e3bb1a37c71e5f7759cbb891be2710068","unresolved":false,"context_lines":[{"line_number":1268,"context_line":"        else:"},{"line_number":1269,"context_line":"            uid_qfd_populated \u003d True"},{"line_number":1270,"context_line":"        if not uid_qfd_populated:"},{"line_number":1271,"context_line":"            LOG.debug(\u0027Falling back to legacy quota counting method for \u0027"},{"line_number":1272,"context_line":"                      \u0027instances, cores, and ram\u0027)"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":"    if CONF.quota.count_usage_from_placement and uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":30,"id":"bfb3d3c7_80fe0a35","line":1271,"in_reply_to":"dfbec78f_a753ab6a","updated":"2019-05-22 23:29:20.000000000","message":"Can do. As I think you mentioned elsewhere, if an operator finds it annoying, they can turn off quota.count_usage_from_placement until the migration has completed.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"e25e234eb5dda7473d6229241042a6a6b7e6594d","unresolved":false,"context_lines":[{"line_number":1271,"context_line":"            LOG.debug(\u0027Falling back to legacy quota counting method for \u0027"},{"line_number":1272,"context_line":"                      \u0027instances, cores, and ram\u0027)"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":"    if CONF.quota.count_usage_from_placement and uid_qfd_populated:"},{"line_number":1275,"context_line":"        return _instances_cores_ram_count_api_db_placement(context, project_id,"},{"line_number":1276,"context_line":"                                                           user_id\u003duser_id)"},{"line_number":1277,"context_line":"    return _instances_cores_ram_count_legacy(context, project_id,"}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_f17f2563","line":1274,"updated":"2019-05-16 14:01:58.000000000","message":"logic looks good to me, I tested this on devstack, the only other smallish thing I would have liked here or anywhere would be a DEBUG/INFO saying that my deployment is using placement to count quotas (I understand it might be too many logs) because I couldn\u0027t see it anywhere in the logs, also after querying for the resources here we go back to quota.py where we print:\n\n Getting quotas for project 3b6beaf083204623b3f54fdbded16916. Resources: set([\u0027instances\u0027, \u0027ram\u0027, \u0027cores\u0027]) {{(pid\u003d32156) _get_quotas /opt/stack/nova/nova/quota.py:386}}\n Getting quotas for user 6104fcfea7c746f9b0380e6bc64fc906 and project 3b6beaf083204623b3f54fdbded16916. Resources: set([\u0027instances\u0027, \u0027ram\u0027, \u0027cores\u0027]) {{(pid\u003d32156) _get_quotas /opt/stack/nova/nova/quota.py:378}}\n\nwhich must be why I got confused.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"07ba290e3bb1a37c71e5f7759cbb891be2710068","unresolved":false,"context_lines":[{"line_number":1271,"context_line":"            LOG.debug(\u0027Falling back to legacy quota counting method for \u0027"},{"line_number":1272,"context_line":"                      \u0027instances, cores, and ram\u0027)"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":"    if CONF.quota.count_usage_from_placement and uid_qfd_populated:"},{"line_number":1275,"context_line":"        return _instances_cores_ram_count_api_db_placement(context, project_id,"},{"line_number":1276,"context_line":"                                                           user_id\u003duser_id)"},{"line_number":1277,"context_line":"    return _instances_cores_ram_count_legacy(context, project_id,"}],"source_content_type":"text/x-python","patch_set":30,"id":"bfb3d3c7_00b9ba78","line":1274,"in_reply_to":"dfbec78f_f17f2563","updated":"2019-05-22 23:29:20.000000000","message":"Yeah, the nova.quota.* APIs are an abstraction, so whether we call placement or not we go through the same APIs (and logging).\n\nAt service startup time, the config option setting for quota.count_usage_from_placement will be shown, but that will get rotated away after some time.\n\nI didn\u0027t add a new log message for the placement thing because I thought it would be super repetitive, but as you point out, we already have repetitive messages about \"getting quotas\" so why not throw another one on the pile. I\u0027ll add something in the next PS.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e18d943135bf8444d196a8a01dcb1312cf319c35","unresolved":false,"context_lines":[{"line_number":1101,"context_line":"        unmigrated_filter)"},{"line_number":1102,"context_line":"    if project_id:"},{"line_number":1103,"context_line":"        query \u003d query.filter_by(project_id\u003dproject_id)"},{"line_number":1104,"context_line":"    return not context.session.query(query.exists()).scalar()"},{"line_number":1105,"context_line":""},{"line_number":1106,"context_line":""},{"line_number":1107,"context_line":"def _keypair_get_count_by_user(context, user_id):"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_359e1ab9","line":1104,"range":{"start_line":1104,"start_character":43,"end_line":1104,"end_character":49},"updated":"2019-05-29 15:56:04.000000000","message":"Hmm, https://docs.sqlalchemy.org/en/13/orm/query.html#sqlalchemy.orm.query.Query.exists says \"Note that some databases such as SQL Server don’t allow an EXISTS expression to be present in the columns clause of a SELECT.\"\n\nWe run tempest-pg-full in the nova experimental queue but we wouldn\u0027t be running it with this code since by default counting quotas from placement is false. We should probably throw up a separate change which changes the default on the config option to True and make sure the tempest-pg-full job is OK.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e18d943135bf8444d196a8a01dcb1312cf319c35","unresolved":false,"context_lines":[{"line_number":1101,"context_line":"        unmigrated_filter)"},{"line_number":1102,"context_line":"    if project_id:"},{"line_number":1103,"context_line":"        query \u003d query.filter_by(project_id\u003dproject_id)"},{"line_number":1104,"context_line":"    return not context.session.query(query.exists()).scalar()"},{"line_number":1105,"context_line":""},{"line_number":1106,"context_line":""},{"line_number":1107,"context_line":"def _keypair_get_count_by_user(context, user_id):"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_55a14ef7","line":1104,"range":{"start_line":1104,"start_character":53,"end_line":1104,"end_character":59},"updated":"2019-05-29 15:56:04.000000000","message":"Note to self: need to make sure we have a test for this which has multiple unmigrated rows so scalar() doesn\u0027t blow up:\n\nhttps://docs.sqlalchemy.org/en/13/orm/query.html#sqlalchemy.orm.query.Query.scalar\n\n\u003e If multiple rows are returned, raises MultipleResultsFound.\n\n(later)\n\nLooks like this is tested and not a problem:\n\nhttps://review.opendev.org/#/c/638073/31/nova/tests/functional/db/test_quota.py@185\n\nMaybe I don\u0027t understand that MultipleResultsFound case in the scalar() docs.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"efaa407c788c8a7860fa4f894f86aebb31792732","unresolved":false,"context_lines":[{"line_number":1101,"context_line":"        unmigrated_filter)"},{"line_number":1102,"context_line":"    if project_id:"},{"line_number":1103,"context_line":"        query \u003d query.filter_by(project_id\u003dproject_id)"},{"line_number":1104,"context_line":"    return not context.session.query(query.exists()).scalar()"},{"line_number":1105,"context_line":""},{"line_number":1106,"context_line":""},{"line_number":1107,"context_line":"def _keypair_get_count_by_user(context, user_id):"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_4a79ac91","line":1104,"range":{"start_line":1104,"start_character":43,"end_line":1104,"end_character":49},"in_reply_to":"bfb3d3c7_359e1ab9","updated":"2019-05-29 16:10:27.000000000","message":"\u003e Hmm, https://docs.sqlalchemy.org/en/13/orm/query.html#sqlalchemy.orm.query.Query.exists\n \u003e says \"Note that some databases such as SQL Server don’t allow an\n \u003e EXISTS expression to be present in the columns clause of a SELECT.\"\n \u003e \n \u003e We run tempest-pg-full in the nova experimental queue but we\n \u003e wouldn\u0027t be running it with this code since by default counting\n \u003e quotas from placement is false. We should probably throw up a\n \u003e separate change which changes the default on the config option to\n \u003e True and make sure the tempest-pg-full job is OK.\n\nhttps://review.opendev.org/#/c/662028/","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"db1a4afb5378d351fad66962aaa098ed59546223","unresolved":false,"context_lines":[{"line_number":1101,"context_line":"        unmigrated_filter)"},{"line_number":1102,"context_line":"    if project_id:"},{"line_number":1103,"context_line":"        query \u003d query.filter_by(project_id\u003dproject_id)"},{"line_number":1104,"context_line":"    return not context.session.query(query.exists()).scalar()"},{"line_number":1105,"context_line":""},{"line_number":1106,"context_line":""},{"line_number":1107,"context_line":"def _keypair_get_count_by_user(context, user_id):"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_fda388fe","line":1104,"range":{"start_line":1104,"start_character":53,"end_line":1104,"end_character":59},"in_reply_to":"bfb3d3c7_55a14ef7","updated":"2019-05-29 17:06:54.000000000","message":"\u003e Note to self: need to make sure we have a test for this which has\n \u003e multiple unmigrated rows so scalar() doesn\u0027t blow up:\n \u003e \n \u003e https://docs.sqlalchemy.org/en/13/orm/query.html#sqlalchemy.orm.query.Query.scalar\n \u003e \n \u003e \u003e If multiple rows are returned, raises MultipleResultsFound.\n \u003e \n \u003e (later)\n \u003e \n \u003e Looks like this is tested and not a problem:\n \u003e \n \u003e https://review.opendev.org/#/c/638073/31/nova/tests/functional/db/test_quota.py@185\n \u003e \n \u003e Maybe I don\u0027t understand that MultipleResultsFound case in the\n \u003e scalar() docs.\n\nI think there\u0027s no problem with that (and no MultipleResultsFound raised) because of the EXISTS subquery. That will make it return either one row or no rows, IIUC:\n\nhttps://docs.sqlalchemy.org/en/13/orm/query.html#sqlalchemy.orm.query.Query.exists","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3d48b462af07eb05a9dac7fe44112c9cf98dde9c","unresolved":false,"context_lines":[{"line_number":1101,"context_line":"        unmigrated_filter)"},{"line_number":1102,"context_line":"    if project_id:"},{"line_number":1103,"context_line":"        query \u003d query.filter_by(project_id\u003dproject_id)"},{"line_number":1104,"context_line":"    return not context.session.query(query.exists()).scalar()"},{"line_number":1105,"context_line":""},{"line_number":1106,"context_line":""},{"line_number":1107,"context_line":"def _keypair_get_count_by_user(context, user_id):"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_566391b2","line":1104,"range":{"start_line":1104,"start_character":53,"end_line":1104,"end_character":59},"in_reply_to":"bfb3d3c7_fda388fe","updated":"2019-05-29 19:34:11.000000000","message":"Ack, and tempest-pg-full is OK with this as well:\n\nhttps://review.opendev.org/#/c/662028/","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e18d943135bf8444d196a8a01dcb1312cf319c35","unresolved":false,"context_lines":[{"line_number":1261,"context_line":"    if CONF.quota.count_usage_from_placement:"},{"line_number":1262,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1263,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1264,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1265,"context_line":"            LOG.debug(\u0027Checking whether user_id and queued_for_delete are \u0027"},{"line_number":1266,"context_line":"                      \u0027populated for project_id %s\u0027, project_id)"},{"line_number":1267,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_95a6e6aa","line":1264,"range":{"start_line":1264,"start_character":22,"end_line":1264,"end_character":28},"updated":"2019-05-29 15:56:04.000000000","message":"TIL that the contains check on a set and a dict are both O(1):\n\nhttps://wiki.python.org/moin/TimeComplexity\n\n(10:34:36 AM) mriedem: sean-k-mooney: i bet you know the answer to this, is \"x in y\" where y is a set the same time as if y were a dict?\n(10:35:26 AM) aspiers: mriedem: same time as in complexity? or same thing? I think the answer to both is yes\n(10:36:06 AM) mriedem: i\u0027m asking if both are O(1)\n(10:36:21 AM) sean-k-mooney: yes they are both O(1)","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"db1a4afb5378d351fad66962aaa098ed59546223","unresolved":false,"context_lines":[{"line_number":1261,"context_line":"    if CONF.quota.count_usage_from_placement:"},{"line_number":1262,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1263,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1264,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1265,"context_line":"            LOG.debug(\u0027Checking whether user_id and queued_for_delete are \u0027"},{"line_number":1266,"context_line":"                      \u0027populated for project_id %s\u0027, project_id)"},{"line_number":1267,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_9d8b6c66","line":1264,"range":{"start_line":1264,"start_character":22,"end_line":1264,"end_character":28},"in_reply_to":"bfb3d3c7_95a6e6aa","updated":"2019-05-29 17:06:54.000000000","message":"Makes sense and is what I have thought. I use set when there\u0027s no value I need to store along with the contains check, instead of adding a placeholder unused value to a dict.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e18d943135bf8444d196a8a01dcb1312cf319c35","unresolved":false,"context_lines":[{"line_number":1262,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1263,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1264,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1265,"context_line":"            LOG.debug(\u0027Checking whether user_id and queued_for_delete are \u0027"},{"line_number":1266,"context_line":"                      \u0027populated for project_id %s\u0027, project_id)"},{"line_number":1267,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("},{"line_number":1268,"context_line":"                context, project_id)"},{"line_number":1269,"context_line":"            if uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_f5a9628f","line":1266,"range":{"start_line":1265,"start_character":12,"end_line":1266,"end_character":64},"updated":"2019-05-29 15:56:04.000000000","message":"This shows up quite a bit in a gate run:\n\nhttp://logs.openstack.org/46/653146/7/check/nova-next/691dc82/logs/screen-n-api.txt.gz\n\n103 times in there. This will eventually be dropped when we drop the online data migration and compat code, right? If so, then I guess the chattiness is OK (and this doesn\u0027t show up by default anyway because count_usage_from_placement\u003dFalse).","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"db1a4afb5378d351fad66962aaa098ed59546223","unresolved":false,"context_lines":[{"line_number":1262,"context_line":"        # If a project has all user_id and queued_for_delete data populated,"},{"line_number":1263,"context_line":"        # cache the result to avoid needless database checking in the future."},{"line_number":1264,"context_line":"        if project_id not in UID_QFD_POPULATED_CACHE_BY_PROJECT:"},{"line_number":1265,"context_line":"            LOG.debug(\u0027Checking whether user_id and queued_for_delete are \u0027"},{"line_number":1266,"context_line":"                      \u0027populated for project_id %s\u0027, project_id)"},{"line_number":1267,"context_line":"            uid_qfd_populated \u003d _user_id_queued_for_delete_populated("},{"line_number":1268,"context_line":"                context, project_id)"},{"line_number":1269,"context_line":"            if uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_3dd5a044","line":1266,"range":{"start_line":1265,"start_character":12,"end_line":1266,"end_character":64},"in_reply_to":"bfb3d3c7_f5a9628f","updated":"2019-05-29 17:06:54.000000000","message":"Yeah.... originally I didn\u0027t have a log message for this because I thought it would be chatty, but added it because tssurya asked for it IIRC. And I did find it useful when I was testing things out in devstack for being to able to see what\u0027s going on, and I could see operators potentially being interested in the same.\n\nAnd yes, we will want to drop this alongside the data migration and compat code when the time comes.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e18d943135bf8444d196a8a01dcb1312cf319c35","unresolved":false,"context_lines":[{"line_number":1271,"context_line":"        else:"},{"line_number":1272,"context_line":"            uid_qfd_populated \u003d True"},{"line_number":1273,"context_line":"        if not uid_qfd_populated:"},{"line_number":1274,"context_line":"            LOG.warning(\u0027Falling back to legacy quota counting method for \u0027"},{"line_number":1275,"context_line":"                        \u0027instances, cores, and ram\u0027)"},{"line_number":1276,"context_line":""},{"line_number":1277,"context_line":"    if CONF.quota.count_usage_from_placement and uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_07f2cd06","line":1274,"range":{"start_line":1274,"start_character":16,"end_line":1274,"end_character":23},"updated":"2019-05-29 15:56:04.000000000","message":"I\u0027m not sure this needs to be a warning, info is probably sufficient, because this warning would get logged every time the project hits this until the data is migrated. When would that data be migrated? On GET of the server or when the online data migration is run, right? The user opts into the former and the operator controls the latter, but I\u0027m not sure it needs to be a warning either way.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"db1a4afb5378d351fad66962aaa098ed59546223","unresolved":false,"context_lines":[{"line_number":1271,"context_line":"        else:"},{"line_number":1272,"context_line":"            uid_qfd_populated \u003d True"},{"line_number":1273,"context_line":"        if not uid_qfd_populated:"},{"line_number":1274,"context_line":"            LOG.warning(\u0027Falling back to legacy quota counting method for \u0027"},{"line_number":1275,"context_line":"                        \u0027instances, cores, and ram\u0027)"},{"line_number":1276,"context_line":""},{"line_number":1277,"context_line":"    if CONF.quota.count_usage_from_placement and uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_1dce9cb3","line":1274,"range":{"start_line":1274,"start_character":16,"end_line":1274,"end_character":23},"in_reply_to":"bfb3d3c7_07f2cd06","updated":"2019-05-29 17:06:54.000000000","message":"tssurya and johnthetubaguy asked for this to be changed from debug to warning in earlier PS. The rationale was that if the operator finds it annoying, they will switch [quota]count_usage_from_placement back to False and let the data migration complete before turning it on again.\n\nYes, data is migrated on GET of the server or when online data migration is run.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dab18a9ad12536b50272cfe90092f9e7ab0e80f1","unresolved":false,"context_lines":[{"line_number":1271,"context_line":"        else:"},{"line_number":1272,"context_line":"            uid_qfd_populated \u003d True"},{"line_number":1273,"context_line":"        if not uid_qfd_populated:"},{"line_number":1274,"context_line":"            LOG.warning(\u0027Falling back to legacy quota counting method for \u0027"},{"line_number":1275,"context_line":"                        \u0027instances, cores, and ram\u0027)"},{"line_number":1276,"context_line":""},{"line_number":1277,"context_line":"    if CONF.quota.count_usage_from_placement and uid_qfd_populated:"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_6c3da88d","line":1274,"range":{"start_line":1274,"start_character":16,"end_line":1274,"end_character":23},"in_reply_to":"bfb3d3c7_1dce9cb3","updated":"2019-05-29 21:15:15.000000000","message":"\u003e The rationale was that if the operator finds it annoying, they will switch [quota]count_usage_from_placement back to False and let the data migration complete before turning it on again.\n\nHmm, OK I don\u0027t see someone really wanting to fiddle with that option since it could throw counting off depending on what happened while this was on. But the release note does say that you should complete the data migration before turning this on so maybe that is sufficient (although it likely won\u0027t be if someone skips past the Train release notes and only enables this in like U or V). I would still opt to INFO for this but it\u0027s not a huge deal, I won\u0027t block on it.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e18d943135bf8444d196a8a01dcb1312cf319c35","unresolved":false,"context_lines":[{"line_number":1274,"context_line":"            LOG.warning(\u0027Falling back to legacy quota counting method for \u0027"},{"line_number":1275,"context_line":"                        \u0027instances, cores, and ram\u0027)"},{"line_number":1276,"context_line":""},{"line_number":1277,"context_line":"    if CONF.quota.count_usage_from_placement and uid_qfd_populated:"},{"line_number":1278,"context_line":"        return _instances_cores_ram_count_api_db_placement(context, project_id,"},{"line_number":1279,"context_line":"                                                           user_id\u003duser_id)"},{"line_number":1280,"context_line":"    return _instances_cores_ram_count_legacy(context, project_id,"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_07a40d07","line":1277,"range":{"start_line":1277,"start_character":49,"end_line":1277,"end_character":66},"updated":"2019-05-29 15:56:04.000000000","message":"nit: This would be an UnboundLocalError if you didn\u0027t have the CONF.quota.count_usage_from_placement condition first. I would have just nested this under the \"if CONF.quota.count_usage_from_placement\" conditional block above.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"db1a4afb5378d351fad66962aaa098ed59546223","unresolved":false,"context_lines":[{"line_number":1274,"context_line":"            LOG.warning(\u0027Falling back to legacy quota counting method for \u0027"},{"line_number":1275,"context_line":"                        \u0027instances, cores, and ram\u0027)"},{"line_number":1276,"context_line":""},{"line_number":1277,"context_line":"    if CONF.quota.count_usage_from_placement and uid_qfd_populated:"},{"line_number":1278,"context_line":"        return _instances_cores_ram_count_api_db_placement(context, project_id,"},{"line_number":1279,"context_line":"                                                           user_id\u003duser_id)"},{"line_number":1280,"context_line":"    return _instances_cores_ram_count_legacy(context, project_id,"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_bd4d103f","line":1277,"range":{"start_line":1277,"start_character":49,"end_line":1277,"end_character":66},"in_reply_to":"bfb3d3c7_07a40d07","updated":"2019-05-29 17:06:54.000000000","message":"Yeah, could have done that. Will do it if I need to respin.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dab18a9ad12536b50272cfe90092f9e7ab0e80f1","unresolved":false,"context_lines":[{"line_number":1274,"context_line":"            LOG.warning(\u0027Falling back to legacy quota counting method for \u0027"},{"line_number":1275,"context_line":"                        \u0027instances, cores, and ram\u0027)"},{"line_number":1276,"context_line":""},{"line_number":1277,"context_line":"    if CONF.quota.count_usage_from_placement and uid_qfd_populated:"},{"line_number":1278,"context_line":"        return _instances_cores_ram_count_api_db_placement(context, project_id,"},{"line_number":1279,"context_line":"                                                           user_id\u003duser_id)"},{"line_number":1280,"context_line":"    return _instances_cores_ram_count_legacy(context, project_id,"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_8c6c9c9c","line":1277,"range":{"start_line":1277,"start_character":49,"end_line":1277,"end_character":66},"in_reply_to":"bfb3d3c7_bd4d103f","updated":"2019-05-29 21:15:15.000000000","message":"How about throwing it into your FUP patch?\n\nhttps://review.opendev.org/#/c/662056/","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"}],"nova/scheduler/client/report.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2631a05c8cf46b38838df324619e89e2150e4c42","unresolved":false,"context_lines":[{"line_number":2219,"context_line":"            cores \u003d data[\u0027usages\u0027].get(fields.ResourceClass.VCPU, 0)"},{"line_number":2220,"context_line":"            ram \u003d data[\u0027usages\u0027].get(fields.ResourceClass.MEMORY_MB, 0)"},{"line_number":2221,"context_line":"            total_counts[\u0027project\u0027] \u003d {\u0027cores\u0027: cores, \u0027ram\u0027: ram}"},{"line_number":2222,"context_line":"        else:"},{"line_number":2223,"context_line":"            self._handle_usages_error_from_placement(resp, project_id, user_id)"},{"line_number":2224,"context_line":"        # If specified, second query counts across one user in the project"},{"line_number":2225,"context_line":"        if user_id:"},{"line_number":2226,"context_line":"            url \u003d \u0027\u0027.join([url, \u0027\u0026user_id\u003d%s\u0027 % user_id])"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_1c743867","line":2223,"range":{"start_line":2222,"start_character":0,"end_line":2223,"end_character":79},"updated":"2019-02-28 21:42:16.000000000","message":"Another option would be to make your placement calls with raise_exc\u003dTrue so you could wrap the whole shebang in one try/except.","commit_id":"28c83029dddd88c1687e909731bb636a58d814d7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7bfd7b9c49b0df9510c3bc4ad178cc386417e1d8","unresolved":false,"context_lines":[{"line_number":2219,"context_line":"            cores \u003d data[\u0027usages\u0027].get(fields.ResourceClass.VCPU, 0)"},{"line_number":2220,"context_line":"            ram \u003d data[\u0027usages\u0027].get(fields.ResourceClass.MEMORY_MB, 0)"},{"line_number":2221,"context_line":"            total_counts[\u0027project\u0027] \u003d {\u0027cores\u0027: cores, \u0027ram\u0027: ram}"},{"line_number":2222,"context_line":"        else:"},{"line_number":2223,"context_line":"            self._handle_usages_error_from_placement(resp, project_id, user_id)"},{"line_number":2224,"context_line":"        # If specified, second query counts across one user in the project"},{"line_number":2225,"context_line":"        if user_id:"},{"line_number":2226,"context_line":"            url \u003d \u0027\u0027.join([url, \u0027\u0026user_id\u003d%s\u0027 % user_id])"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_9cbee862","line":2223,"range":{"start_line":2222,"start_character":0,"end_line":2223,"end_character":79},"in_reply_to":"9fdfeff1_1c743867","updated":"2019-02-28 22:02:20.000000000","message":"I\u0027ll look into that.","commit_id":"28c83029dddd88c1687e909731bb636a58d814d7"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"48493b74b08090ddc23a27ef19c319cd8cd379fa","unresolved":false,"context_lines":[{"line_number":2219,"context_line":"            cores \u003d data[\u0027usages\u0027].get(fields.ResourceClass.VCPU, 0)"},{"line_number":2220,"context_line":"            ram \u003d data[\u0027usages\u0027].get(fields.ResourceClass.MEMORY_MB, 0)"},{"line_number":2221,"context_line":"            total_counts[\u0027project\u0027] \u003d {\u0027cores\u0027: cores, \u0027ram\u0027: ram}"},{"line_number":2222,"context_line":"        else:"},{"line_number":2223,"context_line":"            self._handle_usages_error_from_placement(resp, project_id, user_id)"},{"line_number":2224,"context_line":"        # If specified, second query counts across one user in the project"},{"line_number":2225,"context_line":"        if user_id:"},{"line_number":2226,"context_line":"            url \u003d \u0027\u0027.join([url, \u0027\u0026user_id\u003d%s\u0027 % user_id])"}],"source_content_type":"text/x-python","patch_set":13,"id":"5fc1f717_e1b08e8e","line":2223,"range":{"start_line":2222,"start_character":0,"end_line":2223,"end_character":79},"in_reply_to":"9fdfeff1_5e2110b5","updated":"2019-03-05 21:05:42.000000000","message":"I believe any exception you trap will inherit from keystoneauth1.exceptions.http.HttpError [1], which has a `request_id` attribute, as well as other juicy details like `response`, which I believe is the same Response object you get when you don\u0027t raise_exc\u003dTrue.\n\n[1] https://github.com/openstack/keystoneauth/blob/bde07bc95b5b5d16b829f72be7aaa62fab9d716a/keystoneauth1/exceptions/http.py#L61","commit_id":"28c83029dddd88c1687e909731bb636a58d814d7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e39a326d103b1c5e646a2622ff378f136cba3ebb","unresolved":false,"context_lines":[{"line_number":2219,"context_line":"            cores \u003d data[\u0027usages\u0027].get(fields.ResourceClass.VCPU, 0)"},{"line_number":2220,"context_line":"            ram \u003d data[\u0027usages\u0027].get(fields.ResourceClass.MEMORY_MB, 0)"},{"line_number":2221,"context_line":"            total_counts[\u0027project\u0027] \u003d {\u0027cores\u0027: cores, \u0027ram\u0027: ram}"},{"line_number":2222,"context_line":"        else:"},{"line_number":2223,"context_line":"            self._handle_usages_error_from_placement(resp, project_id, user_id)"},{"line_number":2224,"context_line":"        # If specified, second query counts across one user in the project"},{"line_number":2225,"context_line":"        if user_id:"},{"line_number":2226,"context_line":"            url \u003d \u0027\u0027.join([url, \u0027\u0026user_id\u003d%s\u0027 % user_id])"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_5e2110b5","line":2223,"range":{"start_line":2222,"start_character":0,"end_line":2223,"end_character":79},"in_reply_to":"9fdfeff1_9cbee862","updated":"2019-03-05 07:54:15.000000000","message":"I just tried to do this, but got stuck trying to include the placement_request_id in the error log message. How could I get the placement_request_id for the error logging when I handle the exception?","commit_id":"28c83029dddd88c1687e909731bb636a58d814d7"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8cbc2da8842e92ab1024893b0a013db2200d316b","unresolved":false,"context_lines":[{"line_number":2195,"context_line":"        raise exception.UsagesRetrievalFailed(project_id\u003dproject_id,"},{"line_number":2196,"context_line":"                                              user_id\u003duser_id or \u0027N/A\u0027)"},{"line_number":2197,"context_line":""},{"line_number":2198,"context_line":"    def get_usages_counts_for_quota(self, context, project_id, user_id\u003dNone):"},{"line_number":2199,"context_line":"        \"\"\"Get the usages counts for the purpose of counting quota usage."},{"line_number":2200,"context_line":""},{"line_number":2201,"context_line":"        :param context: The request context"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_2d499ab6","line":2198,"updated":"2019-03-04 12:33:10.000000000","message":"I can\u0027t remember where we landed on doing/controlling retries, but this seems like a good candidate for considering retries on network failures.\n\nSay we have 5 placement servers in a round robin dns (just for an example). The one we try to talk to has a partition, it would perhaps be reasonable to try again.","commit_id":"fe7823a972b57e26ec0533e35dccf5bbba625176"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e39a326d103b1c5e646a2622ff378f136cba3ebb","unresolved":false,"context_lines":[{"line_number":2195,"context_line":"        raise exception.UsagesRetrievalFailed(project_id\u003dproject_id,"},{"line_number":2196,"context_line":"                                              user_id\u003duser_id or \u0027N/A\u0027)"},{"line_number":2197,"context_line":""},{"line_number":2198,"context_line":"    def get_usages_counts_for_quota(self, context, project_id, user_id\u003dNone):"},{"line_number":2199,"context_line":"        \"\"\"Get the usages counts for the purpose of counting quota usage."},{"line_number":2200,"context_line":""},{"line_number":2201,"context_line":"        :param context: The request context"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_79a8aed8","line":2198,"in_reply_to":"9fdfeff1_2d499ab6","updated":"2019-03-05 07:54:15.000000000","message":"I don\u0027t think I have the context on that, but looking at other examples in this file using @retrying.retry, what exception would I handle to retry on network failures?","commit_id":"fe7823a972b57e26ec0533e35dccf5bbba625176"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"48493b74b08090ddc23a27ef19c319cd8cd379fa","unresolved":false,"context_lines":[{"line_number":2195,"context_line":"        raise exception.UsagesRetrievalFailed(project_id\u003dproject_id,"},{"line_number":2196,"context_line":"                                              user_id\u003duser_id or \u0027N/A\u0027)"},{"line_number":2197,"context_line":""},{"line_number":2198,"context_line":"    def get_usages_counts_for_quota(self, context, project_id, user_id\u003dNone):"},{"line_number":2199,"context_line":"        \"\"\"Get the usages counts for the purpose of counting quota usage."},{"line_number":2200,"context_line":""},{"line_number":2201,"context_line":"        :param context: The request context"}],"source_content_type":"text/x-python","patch_set":14,"id":"5fc1f717_414ac280","line":2198,"in_reply_to":"9fdfeff1_79a8aed8","updated":"2019-03-05 21:05:42.000000000","message":"Yeah, keystoneauth1.exceptions.base.ClientException would catch everything from ksa, which would include all the ones we enacted @safe_connect craziness for.\n\n(Don\u0027t use @safe_connect. Please.)","commit_id":"fe7823a972b57e26ec0533e35dccf5bbba625176"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8cbc2da8842e92ab1024893b0a013db2200d316b","unresolved":false,"context_lines":[{"line_number":2216,"context_line":"                        global_request_id\u003dcontext.global_id)"},{"line_number":2217,"context_line":"        if resp.status_code \u003d\u003d 200:"},{"line_number":2218,"context_line":"            data \u003d resp.json()"},{"line_number":2219,"context_line":"            cores \u003d data[\u0027usages\u0027].get(fields.ResourceClass.VCPU, 0)"},{"line_number":2220,"context_line":"            ram \u003d data[\u0027usages\u0027].get(fields.ResourceClass.MEMORY_MB, 0)"},{"line_number":2221,"context_line":"            total_counts[\u0027project\u0027] \u003d {\u0027cores\u0027: cores, \u0027ram\u0027: ram}"},{"line_number":2222,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_edf1526c","line":2219,"updated":"2019-03-04 12:33:10.000000000","message":"This will be os_resource_classes.VCPU now, usually seen as \u0027orc.VCPU\u0027.","commit_id":"fe7823a972b57e26ec0533e35dccf5bbba625176"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e39a326d103b1c5e646a2622ff378f136cba3ebb","unresolved":false,"context_lines":[{"line_number":2216,"context_line":"                        global_request_id\u003dcontext.global_id)"},{"line_number":2217,"context_line":"        if resp.status_code \u003d\u003d 200:"},{"line_number":2218,"context_line":"            data \u003d resp.json()"},{"line_number":2219,"context_line":"            cores \u003d data[\u0027usages\u0027].get(fields.ResourceClass.VCPU, 0)"},{"line_number":2220,"context_line":"            ram \u003d data[\u0027usages\u0027].get(fields.ResourceClass.MEMORY_MB, 0)"},{"line_number":2221,"context_line":"            total_counts[\u0027project\u0027] \u003d {\u0027cores\u0027: cores, \u0027ram\u0027: ram}"},{"line_number":2222,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_1efaa842","line":2219,"in_reply_to":"9fdfeff1_edf1526c","updated":"2019-03-05 07:54:15.000000000","message":"Done","commit_id":"fe7823a972b57e26ec0533e35dccf5bbba625176"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8cbc2da8842e92ab1024893b0a013db2200d316b","unresolved":false,"context_lines":[{"line_number":2217,"context_line":"        if resp.status_code \u003d\u003d 200:"},{"line_number":2218,"context_line":"            data \u003d resp.json()"},{"line_number":2219,"context_line":"            cores \u003d data[\u0027usages\u0027].get(fields.ResourceClass.VCPU, 0)"},{"line_number":2220,"context_line":"            ram \u003d data[\u0027usages\u0027].get(fields.ResourceClass.MEMORY_MB, 0)"},{"line_number":2221,"context_line":"            total_counts[\u0027project\u0027] \u003d {\u0027cores\u0027: cores, \u0027ram\u0027: ram}"},{"line_number":2222,"context_line":"        else:"},{"line_number":2223,"context_line":"            self._handle_usages_error_from_placement(resp, project_id, user_id)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_6dfd623a","line":2220,"updated":"2019-03-04 12:33:10.000000000","message":"ditto-ish","commit_id":"fe7823a972b57e26ec0533e35dccf5bbba625176"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e39a326d103b1c5e646a2622ff378f136cba3ebb","unresolved":false,"context_lines":[{"line_number":2217,"context_line":"        if resp.status_code \u003d\u003d 200:"},{"line_number":2218,"context_line":"            data \u003d resp.json()"},{"line_number":2219,"context_line":"            cores \u003d data[\u0027usages\u0027].get(fields.ResourceClass.VCPU, 0)"},{"line_number":2220,"context_line":"            ram \u003d data[\u0027usages\u0027].get(fields.ResourceClass.MEMORY_MB, 0)"},{"line_number":2221,"context_line":"            total_counts[\u0027project\u0027] \u003d {\u0027cores\u0027: cores, \u0027ram\u0027: ram}"},{"line_number":2222,"context_line":"        else:"},{"line_number":2223,"context_line":"            self._handle_usages_error_from_placement(resp, project_id, user_id)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_3ef76c5c","line":2220,"in_reply_to":"9fdfeff1_6dfd623a","updated":"2019-03-05 07:54:15.000000000","message":"Done","commit_id":"fe7823a972b57e26ec0533e35dccf5bbba625176"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9174d534f7698244bab8719c085ac5658ea1334f","unresolved":false,"context_lines":[{"line_number":2318,"context_line":"            context, rp_uuid, new_aggs, use_cache\u003dFalse, generation\u003dgen)"},{"line_number":2319,"context_line":""},{"line_number":2320,"context_line":"    @staticmethod"},{"line_number":2321,"context_line":"    def _handle_usages_error_from_placement(resp, project_id, user_id):"},{"line_number":2322,"context_line":"        msg \u003d (\u0027[%(placement_req_id)s] Failed to retrieve usages for project \u0027"},{"line_number":2323,"context_line":"               \u0027%(project_id)s and user %(user_id)s. Got %(status_code)d: \u0027"},{"line_number":2324,"context_line":"               \u0027%(err_text)s\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_e5871ad7","line":2321,"range":{"start_line":2321,"start_character":8,"end_line":2321,"end_character":43},"updated":"2019-03-06 22:44:42.000000000","message":"decided the raise_exc\u003dTrue thing was more trouble than it was worth?","commit_id":"6dfc30d0020f3b75cf30c60d85e351994cb34669"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f5f863fffaf734a05b83162b44e96d102e8834ef","unresolved":false,"context_lines":[{"line_number":2318,"context_line":"            context, rp_uuid, new_aggs, use_cache\u003dFalse, generation\u003dgen)"},{"line_number":2319,"context_line":""},{"line_number":2320,"context_line":"    @staticmethod"},{"line_number":2321,"context_line":"    def _handle_usages_error_from_placement(resp, project_id, user_id):"},{"line_number":2322,"context_line":"        msg \u003d (\u0027[%(placement_req_id)s] Failed to retrieve usages for project \u0027"},{"line_number":2323,"context_line":"               \u0027%(project_id)s and user %(user_id)s. Got %(status_code)d: \u0027"},{"line_number":2324,"context_line":"               \u0027%(err_text)s\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_e0af0892","line":2321,"range":{"start_line":2321,"start_character":8,"end_line":2321,"end_character":43},"in_reply_to":"5fc1f717_c00e8ce4","updated":"2019-03-06 23:27:51.000000000","message":"\u003e I have to add a kwarg to our\n \u003e get() method to pass along.\n\noh, ew, I didn\u0027t realize that. Never mind, it\u0027s not worth it.","commit_id":"6dfc30d0020f3b75cf30c60d85e351994cb34669"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"723e4a0664122130a06bee0ef812940b2970541b","unresolved":false,"context_lines":[{"line_number":2318,"context_line":"            context, rp_uuid, new_aggs, use_cache\u003dFalse, generation\u003dgen)"},{"line_number":2319,"context_line":""},{"line_number":2320,"context_line":"    @staticmethod"},{"line_number":2321,"context_line":"    def _handle_usages_error_from_placement(resp, project_id, user_id):"},{"line_number":2322,"context_line":"        msg \u003d (\u0027[%(placement_req_id)s] Failed to retrieve usages for project \u0027"},{"line_number":2323,"context_line":"               \u0027%(project_id)s and user %(user_id)s. Got %(status_code)d: \u0027"},{"line_number":2324,"context_line":"               \u0027%(err_text)s\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_20e2f0d6","line":2321,"range":{"start_line":2321,"start_character":8,"end_line":2321,"end_character":43},"in_reply_to":"5fc1f717_e0af0892","updated":"2019-03-06 23:33:57.000000000","message":"Yeah:\n\nhttps://github.com/openstack/nova/blob/eaa29f71ef01f5da2edfa79886a302f8a5f352ae/nova/scheduler/client/report.py#L241\n\nhttps://github.com/openstack/nova/blob/eaa29f71ef01f5da2edfa79886a302f8a5f352ae/nova/utils.py#L1223\n\nOK, cool.","commit_id":"6dfc30d0020f3b75cf30c60d85e351994cb34669"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9253af62dc7823eaf680f8bd91317e82a5ca805e","unresolved":false,"context_lines":[{"line_number":2318,"context_line":"            context, rp_uuid, new_aggs, use_cache\u003dFalse, generation\u003dgen)"},{"line_number":2319,"context_line":""},{"line_number":2320,"context_line":"    @staticmethod"},{"line_number":2321,"context_line":"    def _handle_usages_error_from_placement(resp, project_id, user_id):"},{"line_number":2322,"context_line":"        msg \u003d (\u0027[%(placement_req_id)s] Failed to retrieve usages for project \u0027"},{"line_number":2323,"context_line":"               \u0027%(project_id)s and user %(user_id)s. Got %(status_code)d: \u0027"},{"line_number":2324,"context_line":"               \u0027%(err_text)s\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_c00e8ce4","line":2321,"range":{"start_line":2321,"start_character":8,"end_line":2321,"end_character":43},"in_reply_to":"5fc1f717_e5871ad7","updated":"2019-03-06 23:18:42.000000000","message":"It\u0027s not as clean as I thought it would be, being that we default our ksa adapter to raise_exc\u003dFalse, so I have to add a kwarg to our get() method to pass along. Which I can do and will probably try again to do, now that I know how to get the request_id from the response for the error log message. It\u0027s just last night I spent hours getting ~250 failing unit/func tests to pass on this patch and prioritized the refactor behind that.","commit_id":"6dfc30d0020f3b75cf30c60d85e351994cb34669"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"723e4a0664122130a06bee0ef812940b2970541b","unresolved":false,"context_lines":[{"line_number":2331,"context_line":"        raise exception.UsagesRetrievalFailed(project_id\u003dproject_id,"},{"line_number":2332,"context_line":"                                              user_id\u003duser_id or \u0027N/A\u0027)"},{"line_number":2333,"context_line":""},{"line_number":2334,"context_line":"    def get_usages_counts_for_quota(self, context, project_id, user_id\u003dNone):"},{"line_number":2335,"context_line":"        \"\"\"Get the usages counts for the purpose of counting quota usage."},{"line_number":2336,"context_line":""},{"line_number":2337,"context_line":"        :param context: The request context"}],"source_content_type":"text/x-python","patch_set":19,"id":"5fc1f717_80d084f9","line":2334,"updated":"2019-03-06 23:33:57.000000000","message":"TODO: add @retrying.retry with ksa_exc.ConnectFailure to this","commit_id":"d132c15418ff600097ca83d528c0b374467e7bef"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c90978be7431eb8b73fe6f8dac0b8bca86fc28ba","unresolved":false,"context_lines":[{"line_number":2351,"context_line":"        total_counts \u003d {\u0027project\u0027: {}}"},{"line_number":2352,"context_line":"        url \u003d \u0027/usages?project_id\u003d%s\u0027 % project_id"},{"line_number":2353,"context_line":"        # First query counts across all users of a project"},{"line_number":2354,"context_line":"        resp \u003d self.get(url, version\u003dGET_USAGES_VERSION,"},{"line_number":2355,"context_line":"                        global_request_id\u003dcontext.global_id)"},{"line_number":2356,"context_line":"        if resp.status_code \u003d\u003d 200:"},{"line_number":2357,"context_line":"            data \u003d resp.json()"}],"source_content_type":"text/x-python","patch_set":23,"id":"5fc1f717_f75b07f3","line":2354,"updated":"2019-04-04 17:25:09.000000000","message":"This is not something to do in this patch, but I continue to think the report client would benefit from something like automatic retry loops on GET failures. It\u0027s harder to do well on the writing methods, but for GET we\u0027d like to ride through a brief network partition as if it never happened.\n\nOne could probably argue \"that\u0027s what the load balancer/proxy/whatever is for\". If that\u0027s a sound argument, cool, no point complicating things here.","commit_id":"f140b079481989aeea3c15109da953564abca4f2"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d573ba0182f4b8ebdad8788d6aa9126ad208323","unresolved":false,"context_lines":[{"line_number":2351,"context_line":"        total_counts \u003d {\u0027project\u0027: {}}"},{"line_number":2352,"context_line":"        url \u003d \u0027/usages?project_id\u003d%s\u0027 % project_id"},{"line_number":2353,"context_line":"        # First query counts across all users of a project"},{"line_number":2354,"context_line":"        resp \u003d self.get(url, version\u003dGET_USAGES_VERSION,"},{"line_number":2355,"context_line":"                        global_request_id\u003dcontext.global_id)"},{"line_number":2356,"context_line":"        if resp.status_code \u003d\u003d 200:"},{"line_number":2357,"context_line":"            data \u003d resp.json()"}],"source_content_type":"text/x-python","patch_set":23,"id":"5fc1f717_d489e96e","line":2354,"in_reply_to":"5fc1f717_f75b07f3","updated":"2019-04-09 01:54:22.000000000","message":"Yeah, would seem better than having to decorate every method that does a GET with the ConnectFailure retry.\n\nLooks like we handle automatic retries to glance here:\n\nhttps://github.com/openstack/nova/blob/fb1fee6772bb101eac83845bac9022df77113aaa/nova/image/glance.py#L140\n\nand cinder here:\n\nhttps://github.com/openstack/nova/blob/fb1fee6772bb101eac83845bac9022df77113aaa/nova/volume/cinder.py#L276\n\nand this is how to do retries with neutron, though we\u0027re not using it:\n\nhttps://github.com/openstack/python-neutronclient/blob/f0c886cbe7754f20ef82349bd66435458c2d5587/neutronclient/v2_0/client.py#L220","commit_id":"f140b079481989aeea3c15109da953564abca4f2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":2331,"context_line":"        raise exception.UsagesRetrievalFailed(project_id\u003dproject_id,"},{"line_number":2332,"context_line":"                                              user_id\u003duser_id or \u0027N/A\u0027)"},{"line_number":2333,"context_line":""},{"line_number":2334,"context_line":"    @retrying.retry(stop_max_attempt_number\u003d4,"},{"line_number":2335,"context_line":"                    retry_on_exception\u003dlambda e: isinstance("},{"line_number":2336,"context_line":"                        e, ks_exc.ConnectFailure))"},{"line_number":2337,"context_line":"    def get_usages_counts_for_quota(self, context, project_id, user_id\u003dNone):"},{"line_number":2338,"context_line":"        \"\"\"Get the usages counts for the purpose of counting quota usage."},{"line_number":2339,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_4f0431c7","line":2336,"range":{"start_line":2334,"start_character":4,"end_line":2336,"end_character":50},"updated":"2019-04-10 20:13:55.000000000","message":"I don\u0027t see this tested anywhere.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":2331,"context_line":"        raise exception.UsagesRetrievalFailed(project_id\u003dproject_id,"},{"line_number":2332,"context_line":"                                              user_id\u003duser_id or \u0027N/A\u0027)"},{"line_number":2333,"context_line":""},{"line_number":2334,"context_line":"    @retrying.retry(stop_max_attempt_number\u003d4,"},{"line_number":2335,"context_line":"                    retry_on_exception\u003dlambda e: isinstance("},{"line_number":2336,"context_line":"                        e, ks_exc.ConnectFailure))"},{"line_number":2337,"context_line":"    def get_usages_counts_for_quota(self, context, project_id, user_id\u003dNone):"},{"line_number":2338,"context_line":"        \"\"\"Get the usages counts for the purpose of counting quota usage."},{"line_number":2339,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_92531ce3","line":2336,"range":{"start_line":2334,"start_character":4,"end_line":2336,"end_character":50},"in_reply_to":"3fce034c_4f0431c7","updated":"2019-04-10 20:53:58.000000000","message":"Yeah, sorry. I took it as a given as it\u0027s used elsewhere already. I\u0027ll add something.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":2334,"context_line":"    @retrying.retry(stop_max_attempt_number\u003d4,"},{"line_number":2335,"context_line":"                    retry_on_exception\u003dlambda e: isinstance("},{"line_number":2336,"context_line":"                        e, ks_exc.ConnectFailure))"},{"line_number":2337,"context_line":"    def get_usages_counts_for_quota(self, context, project_id, user_id\u003dNone):"},{"line_number":2338,"context_line":"        \"\"\"Get the usages counts for the purpose of counting quota usage."},{"line_number":2339,"context_line":""},{"line_number":2340,"context_line":"        :param context: The request context"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_0cd077c9","line":2337,"updated":"2019-04-10 20:13:55.000000000","message":"Can we split this part out into it\u0027s own patch below this in the series? The overall change is not exactly small and this seems like simple plumbing that could live in it\u0027s own patch so reviewers don\u0027t have to digest this entire change in a single go, including trying to keep test coverage in mind.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":2334,"context_line":"    @retrying.retry(stop_max_attempt_number\u003d4,"},{"line_number":2335,"context_line":"                    retry_on_exception\u003dlambda e: isinstance("},{"line_number":2336,"context_line":"                        e, ks_exc.ConnectFailure))"},{"line_number":2337,"context_line":"    def get_usages_counts_for_quota(self, context, project_id, user_id\u003dNone):"},{"line_number":2338,"context_line":"        \"\"\"Get the usages counts for the purpose of counting quota usage."},{"line_number":2339,"context_line":""},{"line_number":2340,"context_line":"        :param context: The request context"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_b25098d6","line":2337,"in_reply_to":"3fce034c_0cd077c9","updated":"2019-04-10 20:53:58.000000000","message":"That\u0027s a good idea. I can do that.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":2346,"context_line":"                                 \u0027ram\u0027: \u003ccount across project\u003e},"},{"line_number":2347,"context_line":"                    {\u0027user\u0027: {\u0027cores\u0027: \u003ccount across user\u003e,"},{"line_number":2348,"context_line":"                              \u0027ram\u0027: \u003ccount across user\u003e},"},{"line_number":2349,"context_line":"        :raises: `exception.UsagesRerievalFailed` if a placement API call fails"},{"line_number":2350,"context_line":"        \"\"\""},{"line_number":2351,"context_line":"        total_counts \u003d {\u0027project\u0027: {}}"},{"line_number":2352,"context_line":"        url \u003d \u0027/usages?project_id\u003d%s\u0027 % project_id"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_2cee9388","line":2349,"range":{"start_line":2349,"start_character":28,"end_line":2349,"end_character":48},"updated":"2019-04-10 20:13:55.000000000","message":"UsagesRetrievalFailed","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e18d943135bf8444d196a8a01dcb1312cf319c35","unresolved":false,"context_lines":[{"line_number":2358,"context_line":"        \"\"\""},{"line_number":2359,"context_line":"        total_counts \u003d {\u0027project\u0027: {}}"},{"line_number":2360,"context_line":"        # First query counts across all users of a project"},{"line_number":2361,"context_line":"        LOG.debug(\u0027Getting usages for project_id %s from placement\u0027,"},{"line_number":2362,"context_line":"                  project_id)"},{"line_number":2363,"context_line":"        resp \u003d self._get_usages(context, project_id)"},{"line_number":2364,"context_line":"        if resp:"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_6fd92520","line":2361,"updated":"2019-05-29 15:56:04.000000000","message":"I\u0027m not sure how much we really need these debug statements in here, were they just for while you were doing test and debug? Because they show up quite a bit in a gate run:\n\nhttp://logs.openstack.org/46/653146/7/check/nova-next/691dc82/logs/screen-n-api.txt.gz\n\n\"Getting usages for project_id\" shows up 209 times in there and I\u0027m not sure how useful it is for debug information, especially if we\u0027re not dumping the usage, and considering I can trace the request ID for whatever the nova operation is in the placement-api logs, e.g.:\n\nhttp://logs.openstack.org/46/653146/7/check/nova-next/691dc82/logs/screen-n-api.txt.gz#_May_24_01_42_23_976769\n\nMay 24 01:42:23.976769 ubuntu-bionic-ovh-bhs1-0006505729 devstack@n-api.service[21627]: DEBUG nova.scheduler.client.report [None req-58839743-a9be-48db-bec5-ef144a382e2e tempest-AggregatesAdminTestJSON-1678361571 tempest-AggregatesAdminTestJSON-1678361571] Getting usages for project_id 24f247af7f004b1c9c13d1451fe34517 from placement {{(pid\u003d21629) get_usages_counts_for_quota /opt/stack/new/nova/nova/scheduler/client/report.py:2369}}\n\nhttp://logs.openstack.org/46/653146/7/check/nova-next/691dc82/logs/screen-placement-api.txt.gz#_May_24_01_42_24_388072\n\nMay 24 01:42:24.388072 ubuntu-bionic-ovh-bhs1-0006505729 devstack@placement-api.service[23030]: DEBUG placement.requestlog [req-58839743-a9be-48db-bec5-ef144a382e2e req-b8e099f4-e055-4820-86c6-ed42e46ac4d9 service placement] Starting request: 158.69.71.231 \"GET /placement/usages?project_id\u003d24f247af7f004b1c9c13d1451fe34517\" {{(pid\u003d23032) __call__ /opt/stack/new/placement/placement/requestlog.py:38}}","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"db1a4afb5378d351fad66962aaa098ed59546223","unresolved":false,"context_lines":[{"line_number":2358,"context_line":"        \"\"\""},{"line_number":2359,"context_line":"        total_counts \u003d {\u0027project\u0027: {}}"},{"line_number":2360,"context_line":"        # First query counts across all users of a project"},{"line_number":2361,"context_line":"        LOG.debug(\u0027Getting usages for project_id %s from placement\u0027,"},{"line_number":2362,"context_line":"                  project_id)"},{"line_number":2363,"context_line":"        resp \u003d self._get_usages(context, project_id)"},{"line_number":2364,"context_line":"        if resp:"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_fd5868fa","line":2361,"in_reply_to":"bfb3d3c7_6fd92520","updated":"2019-05-29 17:06:54.000000000","message":"tssurya requested log messages for this in an earlier PS. Because otherwise, you won\u0027t know a call to placement is happening, log-wise, except in the placement-api logs. I convinced myself this is OK because we also have been emitting \"Getting quotas\" debug log messages forever and these are no worse than those, and they coincide with those.\n\nI\u0027m happy to change or remove this again, if everybody can agree.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d64b852bfef5b886cad8ae47f85398955b549712","unresolved":false,"context_lines":[{"line_number":2358,"context_line":"        \"\"\""},{"line_number":2359,"context_line":"        total_counts \u003d {\u0027project\u0027: {}}"},{"line_number":2360,"context_line":"        # First query counts across all users of a project"},{"line_number":2361,"context_line":"        LOG.debug(\u0027Getting usages for project_id %s from placement\u0027,"},{"line_number":2362,"context_line":"                  project_id)"},{"line_number":2363,"context_line":"        resp \u003d self._get_usages(context, project_id)"},{"line_number":2364,"context_line":"        if resp:"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_eda6680f","line":2361,"in_reply_to":"bfb3d3c7_95481c2d","updated":"2019-05-30 13:53:05.000000000","message":"\u003e \u003e I would have maybe done the logging in the quota method that\n \u003e calls\n \u003e \u003e this instead:\n \u003e \u003e\n \u003e \u003e https://review.opendev.org/#/c/638073/31/nova/quota.py@1220\n \u003e \n \u003e If we did that, it wouldn\u0027t necessarily a call to placement. We\u0027d\n \u003e have to guard it with \u0027if CONF.quota.count_usage_from_placement\u0027 to\n \u003e ensure it\u0027s a call to placement we\u0027re logging about. (And I believe\n \u003e that\u0027s what tssurya was interested in, whether we were calling\n \u003e placement).\n \u003e \n \u003e \u003e And only done it once, e.g.:\n \u003e \u003e\n \u003e \u003e if user_id:\n \u003e \u003e LOG.debug(\u0027Getting usages for project_id %s and user_id \u0027\n\nIf you\u0027re logging from _cores_ram_count_placement or _instances_cores_ram_count_api_db_placement you already know you\u0027re going to placement...","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"308926419d6f9460c00c50c212c695fbab54060e","unresolved":false,"context_lines":[{"line_number":2358,"context_line":"        \"\"\""},{"line_number":2359,"context_line":"        total_counts \u003d {\u0027project\u0027: {}}"},{"line_number":2360,"context_line":"        # First query counts across all users of a project"},{"line_number":2361,"context_line":"        LOG.debug(\u0027Getting usages for project_id %s from placement\u0027,"},{"line_number":2362,"context_line":"                  project_id)"},{"line_number":2363,"context_line":"        resp \u003d self._get_usages(context, project_id)"},{"line_number":2364,"context_line":"        if resp:"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_95481c2d","line":2361,"in_reply_to":"bfb3d3c7_ccfb74be","updated":"2019-05-29 23:16:51.000000000","message":"\u003e I would have maybe done the logging in the quota method that calls\n \u003e this instead:\n \u003e \n \u003e https://review.opendev.org/#/c/638073/31/nova/quota.py@1220\n\nIf we did that, it wouldn\u0027t necessarily a call to placement. We\u0027d have to guard it with \u0027if CONF.quota.count_usage_from_placement\u0027 to ensure it\u0027s a call to placement we\u0027re logging about. (And I believe that\u0027s what tssurya was interested in, whether we were calling placement).\n\n \u003e And only done it once, e.g.:\n \u003e \n \u003e if user_id:\n \u003e LOG.debug(\u0027Getting usages for project_id %s and user_id \u0027\n \u003e \u0027%s from placement\u0027, project_id, user_id)\n \u003e else:\n \u003e LOG.debug(\u0027Getting usages for project_id %s from \u0027\n \u003e \u0027placement\u0027, project_id)\n \u003e return PLACEMENT_CLIENT.get_usages_counts_for_quota(...)\n\nI think we shouldn\u0027t do it only once because if there\u0027s user quota set for a quota limit, we\u0027ll actually make two separate calls to placement, not one. Assuming we want to see each individual call to placement logged.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0cb97771ca70ba5e803a9c6c5bb76029112be258","unresolved":false,"context_lines":[{"line_number":2358,"context_line":"        \"\"\""},{"line_number":2359,"context_line":"        total_counts \u003d {\u0027project\u0027: {}}"},{"line_number":2360,"context_line":"        # First query counts across all users of a project"},{"line_number":2361,"context_line":"        LOG.debug(\u0027Getting usages for project_id %s from placement\u0027,"},{"line_number":2362,"context_line":"                  project_id)"},{"line_number":2363,"context_line":"        resp \u003d self._get_usages(context, project_id)"},{"line_number":2364,"context_line":"        if resp:"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_6367e7b8","line":2361,"in_reply_to":"bfb3d3c7_eda6680f","updated":"2019-05-30 15:21:11.000000000","message":"\u003e If you\u0027re logging from _cores_ram_count_placement or\n \u003e _instances_cores_ram_count_api_db_placement you already know you\u0027re\n \u003e going to placement...\n\nOK, I was thinking about it logging in get_usages in nova/quota.py which is placement agnostic (that is where calls are separated between project_id and project_id + user_id and you could see one call vs two calls).\n\nIn _cores_ram_count_placement or _instances_cores_ram_count_api_db_placement, true we will know we\u0027re calling placement but we wouldn\u0027t be able to log each individual placement call at that call site, if there are two placement calls if user_id is involved. That is why I was not considering that area for log message.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dab18a9ad12536b50272cfe90092f9e7ab0e80f1","unresolved":false,"context_lines":[{"line_number":2358,"context_line":"        \"\"\""},{"line_number":2359,"context_line":"        total_counts \u003d {\u0027project\u0027: {}}"},{"line_number":2360,"context_line":"        # First query counts across all users of a project"},{"line_number":2361,"context_line":"        LOG.debug(\u0027Getting usages for project_id %s from placement\u0027,"},{"line_number":2362,"context_line":"                  project_id)"},{"line_number":2363,"context_line":"        resp \u003d self._get_usages(context, project_id)"},{"line_number":2364,"context_line":"        if resp:"}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_ccfb74be","line":2361,"in_reply_to":"bfb3d3c7_fd5868fa","updated":"2019-05-29 21:15:15.000000000","message":"\u003e I convinced myself this is OK because we also have been emitting \"Getting quotas\" debug log messages forever and these are no worse than those, and they coincide with those.\n\nWhile digging through the nova-next n-api logs that enable this flow I noticed the same - the legacy quota stuff is logging similar things.\n\nI would have maybe done the logging in the quota method that calls this instead:\n\nhttps://review.opendev.org/#/c/638073/31/nova/quota.py@1220\n\nAnd only done it once, e.g.:\n\nif user_id:\n    LOG.debug(\u0027Getting usages for project_id %s and user_id \u0027\n              \u0027%s from placement\u0027, project_id, user_id)\nelse:\n    LOG.debug(\u0027Getting usages for project_id %s from \u0027\n              \u0027placement\u0027, project_id)\nreturn PLACEMENT_CLIENT.get_usages_counts_for_quota(...)\n\n\u003e if everybody can agree.\n\nNever going to happen.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"}],"nova/tests/functional/db/test_instance_mapping.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        # in the check (if project_id is not passed)."},{"line_number":232,"context_line":"        create_mapping(queued_for_delete\u003dFalse)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        # Should be False since only instance 3 has user_id set and we\u0027re not"},{"line_number":235,"context_line":"        # filtering on project."},{"line_number":236,"context_line":"        self.assertFalse("},{"line_number":237,"context_line":"            instance_mapping.user_id_queued_for_delete_populated(self.context))"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"        # Should be True because only instance 3 will be considered when we"},{"line_number":240,"context_line":"        # filter on project."}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_4c4bdf86","line":237,"range":{"start_line":234,"start_character":8,"end_line":237,"end_character":79},"updated":"2019-04-10 20:13:55.000000000","message":"Why not run the check after each mapping is created above to make sure the comments are true and the result isn\u0027t because one mapping makes it so but masks an error in the others?","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        # in the check (if project_id is not passed)."},{"line_number":232,"context_line":"        create_mapping(queued_for_delete\u003dFalse)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        # Should be False since only instance 3 has user_id set and we\u0027re not"},{"line_number":235,"context_line":"        # filtering on project."},{"line_number":236,"context_line":"        self.assertFalse("},{"line_number":237,"context_line":"            instance_mapping.user_id_queued_for_delete_populated(self.context))"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"        # Should be True because only instance 3 will be considered when we"},{"line_number":240,"context_line":"        # filter on project."}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_525ff4e4","line":237,"range":{"start_line":234,"start_character":8,"end_line":237,"end_character":79},"in_reply_to":"3fce034c_4c4bdf86","updated":"2019-04-10 20:53:58.000000000","message":"I just didn\u0027t think about it. Can do.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"}],"nova/tests/functional/db/test_quota.py":[{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"e25e234eb5dda7473d6229241042a6a6b7e6594d","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        # Should be False because it\u0027s not deleted and user_id is unmigrated."},{"line_number":168,"context_line":"        self.assertFalse(quota._user_id_queued_for_delete_populated(ctxt))"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        # A non-deleted instance in a different project, should be considered"},{"line_number":171,"context_line":"        # in the check (if project_id is not passed)."},{"line_number":172,"context_line":"        test_instance_mapping.create_mapping(queued_for_delete\u003dFalse)"},{"line_number":173,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"dfbec78f_87dd47e1","line":170,"range":{"start_line":170,"start_character":36,"end_line":170,"end_character":55},"updated":"2019-05-16 14:01:58.000000000","message":"nit: this was a bit confusing since the mapping is still created in the \u0027fake-project\u0027 but later realized this was for the check from \u0027other-project\u0027","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"07ba290e3bb1a37c71e5f7759cbb891be2710068","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        # Should be False because it\u0027s not deleted and user_id is unmigrated."},{"line_number":168,"context_line":"        self.assertFalse(quota._user_id_queued_for_delete_populated(ctxt))"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        # A non-deleted instance in a different project, should be considered"},{"line_number":171,"context_line":"        # in the check (if project_id is not passed)."},{"line_number":172,"context_line":"        test_instance_mapping.create_mapping(queued_for_delete\u003dFalse)"},{"line_number":173,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"bfb3d3c7_40649222","line":170,"range":{"start_line":170,"start_character":36,"end_line":170,"end_character":55},"in_reply_to":"dfbec78f_87dd47e1","updated":"2019-05-22 23:29:20.000000000","message":"I think I did mess up here, was supposed to create this mapping in \u0027other-project\u0027. The comment on L178 indicates so too.","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e18d943135bf8444d196a8a01dcb1312cf319c35","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        self.assertEqual(1536, count[\u0027user\u0027][\u0027ram\u0027])"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def test_user_id_queued_for_delete_populated(self):"},{"line_number":152,"context_line":"        ctxt \u003d context.RequestContext(\u0027fake-user\u0027, \u0027fake-project\u0027)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        # One deleted or SOFT_DELETED instance with user_id\u003dNone, should not be"},{"line_number":155,"context_line":"        # considered by the check."}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_07596dfc","line":152,"range":{"start_line":152,"start_character":8,"end_line":152,"end_character":66},"updated":"2019-05-29 15:56:04.000000000","message":"nit: this is tightly coupled to the project and user used in test_instance_mapping.create_mapping so it would be a bit more solid to re-use the values used to create the mapping, e.g.:\n\nctxt \u003d context.RequestContext(\n    test_instance_mapping.sample_mapping[\u0027user_id\u0027],\n    test_instance_mapping.sample_mapping[\u0027project_id])","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"db1a4afb5378d351fad66962aaa098ed59546223","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        self.assertEqual(1536, count[\u0027user\u0027][\u0027ram\u0027])"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def test_user_id_queued_for_delete_populated(self):"},{"line_number":152,"context_line":"        ctxt \u003d context.RequestContext(\u0027fake-user\u0027, \u0027fake-project\u0027)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        # One deleted or SOFT_DELETED instance with user_id\u003dNone, should not be"},{"line_number":155,"context_line":"        # considered by the check."}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_1dad3c9a","line":152,"range":{"start_line":152,"start_character":8,"end_line":152,"end_character":66},"in_reply_to":"bfb3d3c7_07596dfc","updated":"2019-05-29 17:06:54.000000000","message":"Can change that if I need to respin.","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dab18a9ad12536b50272cfe90092f9e7ab0e80f1","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        self.assertEqual(1536, count[\u0027user\u0027][\u0027ram\u0027])"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def test_user_id_queued_for_delete_populated(self):"},{"line_number":152,"context_line":"        ctxt \u003d context.RequestContext(\u0027fake-user\u0027, \u0027fake-project\u0027)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        # One deleted or SOFT_DELETED instance with user_id\u003dNone, should not be"},{"line_number":155,"context_line":"        # considered by the check."}],"source_content_type":"text/x-python","patch_set":31,"id":"bfb3d3c7_ac15e0e6","line":152,"range":{"start_line":152,"start_character":8,"end_line":152,"end_character":66},"in_reply_to":"bfb3d3c7_1dad3c9a","updated":"2019-05-29 21:15:15.000000000","message":"FUP it.\n\nhttps://review.opendev.org/#/c/662056/","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"}],"nova/tests/functional/regressions/test_bug_1522536.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ec60024325e79f455553ff11b416a39de0ae1e74","unresolved":false,"context_lines":[{"line_number":30,"context_line":"        super(TestServerGet, self).setUp()"},{"line_number":31,"context_line":"        self.useFixture(policy_fixture.RealPolicyFixture())"},{"line_number":32,"context_line":"        self.useFixture(nova_fixtures.NeutronFixture(self))"},{"line_number":33,"context_line":"        self.useFixture(func_fixtures.PlacementFixture())"},{"line_number":34,"context_line":"        api_fixture \u003d self.useFixture(nova_fixtures.OSAPIFixture("},{"line_number":35,"context_line":"            api_version\u003d\u0027v2.1\u0027))"},{"line_number":36,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3fce034c_aefae9b9","line":33,"updated":"2019-04-18 13:04:33.000000000","message":"Are these needed now the new quota mode is off by default?","commit_id":"40a4a1abd894d11a53d596df70cd3184541f87d1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9952b783b5630eba8b4162243438d59194641fd2","unresolved":false,"context_lines":[{"line_number":30,"context_line":"        super(TestServerGet, self).setUp()"},{"line_number":31,"context_line":"        self.useFixture(policy_fixture.RealPolicyFixture())"},{"line_number":32,"context_line":"        self.useFixture(nova_fixtures.NeutronFixture(self))"},{"line_number":33,"context_line":"        self.useFixture(func_fixtures.PlacementFixture())"},{"line_number":34,"context_line":"        api_fixture \u003d self.useFixture(nova_fixtures.OSAPIFixture("},{"line_number":35,"context_line":"            api_version\u003d\u0027v2.1\u0027))"},{"line_number":36,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3fce034c_f732005e","line":33,"in_reply_to":"3fce034c_aefae9b9","updated":"2019-04-18 15:14:47.000000000","message":"No, they\u0027re not. Sorry about that. This patch began with counting from placement being \u0027on\u0027 by default and has since changed to \u0027off\u0027 by default.","commit_id":"40a4a1abd894d11a53d596df70cd3184541f87d1"}],"nova/tests/unit/api/openstack/compute/test_serversV21.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":3878,"context_line":"        self.assertEqual(response[\u0027server\u0027][\u0027status\u0027], \u0027SHUTOFF\u0027)"},{"line_number":3879,"context_line":""},{"line_number":3880,"context_line":""},{"line_number":3881,"context_line":"@mock.patch(\u0027nova.compute.utils.check_num_instances_quota\u0027,"},{"line_number":3882,"context_line":"            new\u003dlambda *args, **kwargs: 1)"},{"line_number":3883,"context_line":"class ServersControllerCreateTest(test.TestCase):"},{"line_number":3884,"context_line":"    image_uuid \u003d \u002776fa36fc-c930-4bf3-8c8a-ea2a2420deb6\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_0f562908","line":3881,"updated":"2019-04-10 20:13:55.000000000","message":"Why does this need to be mocked out now here and below? Because the unit tests aren\u0027t using a placement fixture and since they are extending TestCase they had access to a default database which just didn\u0027t return anything before wrt usage?","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":3878,"context_line":"        self.assertEqual(response[\u0027server\u0027][\u0027status\u0027], \u0027SHUTOFF\u0027)"},{"line_number":3879,"context_line":""},{"line_number":3880,"context_line":""},{"line_number":3881,"context_line":"@mock.patch(\u0027nova.compute.utils.check_num_instances_quota\u0027,"},{"line_number":3882,"context_line":"            new\u003dlambda *args, **kwargs: 1)"},{"line_number":3883,"context_line":"class ServersControllerCreateTest(test.TestCase):"},{"line_number":3884,"context_line":"    image_uuid \u003d \u002776fa36fc-c930-4bf3-8c8a-ea2a2420deb6\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_b2fe58ac","line":3881,"in_reply_to":"3fce034c_0f562908","updated":"2019-04-10 20:53:58.000000000","message":"Correct, no placement fixture so we get failure trying to talk to placement. I believe so. Let me try adding the placement fixture to see if that also resolves the issue. I was struggling with making all of this work and don\u0027t remember why I did this.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f73bd7aed2cf8ae71c6625cd9f3dc5ee214ac544","unresolved":false,"context_lines":[{"line_number":3878,"context_line":"        self.assertEqual(response[\u0027server\u0027][\u0027status\u0027], \u0027SHUTOFF\u0027)"},{"line_number":3879,"context_line":""},{"line_number":3880,"context_line":""},{"line_number":3881,"context_line":"@mock.patch(\u0027nova.compute.utils.check_num_instances_quota\u0027,"},{"line_number":3882,"context_line":"            new\u003dlambda *args, **kwargs: 1)"},{"line_number":3883,"context_line":"class ServersControllerCreateTest(test.TestCase):"},{"line_number":3884,"context_line":"    image_uuid \u003d \u002776fa36fc-c930-4bf3-8c8a-ea2a2420deb6\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_47596dfc","line":3881,"in_reply_to":"3fce034c_27478120","updated":"2019-04-11 10:43:23.000000000","message":"Disregard, was responding to an older comment.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"5715db9b73c1264a1875f77a8d1bef5e294ff966","unresolved":false,"context_lines":[{"line_number":3878,"context_line":"        self.assertEqual(response[\u0027server\u0027][\u0027status\u0027], \u0027SHUTOFF\u0027)"},{"line_number":3879,"context_line":""},{"line_number":3880,"context_line":""},{"line_number":3881,"context_line":"@mock.patch(\u0027nova.compute.utils.check_num_instances_quota\u0027,"},{"line_number":3882,"context_line":"            new\u003dlambda *args, **kwargs: 1)"},{"line_number":3883,"context_line":"class ServersControllerCreateTest(test.TestCase):"},{"line_number":3884,"context_line":"    image_uuid \u003d \u002776fa36fc-c930-4bf3-8c8a-ea2a2420deb6\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_27478120","line":3881,"in_reply_to":"3fce034c_7b2f6bf6","updated":"2019-04-11 10:41:54.000000000","message":"If you\u0027re finding yourself wanting to use PlacementFixture, perhaps it means this should be moved to functional.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1d35581a5575b7811c4d4876ce9a1a605e906194","unresolved":false,"context_lines":[{"line_number":3878,"context_line":"        self.assertEqual(response[\u0027server\u0027][\u0027status\u0027], \u0027SHUTOFF\u0027)"},{"line_number":3879,"context_line":""},{"line_number":3880,"context_line":""},{"line_number":3881,"context_line":"@mock.patch(\u0027nova.compute.utils.check_num_instances_quota\u0027,"},{"line_number":3882,"context_line":"            new\u003dlambda *args, **kwargs: 1)"},{"line_number":3883,"context_line":"class ServersControllerCreateTest(test.TestCase):"},{"line_number":3884,"context_line":"    image_uuid \u003d \u002776fa36fc-c930-4bf3-8c8a-ea2a2420deb6\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_7b2f6bf6","line":3881,"in_reply_to":"3fce034c_7b8e8b2f","updated":"2019-04-11 00:37:45.000000000","message":"Maybe I\u0027ll just punt and self.flags to configure legacy quota counting. It seems like it would be more useful/more code coverage than just faking out all of the counting anyway.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6297ebeadd65eb7f3aa7fc590c719e5b1de0daad","unresolved":false,"context_lines":[{"line_number":3878,"context_line":"        self.assertEqual(response[\u0027server\u0027][\u0027status\u0027], \u0027SHUTOFF\u0027)"},{"line_number":3879,"context_line":""},{"line_number":3880,"context_line":""},{"line_number":3881,"context_line":"@mock.patch(\u0027nova.compute.utils.check_num_instances_quota\u0027,"},{"line_number":3882,"context_line":"            new\u003dlambda *args, **kwargs: 1)"},{"line_number":3883,"context_line":"class ServersControllerCreateTest(test.TestCase):"},{"line_number":3884,"context_line":"    image_uuid \u003d \u002776fa36fc-c930-4bf3-8c8a-ea2a2420deb6\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_7b8e8b2f","line":3881,"in_reply_to":"3fce034c_b2fe58ac","updated":"2019-04-11 00:18:58.000000000","message":"Apparently it\u0027s illegal to use PlacementFixture in unit tests:\n\n ImportError: No module named placement.tests.functional.fixtures\n\nNone of the unit tests can tolerate a call to placement and a lot of these tests are relying on local database quota counts. So I have to do something here. I\u0027ll try one of your ideas to make it less ugly.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":5247,"context_line":""},{"line_number":5248,"context_line":"        self.assertEqual(encodeutils.safe_decode(robj[\u0027Location\u0027]), selfhref)"},{"line_number":5249,"context_line":""},{"line_number":5250,"context_line":"    # This works to use the real check_num_instances_quota method because"},{"line_number":5251,"context_line":"    # the mock.patch class decorator only decorates test_* named methods."},{"line_number":5252,"context_line":"    @mock.patch(\u0027nova.compute.utils.check_num_instances_quota\u0027,"},{"line_number":5253,"context_line":"                nova.compute.utils.check_num_instances_quota)"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_8c06a720","line":5250,"range":{"start_line":5250,"start_character":11,"end_line":5250,"end_character":16},"updated":"2019-04-10 20:13:55.000000000","message":"needs? I\u0027m not really sure what this is trying to do. Are you trying to mock out check_num_instances_quota but have to do it explicitly here because it\u0027s not on the private method or what? If so, can we just create a new fixture and use that in each test that needs it rather than decorate the test class and individual test methods as well? Seems very noisy this way.\n\nOr is this a case where you\u0027re trying to undo the mock because this test actually needs the real thing? If that\u0027s the case, it might be cleaner to split the handful of tests that need the real thing into new test classes and then we don\u0027t need to unset stubs and such (and you could create a fixture and re-use that everywhere rather than the mock class decorators in this module and the rest of this change). Maybe that\u0027s too much change though, idk - could be harder to review that way in a single patch.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":5247,"context_line":""},{"line_number":5248,"context_line":"        self.assertEqual(encodeutils.safe_decode(robj[\u0027Location\u0027]), selfhref)"},{"line_number":5249,"context_line":""},{"line_number":5250,"context_line":"    # This works to use the real check_num_instances_quota method because"},{"line_number":5251,"context_line":"    # the mock.patch class decorator only decorates test_* named methods."},{"line_number":5252,"context_line":"    @mock.patch(\u0027nova.compute.utils.check_num_instances_quota\u0027,"},{"line_number":5253,"context_line":"                nova.compute.utils.check_num_instances_quota)"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_d220c44c","line":5250,"range":{"start_line":5250,"start_character":11,"end_line":5250,"end_character":16},"in_reply_to":"3fce034c_8c06a720","updated":"2019-04-10 20:53:58.000000000","message":"Yeah, let me digest all this and try to make it cleaner in the next PS. Thanks for the ideas.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"36c5f0c90f0409922024563ed5f7b98dc73d7319","unresolved":false,"context_lines":[{"line_number":6461,"context_line":""},{"line_number":6462,"context_line":""},{"line_number":6463,"context_line":"@mock.patch(\u0027nova.compute.utils.check_num_instances_quota\u0027,"},{"line_number":6464,"context_line":"            new\u003dlambda *args, **kwargs: 1)"},{"line_number":6465,"context_line":"class ServersControllerCreateTestV260(test.NoDBTestCase):"},{"line_number":6466,"context_line":"    \"\"\"Negative tests for creating a server with a multiattach volume.\"\"\""},{"line_number":6467,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_5b94efe8","line":6464,"updated":"2019-04-11 00:42:31.000000000","message":"This is actually what I modeled all my other mocking around. Now I see that the reason it was added was to avoid database access by the quota counting since this test case derives from test.NoDBTestCase.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4ac77c7142739df667cab0be7825a1ac6c19090b","unresolved":false,"context_lines":[{"line_number":6461,"context_line":""},{"line_number":6462,"context_line":""},{"line_number":6463,"context_line":"@mock.patch(\u0027nova.compute.utils.check_num_instances_quota\u0027,"},{"line_number":6464,"context_line":"            new\u003dlambda *args, **kwargs: 1)"},{"line_number":6465,"context_line":"class ServersControllerCreateTestV260(test.NoDBTestCase):"},{"line_number":6466,"context_line":"    \"\"\"Negative tests for creating a server with a multiattach volume.\"\"\""},{"line_number":6467,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_5b4b8f86","line":6464,"in_reply_to":"3fce034c_5b94efe8","updated":"2019-04-11 00:52:12.000000000","message":"And, turns out you don\u0027t need it because of the NoopQuotaDriverFixture. That avoids all quota DB access on its own. I just tried removing this locally and the test still works fine.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"}],"nova/tests/unit/scheduler/client/test_report.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8cbc2da8842e92ab1024893b0a013db2200d316b","unresolved":false,"context_lines":[{"line_number":3596,"context_line":"        mock_get.reset_mock()"},{"line_number":3597,"context_line":"        fake_good_response \u003d fake_requests.FakeResponse("},{"line_number":3598,"context_line":"            200, content\u003djsonutils.dumps("},{"line_number":3599,"context_line":"                {\u0027usages\u0027: {fields.ResourceClass.VCPU: 2,"},{"line_number":3600,"context_line":"                            fields.ResourceClass.MEMORY_MB: 512}}))"},{"line_number":3601,"context_line":"        mock_get.side_effect \u003d [fake_good_response,"},{"line_number":3602,"context_line":"                                fake_requests.FakeResponse(500, content\u003d\u0027err\u0027)]"}],"source_content_type":"text/x-python","patch_set":14,"id":"9fdfeff1_6d8242a8","line":3599,"updated":"2019-03-04 12:33:10.000000000","message":"dittoish","commit_id":"fe7823a972b57e26ec0533e35dccf5bbba625176"}],"nova/tests/unit/test_quota.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":1985,"context_line":"    @mock.patch(\u0027nova.quota._instances_cores_ram_count_legacy\u0027)"},{"line_number":1986,"context_line":"    @mock.patch(\u0027nova.objects.InstanceMappingList.get_counts\u0027)"},{"line_number":1987,"context_line":"    @mock.patch(\u0027nova.quota._cores_ram_count_placement\u0027)"},{"line_number":1988,"context_line":"    def test_instances_cores_ram_count(self, disable_quota_from_placement,"},{"line_number":1989,"context_line":"                                       uid_qfd_populated, mock_placement_count,"},{"line_number":1990,"context_line":"                                       mock_get_im_count, mock_legacy_count,"},{"line_number":1991,"context_line":"                                       mock_uid_qfd_populated, mock_debug_log):"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_4f2d1145","line":1988,"range":{"start_line":1988,"start_character":8,"end_line":1988,"end_character":38},"updated":"2019-04-10 20:13:55.000000000","message":"This test could use some inline comments for documenting what\u0027s going on.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":1985,"context_line":"    @mock.patch(\u0027nova.quota._instances_cores_ram_count_legacy\u0027)"},{"line_number":1986,"context_line":"    @mock.patch(\u0027nova.objects.InstanceMappingList.get_counts\u0027)"},{"line_number":1987,"context_line":"    @mock.patch(\u0027nova.quota._cores_ram_count_placement\u0027)"},{"line_number":1988,"context_line":"    def test_instances_cores_ram_count(self, disable_quota_from_placement,"},{"line_number":1989,"context_line":"                                       uid_qfd_populated, mock_placement_count,"},{"line_number":1990,"context_line":"                                       mock_get_im_count, mock_legacy_count,"},{"line_number":1991,"context_line":"                                       mock_uid_qfd_populated, mock_debug_log):"}],"source_content_type":"text/x-python","patch_set":24,"id":"3fce034c_b21738ee","line":1988,"range":{"start_line":1988,"start_character":8,"end_line":1988,"end_character":38},"in_reply_to":"3fce034c_4f2d1145","updated":"2019-04-10 20:53:58.000000000","message":"Can do.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5d6f97368180ead70591bea781cd1a1670593b89","unresolved":false,"context_lines":[{"line_number":2075,"context_line":"            self, mock_api_db_placement_count, mock_legacy_count,"},{"line_number":2076,"context_line":"            mock_uid_qfd_populated):"},{"line_number":2077,"context_line":"        # We need quota usage from placement enabled to test this. For legacy"},{"line_number":2078,"context_line":"        # counting, the cache is not useful."},{"line_number":2079,"context_line":"        self.flags(count_usage_from_placement\u003dTrue, group\u003d\u0027quota\u0027)"},{"line_number":2080,"context_line":"        # Fake count of instances, cores, and ram."},{"line_number":2081,"context_line":"        fake_counts \u003d {\u0027project\u0027: {\u0027instances\u0027: 2, \u0027cores\u0027: 2, \u0027ram\u0027: 4},"}],"source_content_type":"text/x-python","patch_set":28,"id":"3fce034c_03b50ecd","line":2078,"range":{"start_line":2078,"start_character":37,"end_line":2078,"end_character":43},"updated":"2019-04-18 18:28:25.000000000","message":"used","commit_id":"178fad2395374a463cd43133003520fbda581cbc"}],"releasenotes/notes/quota-usage-placement-5b3f62e83056f59d.yaml":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c90978be7431eb8b73fe6f8dac0b8bca86fc28ba","unresolved":false,"context_lines":[{"line_number":12,"context_line":"    multiple Nova deployments that share a placement deployment should set the"},{"line_number":13,"context_line":"    ``[workarounds]disable_quota_usage_from_placement`` configuration option to"},{"line_number":14,"context_line":"    ``True`` in order to use the legacy quota usage counting method that counts"},{"line_number":15,"context_line":"    resource from cell databases."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"    Note that counting quota usage from placement depends on the completion of"},{"line_number":18,"context_line":"    an online data migration. Until the data migration is complete, the system"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"5fc1f717_d797eb39","line":15,"updated":"2019-04-04 17:25:09.000000000","message":"Out of curiosity, do we have people who are already doing this or is it more something that we are expecting to happen?","commit_id":"f140b079481989aeea3c15109da953564abca4f2"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d573ba0182f4b8ebdad8788d6aa9126ad208323","unresolved":false,"context_lines":[{"line_number":12,"context_line":"    multiple Nova deployments that share a placement deployment should set the"},{"line_number":13,"context_line":"    ``[workarounds]disable_quota_usage_from_placement`` configuration option to"},{"line_number":14,"context_line":"    ``True`` in order to use the legacy quota usage counting method that counts"},{"line_number":15,"context_line":"    resource from cell databases."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"    Note that counting quota usage from placement depends on the completion of"},{"line_number":18,"context_line":"    an online data migration. Until the data migration is complete, the system"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"5fc1f717_cfd2af34","line":15,"in_reply_to":"5fc1f717_d797eb39","updated":"2019-04-09 01:54:22.000000000","message":"Not that I know of. The most feasible place I could see it possibly happening is in an Edge™ environment, but AFAIK the recommended edge deployment topologies are running a nova deployment one-to-one with a placement deployment.\n\nWhen I first proposed the idea of counting quota usage from placement, dansmith pointed out the potential for a problem if multiple nova deployments are using a single placement deployment, so I put warnings and workaround here to be safer than sorrier.","commit_id":"f140b079481989aeea3c15109da953564abca4f2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":1,"context_line":"upgrade:"},{"line_number":2,"context_line":"  - |"},{"line_number":3,"context_line":"    Quota usage for VCPU and MEMORY_MB are now counted from the placement"},{"line_number":4,"context_line":"    service and instances counted from instance mappings in the API database"}],"source_content_type":"text/x-yaml","patch_set":24,"id":"3fce034c_cf93415c","line":1,"updated":"2019-04-10 20:13:55.000000000","message":"This could be a follow up but it would be good to document something about this:\n\nhttps://docs.openstack.org/nova/latest/user/quotas.html\n\nThe spec also called out updating this doc:\n\nhttps://docs.openstack.org/nova/latest/user/cellsv2-layout.html#quota-related-quirks","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":1,"context_line":"upgrade:"},{"line_number":2,"context_line":"  - |"},{"line_number":3,"context_line":"    Quota usage for VCPU and MEMORY_MB are now counted from the placement"},{"line_number":4,"context_line":"    service and instances counted from instance mappings in the API database"}],"source_content_type":"text/x-yaml","patch_set":24,"id":"3fce034c_d2e5240c","line":1,"in_reply_to":"3fce034c_cf93415c","updated":"2019-04-10 20:53:58.000000000","message":"Ack, will do probably another patch to avoid making this patch bigger.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d16e492a0ede733f70b757584fbe20b0818baea9","unresolved":false,"context_lines":[{"line_number":1,"context_line":"upgrade:"},{"line_number":2,"context_line":"  - |"},{"line_number":3,"context_line":"    Quota usage for VCPU and MEMORY_MB are now counted from the placement"},{"line_number":4,"context_line":"    service and instances counted from instance mappings in the API database"}],"source_content_type":"text/x-yaml","patch_set":24,"id":"3fce034c_38ff9e68","line":1,"in_reply_to":"3fce034c_d2e5240c","updated":"2019-04-11 13:51:25.000000000","message":"Forgot but #2 here as well:\n\nhttps://docs.openstack.org/nova/latest/admin/cells.html#known-issues","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":1,"context_line":"upgrade:"},{"line_number":2,"context_line":"  - |"},{"line_number":3,"context_line":"    Quota usage for VCPU and MEMORY_MB are now counted from the placement"},{"line_number":4,"context_line":"    service and instances counted from instance mappings in the API database"},{"line_number":5,"context_line":"    instead of counting resources from cell databases. This makes quota usage"},{"line_number":6,"context_line":"    counting resilient in the presence of down or poor-performing cells."}],"source_content_type":"text/x-yaml","patch_set":24,"id":"3fce034c_2fba05de","line":3,"range":{"start_line":3,"start_character":20,"end_line":3,"end_character":38},"updated":"2019-04-10 20:13:55.000000000","message":"nit: probably better to use the terms from the actual quota API so \u0027cores\u0027 and \u0027ram\u0027.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":1,"context_line":"upgrade:"},{"line_number":2,"context_line":"  - |"},{"line_number":3,"context_line":"    Quota usage for VCPU and MEMORY_MB are now counted from the placement"},{"line_number":4,"context_line":"    service and instances counted from instance mappings in the API database"},{"line_number":5,"context_line":"    instead of counting resources from cell databases. This makes quota usage"},{"line_number":6,"context_line":"    counting resilient in the presence of down or poor-performing cells."}],"source_content_type":"text/x-yaml","patch_set":24,"id":"3fce034c_f2e2a001","line":3,"range":{"start_line":3,"start_character":20,"end_line":3,"end_character":38},"in_reply_to":"3fce034c_2fba05de","updated":"2019-04-10 20:53:58.000000000","message":"OK, can do.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":7,"context_line":""},{"line_number":8,"context_line":"    Note that counting quota usage from placement will not work properly in an"},{"line_number":9,"context_line":"    environment where multiple Nova deployments are sharing a placement"},{"line_number":10,"context_line":"    deployment because currently, placement has no way of partitioning resource"},{"line_number":11,"context_line":"    allocations between different Nova deployments. Operators who are running"},{"line_number":12,"context_line":"    multiple Nova deployments that share a placement deployment should set the"},{"line_number":13,"context_line":"    ``[workarounds]disable_quota_usage_from_placement`` configuration option to"}],"source_content_type":"text/x-yaml","patch_set":24,"id":"3fce034c_6fd7ad25","line":10,"range":{"start_line":10,"start_character":32,"end_line":10,"end_character":33},"updated":"2019-04-10 20:13:55.000000000","message":"nix","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cddeb6366651aa96cb5848e61678343883b4411d","unresolved":false,"context_lines":[{"line_number":15,"context_line":"    resource from cell databases."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"    Note that counting quota usage from placement depends on the completion of"},{"line_number":18,"context_line":"    an online data migration. Until the data migration is complete, the system"},{"line_number":19,"context_line":"    will fall back on legacy quota usage counting from cell databases depending"},{"line_number":20,"context_line":"    on the result of an EXISTS database query during each quota check."},{"line_number":21,"context_line":"    Operators who need to avoid the performance hit from the EXISTS query may"}],"source_content_type":"text/x-yaml","patch_set":24,"id":"3fce034c_afc5955d","line":18,"range":{"start_line":18,"start_character":4,"end_line":18,"end_character":27},"updated":"2019-04-10 20:13:55.000000000","message":"Probably would be good to call out the name of the online data migration.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f5683ed0cd60cb5490fb85fcf11d7bdf9a6f985","unresolved":false,"context_lines":[{"line_number":15,"context_line":"    resource from cell databases."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"    Note that counting quota usage from placement depends on the completion of"},{"line_number":18,"context_line":"    an online data migration. Until the data migration is complete, the system"},{"line_number":19,"context_line":"    will fall back on legacy quota usage counting from cell databases depending"},{"line_number":20,"context_line":"    on the result of an EXISTS database query during each quota check."},{"line_number":21,"context_line":"    Operators who need to avoid the performance hit from the EXISTS query may"}],"source_content_type":"text/x-yaml","patch_set":24,"id":"3fce034c_92ef9ce8","line":18,"range":{"start_line":18,"start_character":4,"end_line":18,"end_character":27},"in_reply_to":"3fce034c_afc5955d","updated":"2019-04-10 20:53:58.000000000","message":"Will do.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"00961009a5926c5e7d37dee727dd7347a8b22252","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"3fce034c_0dcfff7f","line":25,"updated":"2019-04-11 18:34:53.000000000","message":"Another thing that will come up with this is a change in behavior for our instances in VERIFY_RESIZE status will have vcpus/ram counted as discussed in IRC:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-04-11.log.html#t2019-04-11T18:22:20\n\nThat should probably go in the release note and/or docs somewhere, and in the spec if you end up needing to amend the spec.","commit_id":"bd7c4fb65c1698dd00b62ab4650e8f6dff903062"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"06ecdd3f01f694925403bc9cb8b14c87e711000c","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    Operators who want to avoid the performance hit from the EXISTS queries"},{"line_number":33,"context_line":"    should wait to set the ``[quota]count_usage_from_placement`` configuration"},{"line_number":34,"context_line":"    option to ``True`` until after they have completed their online data"},{"line_number":35,"context_line":"    migrations via ``nova-manage db online_data_migrations``."}],"source_content_type":"text/x-yaml","patch_set":26,"id":"3fce034c_484920ce","line":35,"updated":"2019-04-17 00:37:52.000000000","message":"mriedem mentioned on the ML [1] the change in behavior that happens with ERROR instances that have never been scheduled. Today, they consume quota usage for instances, cores, and ram until they are deleted. When we count cores and ram from placement, they will not consume cores and ram quota usage because they have no placement allocations. Yet another thing to add to the release note pile and config option help.\n\n[1] http://lists.openstack.org/pipermail/openstack-discuss/2019-April/005234.html","commit_id":"c1b997be1734a0c853c65b95e029a1d0335481f1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"546400bc07aa795f9564c9baf6665793bb636033","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    Operators who want to avoid the performance hit from the EXISTS queries"},{"line_number":33,"context_line":"    should wait to set the ``[quota]count_usage_from_placement`` configuration"},{"line_number":34,"context_line":"    option to ``True`` until after they have completed their online data"},{"line_number":35,"context_line":"    migrations via ``nova-manage db online_data_migrations``."}],"source_content_type":"text/x-yaml","patch_set":26,"id":"3fce034c_6e565d5f","line":35,"in_reply_to":"3fce034c_23e14ed1","updated":"2019-04-17 09:32:09.000000000","message":"This is something I\u0027ve never quite understood: If an instance goes into ERROR state, why is the user who is trying to place the VM penalized (by having their quota used up) for the error?","commit_id":"c1b997be1734a0c853c65b95e029a1d0335481f1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"73321ec3b6e26e2175c016c8b7d2d13bee0abdea","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    Operators who want to avoid the performance hit from the EXISTS queries"},{"line_number":33,"context_line":"    should wait to set the ``[quota]count_usage_from_placement`` configuration"},{"line_number":34,"context_line":"    option to ``True`` until after they have completed their online data"},{"line_number":35,"context_line":"    migrations via ``nova-manage db online_data_migrations``."}],"source_content_type":"text/x-yaml","patch_set":26,"id":"3fce034c_891dc310","line":35,"in_reply_to":"3fce034c_29036fba","updated":"2019-04-17 10:11:56.000000000","message":"Yeah, that makes sense, but it raises the next question: Why is a VM in ERROR state the responsibility of the requestor? Much of the time it is in that state because the cloud is inadequate or broken in some way, yes? If that\u0027s the case it should be the cloud\u0027s responsibility to clean it up and account for it.\n\nI guess this is a significant factor in what\u0027s driving the PENDING state (which I think is a fine idea).","commit_id":"c1b997be1734a0c853c65b95e029a1d0335481f1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"8bfee91d0afa696da9e7c8f78f1fb0af15c61e2f","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    Operators who want to avoid the performance hit from the EXISTS queries"},{"line_number":33,"context_line":"    should wait to set the ``[quota]count_usage_from_placement`` configuration"},{"line_number":34,"context_line":"    option to ``True`` until after they have completed their online data"},{"line_number":35,"context_line":"    migrations via ``nova-manage db online_data_migrations``."}],"source_content_type":"text/x-yaml","patch_set":26,"id":"3fce034c_286a70ed","line":35,"in_reply_to":"3fce034c_31ee62ed","updated":"2019-04-18 12:35:27.000000000","message":"FWIW, I remember a denial of service argument for this behaviour. If you find a bug that gets your instance into the Error state (such as uploading yourself a rubbish image), you don\u0027t want the user retrying for ever and filling up the DB.\n\nIn my head it was accidental behaviour that was later considered useful, but I don\u0027t really remember my conversion with Kevin about why he did it that way.","commit_id":"c1b997be1734a0c853c65b95e029a1d0335481f1"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"fc759a9f90ddcba027c3a177533a76e1286b3dc6","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    Operators who want to avoid the performance hit from the EXISTS queries"},{"line_number":33,"context_line":"    should wait to set the ``[quota]count_usage_from_placement`` configuration"},{"line_number":34,"context_line":"    option to ``True`` until after they have completed their online data"},{"line_number":35,"context_line":"    migrations via ``nova-manage db online_data_migrations``."}],"source_content_type":"text/x-yaml","patch_set":26,"id":"3fce034c_23e14ed1","line":35,"in_reply_to":"3fce034c_484920ce","updated":"2019-04-17 08:30:18.000000000","message":"oh nice catch, but shouldn\u0027t this be the correct behaviour? As in if an instance goes into ERROR state immediately without going through scheduler host selection, we should count the instance but not count cores and RAM right ?","commit_id":"c1b997be1734a0c853c65b95e029a1d0335481f1"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"3b007ad531c016b9d2475f8b8c44ae8bd1bf0442","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    Operators who want to avoid the performance hit from the EXISTS queries"},{"line_number":33,"context_line":"    should wait to set the ``[quota]count_usage_from_placement`` configuration"},{"line_number":34,"context_line":"    option to ``True`` until after they have completed their online data"},{"line_number":35,"context_line":"    migrations via ``nova-manage db online_data_migrations``."}],"source_content_type":"text/x-yaml","patch_set":26,"id":"3fce034c_29036fba","line":35,"in_reply_to":"3fce034c_6e565d5f","updated":"2019-04-17 10:08:10.000000000","message":"@cdent: I agree that the RAM and cores shouldn\u0027t be counted and user shouldn\u0027t suffer on the quota cap for cores/ram. But I feel we should include the error instance for the quota on the number of instances as an incentive for the user to delete it at some point. Else the error instances that never got scheduled would be listed for the user always and if not counted in quota they won\u0027t bother cleaning it up (at least I wouldn\u0027t :D).","commit_id":"c1b997be1734a0c853c65b95e029a1d0335481f1"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"275d858addd28f957117eb8eaacd35523c8d0ede","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    Operators who want to avoid the performance hit from the EXISTS queries"},{"line_number":33,"context_line":"    should wait to set the ``[quota]count_usage_from_placement`` configuration"},{"line_number":34,"context_line":"    option to ``True`` until after they have completed their online data"},{"line_number":35,"context_line":"    migrations via ``nova-manage db online_data_migrations``."}],"source_content_type":"text/x-yaml","patch_set":26,"id":"3fce034c_d4703edf","line":35,"in_reply_to":"3fce034c_891dc310","updated":"2019-04-17 11:33:38.000000000","message":"\u003e Yeah, that makes sense, but it raises the next question: Why is a\n \u003e VM in ERROR state the responsibility of the requestor? Much of the\n \u003e time it is in that state because the cloud is inadequate or broken\n \u003e in some way, yes? If that\u0027s the case it should be the cloud\u0027s\n \u003e responsibility to clean it up and account for it.\n\nagreed. It shouldn\u0027t be a user responsibility, should be the operators cleaning it if the mess was created by them which lead to the instance going into cell0.\n\n \u003e \n \u003e I guess this is a significant factor in what\u0027s driving the PENDING\n \u003e state (which I think is a fine idea).\n\nah yes you mean the operators can now do whatever they want like automatically deleting the instance forever etc etc from PENDING state?","commit_id":"c1b997be1734a0c853c65b95e029a1d0335481f1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"79236d170d6d0b93dba3cc982b8934d69f924a1b","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    Operators who want to avoid the performance hit from the EXISTS queries"},{"line_number":33,"context_line":"    should wait to set the ``[quota]count_usage_from_placement`` configuration"},{"line_number":34,"context_line":"    option to ``True`` until after they have completed their online data"},{"line_number":35,"context_line":"    migrations via ``nova-manage db online_data_migrations``."}],"source_content_type":"text/x-yaml","patch_set":26,"id":"3fce034c_31ee62ed","line":35,"in_reply_to":"3fce034c_d4703edf","updated":"2019-04-18 01:05:02.000000000","message":"This behavior is super legacy nova. I\u0027m not sure whether there was a conscious decision around counting unscheduled instances in ERROR state against quota usage. Quota has always been counted based on instance flavor, so if the instance record exists (deleted\u003d0), it gets counted against quota usage. True, it might incentivize end users to delete the instance.\n\nAs for why an unscheduled ERROR instance is the end user (or operator\u0027s) responsibility, 1) it\u0027s super legacy and 2) we\u0027ve no other way of providing feedback to the user about why their instance failed to build, if we were to delete it automatically in nova. Most of the instance create process is asynchronous and we return HTTP 202. After that, the user expects an instance to be somewhere. Because if they get 202 and then \u0027nova list\u0027 and see nothing, they\u0027re going to be confused and have no way of knowing what happened.\n\nBack in the day, alaski had an idea about how to fix this (tasks API) and wrote up the following backlog spec explaining it:\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/backlog/approved/instance-tasks.html","commit_id":"c1b997be1734a0c853c65b95e029a1d0335481f1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"97a6196088fd2f9e3b911740f90fb7e8089edc6b","unresolved":false,"context_lines":[{"line_number":1,"context_line":"upgrade:"},{"line_number":2,"context_line":"  - |"},{"line_number":3,"context_line":"    It is now possible to count quota usage for cores and ram from the"},{"line_number":4,"context_line":"    placement service and instances from instance mappings in the API database"}],"source_content_type":"text/x-yaml","patch_set":28,"id":"3fce034c_a3a43aa7","line":1,"updated":"2019-04-18 18:26:53.000000000","message":"Don\u0027t forget docs at some point in the series:\n\nhttps://review.openstack.org/#/c/638073/24/releasenotes/notes/quota-usage-placement-5b3f62e83056f59d.yaml","commit_id":"178fad2395374a463cd43133003520fbda581cbc"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"aa8d36212b9562aa8510b5501d731c28c42f992e","unresolved":false,"context_lines":[{"line_number":1,"context_line":"upgrade:"},{"line_number":2,"context_line":"  - |"},{"line_number":3,"context_line":"    It is now possible to count quota usage for cores and ram from the"},{"line_number":4,"context_line":"    placement service and instances from instance mappings in the API database"}],"source_content_type":"text/x-yaml","patch_set":28,"id":"3fce034c_d8d13025","line":1,"in_reply_to":"3fce034c_a3a43aa7","updated":"2019-04-18 20:02:10.000000000","message":"Done: https://review.openstack.org/653845","commit_id":"178fad2395374a463cd43133003520fbda581cbc"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4a36daee1b7e8b39f4dc23b4c9f8463e223403ba","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    * Behavior will be different for unscheduled servers in ``ERROR`` state."},{"line_number":40,"context_line":"      A server in ``ERROR`` state that has never been scheduled to a compute"},{"line_number":41,"context_line":"      host will not have placement allocations, so it will not consume quota"},{"line_number":42,"context_line":"      usage for cores and ram."}],"source_content_type":"text/x-yaml","patch_set":29,"id":"dfbec78f_2f876dda","line":42,"updated":"2019-05-02 17:22:43.000000000","message":"TODO: Need to also add that behavior will be different for SHELVED_OFFLOADED servers. Cores and ram will not be consumed for servers in this state.","commit_id":"e34a1ee797ba3b3016bf27664c3a0fe985dcf247"},{"author":{"_account_id":12408,"name":"Maxim Nestratov","email":"mnestratov@virtuozzo.com","username":"mnestratov"},"change_message_id":"db743c3b1d872c93c36a4480ee9d7f8f25f3d078","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    * Behavior will be different for unscheduled servers in ``ERROR`` state."},{"line_number":40,"context_line":"      A server in ``ERROR`` state that has never been scheduled to a compute"},{"line_number":41,"context_line":"      host will not have placement allocations, so it will not consume quota"},{"line_number":42,"context_line":"      usage for cores and ram."}],"source_content_type":"text/x-yaml","patch_set":29,"id":"dfbec78f_9d3493e0","line":42,"in_reply_to":"dfbec78f_2f876dda","updated":"2019-05-02 18:14:41.000000000","message":"This is not only affect SHELVED_OFFLOADED quota usage but also unshelve operation as it could fail now due to insufficient quota.","commit_id":"e34a1ee797ba3b3016bf27664c3a0fe985dcf247"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f5e4e36bc388046389087034a0e24cb4838d79ac","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    * Behavior will be different for unscheduled servers in ``ERROR`` state."},{"line_number":40,"context_line":"      A server in ``ERROR`` state that has never been scheduled to a compute"},{"line_number":41,"context_line":"      host will not have placement allocations, so it will not consume quota"},{"line_number":42,"context_line":"      usage for cores and ram."}],"source_content_type":"text/x-yaml","patch_set":29,"id":"dfbec78f_3da9e7de","line":42,"in_reply_to":"dfbec78f_9d3493e0","updated":"2019-05-02 18:18:56.000000000","message":"Yes, good point. I will be sure to mention that too in the bullet point too.","commit_id":"e34a1ee797ba3b3016bf27664c3a0fe985dcf247"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ac659187e75fba00d427bdd0a17a19c2995fe704","unresolved":false,"context_lines":[{"line_number":22,"context_line":"      allocations are held on both the source and destination (even on the same"},{"line_number":23,"context_line":"      host, see https://bugs.launchpad.net/nova/+bug/1790204) until the resize"},{"line_number":24,"context_line":"      is confirmed or reverted. Quota usage will be inflated for servers in"},{"line_number":25,"context_line":"      this state and operators should weigh the advantages and disadvantages"},{"line_number":26,"context_line":"      before enabling ``[quota]count_usage_from_placement``."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    * The ``populate_queued_for_delete`` and ``populate_user_id`` online data"}],"source_content_type":"text/x-yaml","patch_set":30,"id":"dfbec78f_3eaf9d95","line":25,"range":{"start_line":25,"start_character":6,"end_line":25,"end_character":10},"updated":"2019-05-02 21:47:47.000000000","message":"the ``VERIFY_RESIZE``","commit_id":"8351edec9615278e56979362c9f2b20dc8ed9721"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dab18a9ad12536b50272cfe90092f9e7ab0e80f1","unresolved":false,"context_lines":[{"line_number":25,"context_line":"      the ``VERIFY_RESIZE`` state and operators should weigh the advantages and"},{"line_number":26,"context_line":"      disadvantages before enabling ``[quota]count_usage_from_placement``."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    * The ``populate_queued_for_delete`` and ``populate_user_id`` online data"},{"line_number":29,"context_line":"      migrations must be completed before usage can be counted from placement."},{"line_number":30,"context_line":"      Until the data migration is complete, the system will fall back to legacy"},{"line_number":31,"context_line":"      quota usage counting from cell databases depending on the result of an"}],"source_content_type":"text/x-yaml","patch_set":31,"id":"bfb3d3c7_cc4db43e","line":28,"updated":"2019-05-29 21:15:15.000000000","message":"Given my comment about the warning log level when we switch back to legacy for an unmigrated project, this might be good to have in the config option help as well because the release notes are per-release and someone might miss this if they turn the option on in U or V (that\u0027s assuming we don\u0027t add a blocker migration and remove the data migration and compat code by then, and we tend to be pretty slow about removing that data migration stuff).","commit_id":"8354f42e20da952a6ab5612363afb77386886df0"}]}