)]}'
{"nova/objects/resource_provider.py":[{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"0e2594eef27f543f7ba16c960947288d9962a3dd","unresolved":false,"context_lines":[{"line_number":1116,"context_line":"    # is what we want."},{"line_number":1117,"context_line":"    #"},{"line_number":1118,"context_line":"    # ref: https://bugs.launchpad.net/nova/+bug/1705231"},{"line_number":1119,"context_line":"    for rc_id, sps in sorted(sharing_providers.items(), key\u003dlambda x: x[1]):"},{"line_number":1120,"context_line":"        it \u003d inv_tables[rc_id]"},{"line_number":1121,"context_line":"        ut \u003d usage_tables[rc_id]"},{"line_number":1122,"context_line":"        amount \u003d resources[rc_id]"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_9ba4800a","side":"PARENT","line":1119,"range":{"start_line":1119,"start_character":22,"end_line":1119,"end_character":75},"updated":"2017-11-21 11:16:51.000000000","message":"I tested locally and seems like that some bugs come from here, that is, this sorting is reverse and we need reverse\u003dTrue. I\u0027d like to fix this at first.","commit_id":"bdc5cf0c48e7b317ebc12be744e91eb895784a25"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"da3a4e3208ae50609e8d60cb921d9bfde60223ac","unresolved":false,"context_lines":[{"line_number":1116,"context_line":"    # is what we want."},{"line_number":1117,"context_line":"    #"},{"line_number":1118,"context_line":"    # ref: https://bugs.launchpad.net/nova/+bug/1705231"},{"line_number":1119,"context_line":"    for rc_id, sps in sorted(sharing_providers.items(), key\u003dlambda x: x[1]):"},{"line_number":1120,"context_line":"        it \u003d inv_tables[rc_id]"},{"line_number":1121,"context_line":"        ut \u003d usage_tables[rc_id]"},{"line_number":1122,"context_line":"        amount \u003d resources[rc_id]"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_9bb1e0e4","side":"PARENT","line":1119,"range":{"start_line":1119,"start_character":22,"end_line":1119,"end_character":75},"in_reply_to":"ff82abbf_9ba4800a","updated":"2017-11-21 11:35:05.000000000","message":"s/seems like/am becoming to think/\n\nI am not sure. sorry.","commit_id":"bdc5cf0c48e7b317ebc12be744e91eb895784a25"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3efdad2d5dd4d207bff57c0c675a9ebb937551be","unresolved":false,"context_lines":[{"line_number":1116,"context_line":"    # is what we want."},{"line_number":1117,"context_line":"    #"},{"line_number":1118,"context_line":"    # ref: https://bugs.launchpad.net/nova/+bug/1705231"},{"line_number":1119,"context_line":"    for rc_id, sps in sorted(sharing_providers.items(), key\u003dlambda x: x[1]):"},{"line_number":1120,"context_line":"        it \u003d inv_tables[rc_id]"},{"line_number":1121,"context_line":"        ut \u003d usage_tables[rc_id]"},{"line_number":1122,"context_line":"        amount \u003d resources[rc_id]"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_596a8080","side":"PARENT","line":1119,"range":{"start_line":1119,"start_character":22,"end_line":1119,"end_character":75},"in_reply_to":"ff82abbf_9bb1e0e4","updated":"2017-11-21 12:42:18.000000000","message":"Sorry, no need for reverse, maybe the key should be not x[1] but len(x[1])?? still not sure, so I will keep looking into this.... Please never mind. I am really sorry.","commit_id":"bdc5cf0c48e7b317ebc12be744e91eb895784a25"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"235a724e8703819bd56e45dc7d0d6a83a6264850","unresolved":false,"context_lines":[{"line_number":1136,"context_line":"        inv_join \u003d joiner("},{"line_number":1137,"context_line":"            rp_link, it,"},{"line_number":1138,"context_line":"            sa.and_("},{"line_number":1139,"context_line":"                jc,"},{"line_number":1140,"context_line":"                # Add a join condition winnowing this copy of inventories table"},{"line_number":1141,"context_line":"                # to only the resource class being analyzed in this loop..."},{"line_number":1142,"context_line":"                it.c.resource_class_id \u003d\u003d rc_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_358d1f7b","side":"PARENT","line":1139,"range":{"start_line":1139,"start_character":16,"end_line":1139,"end_character":18},"updated":"2017-11-22 10:53:23.000000000","message":"jc should be always \u0027rpt.c.id \u003d\u003d it.c.resource_provider_id\u0027\nbecause we are not sure we have that resource_provider_id for that resource class in the inventory table, and if not the row disappears despite that rp potentially meets the requirement with shared resources, which results in bug#1731072(2).","commit_id":"bdc5cf0c48e7b317ebc12be744e91eb895784a25"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"235a724e8703819bd56e45dc7d0d6a83a6264850","unresolved":false,"context_lines":[{"line_number":1145,"context_line":"        lastij \u003d it"},{"line_number":1146,"context_line":"        usage_join \u003d sa.outerjoin("},{"line_number":1147,"context_line":"            inv_join, ut,"},{"line_number":1148,"context_line":"            it.c.resource_provider_id \u003d\u003d ut.c.resource_provider_id,"},{"line_number":1149,"context_line":"        )"},{"line_number":1150,"context_line":"        join_chain \u003d usage_join"},{"line_number":1151,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_b5cfefae","side":"PARENT","line":1148,"range":{"start_line":1148,"start_character":12,"end_line":1148,"end_character":66},"updated":"2017-11-22 10:53:23.000000000","message":"should be \u0027rpt.c.id \u003d\u003d ut.c.resource_provider_id\u0027 for the same reason above.","commit_id":"bdc5cf0c48e7b317ebc12be744e91eb895784a25"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"08ed7a51977b8f94de431958c30d97ab4adf857b","unresolved":false,"context_lines":[{"line_number":910,"context_line":"    rpt \u003d sa.alias(_RP_TBL, name\u003d\"rp\")"},{"line_number":911,"context_line":"    inv \u003d sa.alias(_INV_TBL, name\u003d\"inv\")"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"    # Contains a set of resource provider IDs for each resource class requested"},{"line_number":914,"context_line":"    sharing_providers \u003d {"},{"line_number":915,"context_line":"        rc_id: _get_providers_with_shared_capacity(ctx, rc_id, amount)"},{"line_number":916,"context_line":"        for rc_id, amount in resources.items()"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_39f3fca0","line":913,"range":{"start_line":913,"start_character":17,"end_line":913,"end_character":20},"updated":"2017-11-21 13:16:59.000000000","message":"list","commit_id":"be637e723549e230867d822ecdeccfcb4ba44b30"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"08ed7a51977b8f94de431958c30d97ab4adf857b","unresolved":false,"context_lines":[{"line_number":1002,"context_line":"    sel \u003d sa.select([sub_query.c.id])"},{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"    join_chain \u003d None"},{"line_number":1005,"context_line":"    for rc_id, sps in sorted(sharing_providers.items(), key\u003dlambda x: x[1]):"},{"line_number":1006,"context_line":"        if sps:"},{"line_number":1007,"context_line":"            sharing \u003d sharing_tables[rc_id]"},{"line_number":1008,"context_line":"            shared \u003d shared_tables[rc_id]"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_998568b0","line":1005,"range":{"start_line":1005,"start_character":22,"end_line":1005,"end_character":76},"updated":"2017-11-21 13:16:59.000000000","message":"Following on Tetsuro\u0027s comment...\n\nYeah, this sort got ported over from the base, but lost the long explanatory comment...\n\n\n...which doesn\u0027t make any sense to me.  IIUC, we\u0027re sorting the list of sharing_providers items on the dict value, which is a list of RP IDs.  _get_providers_with_shared_capacity doesn\u0027t ORDER BY anything, so the RP ID list we\u0027re sorting on could be in any order.  But even if it weren\u0027t, how could this sort possibly be meaningful?\n\nI\u0027m not saying there\u0027s a bug here.  But I\u0027m pretty sure this sort isn\u0027t doing anything, and ought to be removed.\n\n[Later] Well, it\u0027s clearly doing *something*, because when I remove it, the test results change.  I didn\u0027t fully analyze the results, but removing the sort appears to fix bug #1731072 bullet (2).\n\nIt would help if I understood what the actual intent was.  Jay?","commit_id":"be637e723549e230867d822ecdeccfcb4ba44b30"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"7c9c11a9585c056e344ac261d3654ad97d170831","unresolved":false,"context_lines":[{"line_number":1002,"context_line":"    sel \u003d sa.select([sub_query.c.id])"},{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"    join_chain \u003d None"},{"line_number":1005,"context_line":"    for rc_id, sps in sorted(sharing_providers.items(), key\u003dlambda x: x[1]):"},{"line_number":1006,"context_line":"        if sps:"},{"line_number":1007,"context_line":"            sharing \u003d sharing_tables[rc_id]"},{"line_number":1008,"context_line":"            shared \u003d shared_tables[rc_id]"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_52fbe477","line":1005,"range":{"start_line":1005,"start_character":22,"end_line":1005,"end_character":76},"in_reply_to":"ff82abbf_5c931950","updated":"2017-11-24 05:06:21.000000000","message":"There *is* inner join. Whether there are sharing resources or not depends on the resource class we are considering.\n\nAnyway, my recommendation is to debug bug:1732707(cross aggs bug) and bug:1731072(missing rp bug) before performance tuning.\nI mean, why don\u0027t we clean up the *test code*, first?","commit_id":"be637e723549e230867d822ecdeccfcb4ba44b30"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"1dba6bb02115b074804992bdfcd06b5ed2387ca3","unresolved":false,"context_lines":[{"line_number":1002,"context_line":"    sel \u003d sa.select([sub_query.c.id])"},{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"    join_chain \u003d None"},{"line_number":1005,"context_line":"    for rc_id, sps in sorted(sharing_providers.items(), key\u003dlambda x: x[1]):"},{"line_number":1006,"context_line":"        if sps:"},{"line_number":1007,"context_line":"            sharing \u003d sharing_tables[rc_id]"},{"line_number":1008,"context_line":"            shared \u003d shared_tables[rc_id]"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_5c931950","line":1005,"range":{"start_line":1005,"start_character":22,"end_line":1005,"end_character":76},"in_reply_to":"ff82abbf_6c653db5","updated":"2017-11-23 02:06:30.000000000","message":"@Eric, @Tetsuro, the Eric is right, the sort is useless, since all the things in this loop is about sharing resource, and all the join for those sharing resource is \u0027left join\u0027 https://review.openstack.org/#/c/519081/4/nova/objects/resource_provider.py@984,\n\nsorry, I just copy this from the base. I still left few testcases need to debug, and then I will cleanup the code.","commit_id":"be637e723549e230867d822ecdeccfcb4ba44b30"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"f2e0c61bc5202b672df404f60b3d6ba44f99cc7f","unresolved":false,"context_lines":[{"line_number":1002,"context_line":"    sel \u003d sa.select([sub_query.c.id])"},{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"    join_chain \u003d None"},{"line_number":1005,"context_line":"    for rc_id, sps in sorted(sharing_providers.items(), key\u003dlambda x: x[1]):"},{"line_number":1006,"context_line":"        if sps:"},{"line_number":1007,"context_line":"            sharing \u003d sharing_tables[rc_id]"},{"line_number":1008,"context_line":"            shared \u003d shared_tables[rc_id]"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_6c653db5","line":1005,"range":{"start_line":1005,"start_character":22,"end_line":1005,"end_character":76},"in_reply_to":"ff82abbf_998568b0","updated":"2017-11-21 15:20:43.000000000","message":"It makes sense to sort in order to make inner join come first. But what doesn\u0027t make sense to me so far is that the test result seems to depend on the order of outer join, too. This may be the problem we are seeking out? I\u0027m not sure. I will check out Alex\u0027s patch if the behavior is the same, and try to make out what is happening (with my little sql knowlege.)","commit_id":"be637e723549e230867d822ecdeccfcb4ba44b30"}]}