)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"600666dd93317427b3b6249c987fcb50ba9c10d9","unresolved":false,"context_lines":[{"line_number":14,"context_line":"Also that our current check is currently broken since it does not do"},{"line_number":15,"context_line":"the check based on pages. The total of physical memory can be enough,"},{"line_number":16,"context_line":"but for the requested page size by instance that coud not be the"},{"line_number":17,"context_line":"case. The next page will fix this issue."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I5909c9ceebabedb9dee3b859caa1a514977bd128"},{"line_number":20,"context_line":"Signed-off-by: Sahid Orentino Ferdjaoui \u003csahid.ferdjaoui@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f7c97a3_f9affd27","line":17,"range":{"start_line":17,"start_character":15,"end_line":17,"end_character":19},"updated":"2018-07-10 13:58:27.000000000","message":"s/patch/page/. Also, you should probably say \"a subsequent patch\" in case they don\u0027t land in the same order","commit_id":"495077d3ca1ebcd7f083eda6a3982e64f8b1d5d5"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"98e4210efb51768dd54fc5bf4d4b83e56d95ad5e","unresolved":false,"context_lines":[{"line_number":14,"context_line":"Also that our current check is currently broken since it does not do"},{"line_number":15,"context_line":"the check based on pages. The total of physical memory can be enough,"},{"line_number":16,"context_line":"but for the requested page size by instance that coud not be the"},{"line_number":17,"context_line":"case. The next page will fix this issue."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I5909c9ceebabedb9dee3b859caa1a514977bd128"},{"line_number":20,"context_line":"Signed-off-by: Sahid Orentino Ferdjaoui \u003csahid.ferdjaoui@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f7c97a3_3451fc0d","line":17,"range":{"start_line":17,"start_character":15,"end_line":17,"end_character":19},"in_reply_to":"5f7c97a3_f9affd27","updated":"2018-07-10 14:03:31.000000000","message":"Done","commit_id":"495077d3ca1ebcd7f083eda6a3982e64f8b1d5d5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"600666dd93317427b3b6249c987fcb50ba9c10d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"5f7c97a3_9964c922","line":21,"updated":"2018-07-10 13:58:27.000000000","message":"Related-bug?","commit_id":"495077d3ca1ebcd7f083eda6a3982e64f8b1d5d5"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"98e4210efb51768dd54fc5bf4d4b83e56d95ad5e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"5f7c97a3_5454b8fc","line":21,"in_reply_to":"5f7c97a3_9964c922","updated":"2018-07-10 14:03:31.000000000","message":"Done","commit_id":"495077d3ca1ebcd7f083eda6a3982e64f8b1d5d5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7272cf7b7e482514774d7bf28150209c02998dc5","unresolved":false,"context_lines":[{"line_number":9,"context_line":"The method \u0027_numa_fit_instance_cell_with_pinning\u0027 is checking total of"},{"line_number":10,"context_line":"physical memory available on the NUMA host against the total"},{"line_number":11,"context_line":"requested. But that check is already done by"},{"line_number":12,"context_line":"\u0027_numa_fit_instance_cell\u0027."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Also that our current check is currently broken since it does not do"},{"line_number":15,"context_line":"the check based on pages. The total of physical memory can be enough,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f7c97a3_88647022","line":12,"range":{"start_line":12,"start_character":24,"end_line":12,"end_character":26},"updated":"2018-07-10 15:28:49.000000000","message":"\u0027, which is a caller of this function.","commit_id":"889111f2c90229d39692de1eb57be0881d13e084"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7272cf7b7e482514774d7bf28150209c02998dc5","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Also that our current check is currently broken since it does not do"},{"line_number":15,"context_line":"the check based on pages. The total of physical memory can be enough,"},{"line_number":16,"context_line":"but for the requested page size by instance that coud not be the"},{"line_number":17,"context_line":"case. A subsequent patch patch will fix this issue."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Partial-Bug: #1594529"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f7c97a3_c82348ba","line":16,"range":{"start_line":16,"start_character":49,"end_line":16,"end_character":53},"updated":"2018-07-10 15:28:49.000000000","message":"could","commit_id":"889111f2c90229d39692de1eb57be0881d13e084"}],"nova/virt/hardware.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7272cf7b7e482514774d7bf28150209c02998dc5","unresolved":false,"context_lines":[{"line_number":1009,"context_line":"              \u0027%(host_cell)s\u0027, {\u0027cell\u0027: instance_cell, \u0027host_cell\u0027: host_cell})"},{"line_number":1010,"context_line":"    # NOTE (ndipanov): do not allow an instance to overcommit against"},{"line_number":1011,"context_line":"    # itself on any NUMA cell"},{"line_number":1012,"context_line":"    if instance_cell.memory \u003e host_cell.memory:"},{"line_number":1013,"context_line":"        LOG.debug(\u0027Not enough host cell memory to fit instance cell. \u0027"},{"line_number":1014,"context_line":"                  \u0027Required: %(required)d, actual: %(actual)d\u0027,"},{"line_number":1015,"context_line":"                  {\u0027required\u0027: instance_cell.memory,"},{"line_number":1016,"context_line":"                   \u0027actual\u0027: host_cell.memory})"},{"line_number":1017,"context_line":"        return"},{"line_number":1018,"context_line":""},{"line_number":1019,"context_line":"    if len(instance_cell.cpuset) + cpuset_reserved \u003e len(host_cell.cpuset):"},{"line_number":1020,"context_line":"        LOG.debug(\u0027Not enough host cell CPUs to fit instance cell. Required: \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_8b9c6abf","line":1017,"range":{"start_line":1012,"start_character":0,"end_line":1017,"end_character":14},"updated":"2018-07-10 15:28:49.000000000","message":"This measures against total memory and not available memory. As the comments notes, this is to avoid an individual cell overcommiting against itself...","commit_id":"889111f2c90229d39692de1eb57be0881d13e084"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7272cf7b7e482514774d7bf28150209c02998dc5","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"        memory_usage \u003d host_cell.memory_usage + instance_cell.memory"},{"line_number":1041,"context_line":"        cpu_usage \u003d host_cell.cpu_usage + len(instance_cell.cpuset)"},{"line_number":1042,"context_line":"        cpu_limit \u003d len(host_cell.cpuset) * limit_cell.cpu_allocation_ratio"},{"line_number":1043,"context_line":"        ram_limit \u003d host_cell.memory * limit_cell.ram_allocation_ratio"},{"line_number":1044,"context_line":"        if memory_usage \u003e ram_limit:"},{"line_number":1045,"context_line":"            LOG.debug(\u0027Host cell has limitations on usable memory. There is \u0027"},{"line_number":1046,"context_line":"                      \u0027not enough free memory to schedule this instance. \u0027"},{"line_number":1047,"context_line":"                      \u0027Usage: %(usage)d, limit: %(limit)d\u0027,"},{"line_number":1048,"context_line":"                      {\u0027usage\u0027: memory_usage, \u0027limit\u0027: ram_limit})"},{"line_number":1049,"context_line":"            return"},{"line_number":1050,"context_line":"        if cpu_usage \u003e cpu_limit:"},{"line_number":1051,"context_line":"            LOG.debug(\u0027Host cell has limitations on usable CPUs. There are \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_abe04628","line":1048,"range":{"start_line":1043,"start_character":0,"end_line":1048,"end_character":66},"updated":"2018-07-10 15:28:49.000000000","message":"...however, there\u0027s no reason we can\u0027t overcommit against other cells. That\u0027s what we\u0027re checking here, e.g. the below would be valid.\n\n  host.cell[0].memory -\u003e 1GB\n  instance1.cell[0].memory -\u003e 1GB\n  instance2.cell[0].memory -\u003e 1GB\n  limit_cell.ram_allocation_ratio -\u003e 2\n\nHowever, overcommit is not valid for pinned instances, hence the other check which we\u0027re (wrongfully, it would seem) removing in the patch.","commit_id":"889111f2c90229d39692de1eb57be0881d13e084"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"59b9c153d2c35bb7fdda576773d9e32e1cd847fb","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"        memory_usage \u003d host_cell.memory_usage + instance_cell.memory"},{"line_number":1041,"context_line":"        cpu_usage \u003d host_cell.cpu_usage + len(instance_cell.cpuset)"},{"line_number":1042,"context_line":"        cpu_limit \u003d len(host_cell.cpuset) * limit_cell.cpu_allocation_ratio"},{"line_number":1043,"context_line":"        ram_limit \u003d host_cell.memory * limit_cell.ram_allocation_ratio"},{"line_number":1044,"context_line":"        if memory_usage \u003e ram_limit:"},{"line_number":1045,"context_line":"            LOG.debug(\u0027Host cell has limitations on usable memory. There is \u0027"},{"line_number":1046,"context_line":"                      \u0027not enough free memory to schedule this instance. \u0027"},{"line_number":1047,"context_line":"                      \u0027Usage: %(usage)d, limit: %(limit)d\u0027,"},{"line_number":1048,"context_line":"                      {\u0027usage\u0027: memory_usage, \u0027limit\u0027: ram_limit})"},{"line_number":1049,"context_line":"            return"},{"line_number":1050,"context_line":"        if cpu_usage \u003e cpu_limit:"},{"line_number":1051,"context_line":"            LOG.debug(\u0027Host cell has limitations on usable CPUs. There are \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_de25420b","line":1048,"range":{"start_line":1043,"start_character":0,"end_line":1048,"end_character":66},"in_reply_to":"5f7c97a3_2b3516f6","updated":"2018-07-10 16:25:45.000000000","message":"\u003e Hum... that part looks to be broken for two reasons:\n \u003e \n \u003e 1/ we check memory requested by instance cell against host cell\n \u003e before to use the ram_allocation_ratio so the case bellow does not\n \u003e pass where that should work.\n \u003e \n \u003e instance1.cell[0].memory -\u003e 1,2GB\n \nYou\u0027re referring to the check on 1012, I assume? If so, note that this is checking for \u0027host_cell.memory\u0027 which is the total amount of memory on the node - not the amount used. This prevents the following case:\n\n  host.cell[0].memory -\u003e 1GB\n  instance1.cell[0].memory -\u003e 2GB\n\n \u003e 2/ the guest memory will be confined in the selected NUMA host\n \u003e meaning that, there is no swap we could be in a situation where the\n \u003e guest get killed if cell run out of memory.\n\nI\u0027m not sure about this. If we\u0027re using NUMA topologies, aren\u0027t we already confining guest memory to the selected host NUMA node?\n\n \u003e My thinking is we should remove that part, it\u0027s already broken\n \u003e anyway. What do you think?\n\nWhich part? The one we\u0027ve removed above or another part? If it\u0027s the one we\u0027re removing above, what replaces it? As noted, the check on line 1012 checks against total memory, not available memory, and the check on line 1044 doesn\u0027t apply for pinned instances","commit_id":"889111f2c90229d39692de1eb57be0881d13e084"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"abf7938aa76ff998bab0868c077938eb8ba1c350","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"        memory_usage \u003d host_cell.memory_usage + instance_cell.memory"},{"line_number":1041,"context_line":"        cpu_usage \u003d host_cell.cpu_usage + len(instance_cell.cpuset)"},{"line_number":1042,"context_line":"        cpu_limit \u003d len(host_cell.cpuset) * limit_cell.cpu_allocation_ratio"},{"line_number":1043,"context_line":"        ram_limit \u003d host_cell.memory * limit_cell.ram_allocation_ratio"},{"line_number":1044,"context_line":"        if memory_usage \u003e ram_limit:"},{"line_number":1045,"context_line":"            LOG.debug(\u0027Host cell has limitations on usable memory. There is \u0027"},{"line_number":1046,"context_line":"                      \u0027not enough free memory to schedule this instance. \u0027"},{"line_number":1047,"context_line":"                      \u0027Usage: %(usage)d, limit: %(limit)d\u0027,"},{"line_number":1048,"context_line":"                      {\u0027usage\u0027: memory_usage, \u0027limit\u0027: ram_limit})"},{"line_number":1049,"context_line":"            return"},{"line_number":1050,"context_line":"        if cpu_usage \u003e cpu_limit:"},{"line_number":1051,"context_line":"            LOG.debug(\u0027Host cell has limitations on usable CPUs. There are \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_2b3516f6","line":1048,"range":{"start_line":1043,"start_character":0,"end_line":1048,"end_character":66},"in_reply_to":"5f7c97a3_abe04628","updated":"2018-07-10 15:56:43.000000000","message":"Hum... that part looks to be broken for two reasons:\n\n 1/ we check memory requested by instance cell against host cell before to use the ram_allocation_ratio so the case bellow does not pass where that should work.\n\n  instance1.cell[0].memory -\u003e 1,2GB\n  \n 2/ the guest memory will be confined in the selected NUMA host meaning that, there is no swap we could be in a situation where the guest get killed if cell run out of memory.\n\nMy thinking is we should remove that part, it\u0027s already broken anyway. What do you think?","commit_id":"889111f2c90229d39692de1eb57be0881d13e084"}]}