)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"41c9722c91d90c3e9ec768de3cb3e6e1d44dfe3f","unresolved":false,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"When ResourceProviderList.get_all_by_filters() fails to find any resource"},{"line_number":13,"context_line":"providers using its normal \"find me all the resource providers that have"},{"line_number":14,"context_line":"capacioty for all of these resources\" query, we fall into a new"},{"line_number":15,"context_line":"_get_all_with_shared() method that constructs an advanced SQL expression that"},{"line_number":16,"context_line":"takes into account providers that share resource with other providers via an"},{"line_number":17,"context_line":"aggregate association."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5ff73747_8c5e8fe7","line":14,"range":{"start_line":14,"start_character":0,"end_line":14,"end_character":9},"updated":"2017-05-02 14:13:02.000000000","message":"creative spelling! :)","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd2049c11d3fffbfd924e0e849fa2a0f2710010e","unresolved":false,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"When ResourceProviderList.get_all_by_filters() fails to find any resource"},{"line_number":13,"context_line":"providers using its normal \"find me all the resource providers that have"},{"line_number":14,"context_line":"capacioty for all of these resources\" query, we fall into a new"},{"line_number":15,"context_line":"_get_all_with_shared() method that constructs an advanced SQL expression that"},{"line_number":16,"context_line":"takes into account providers that share resource with other providers via an"},{"line_number":17,"context_line":"aggregate association."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5ff73747_77c1d0e4","line":14,"range":{"start_line":14,"start_character":0,"end_line":14,"end_character":9},"in_reply_to":"5ff73747_8c5e8fe7","updated":"2017-05-02 14:39:45.000000000","message":":) fixed","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"fb973434079523f5b6066c48ccdadac11b7d05e5","unresolved":false,"context_lines":[{"line_number":100,"context_line":")"},{"line_number":101,"context_line":"GROUP BY rp.id"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"While the above may look hairy, the query plan produce is remarkably efficient."},{"line_number":104,"context_line":"Tested on a database populated with 10K compute nodes arranged in 100"},{"line_number":105,"context_line":"aggregates each with an associated storage pool, a MySQL server with minimal"},{"line_number":106,"context_line":"RAM (128MB InnoDB buffer pool size) was able to execute the above in an average"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5ff73747_838d422c","line":103,"updated":"2017-05-02 12:42:47.000000000","message":"nit: produced","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd2049c11d3fffbfd924e0e849fa2a0f2710010e","unresolved":false,"context_lines":[{"line_number":100,"context_line":")"},{"line_number":101,"context_line":"GROUP BY rp.id"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"While the above may look hairy, the query plan produce is remarkably efficient."},{"line_number":104,"context_line":"Tested on a database populated with 10K compute nodes arranged in 100"},{"line_number":105,"context_line":"aggregates each with an associated storage pool, a MySQL server with minimal"},{"line_number":106,"context_line":"RAM (128MB InnoDB buffer pool size) was able to execute the above in an average"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5ff73747_37bb4873","line":103,"in_reply_to":"5ff73747_838d422c","updated":"2017-05-02 14:39:45.000000000","message":"Done","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cfff5230617b437eb7e58b7b3d10dd48e256b2f7","unresolved":false,"context_lines":[{"line_number":11,"context_line":"function constructs an advanced SQL expression that takes into account"},{"line_number":12,"context_line":"providers that share resource with other providers via an aggregate"},{"line_number":13,"context_line":"association."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"The SQL generated here is pretty complex so I\u0027ve included ample code comments"},{"line_number":16,"context_line":"showing the logic of how the join expressions are built up, one on top of the"},{"line_number":17,"context_line":"other."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"1f013ff3_bc37fdb0","line":14,"updated":"2017-05-16 14:09:09.000000000","message":"All the paragraphs in this commit message are past 80 columns which makes it hard to review on the main page. Can you just wrap them please?","commit_id":"d001582a46a5cb58767f3187a6a2979b93b39322"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"da4715c63da834fa08908047b35bd6a5ca2e9c45","unresolved":false,"context_lines":[{"line_number":11,"context_line":"function constructs an advanced SQL expression that takes into account"},{"line_number":12,"context_line":"providers that share resource with other providers via an aggregate"},{"line_number":13,"context_line":"association."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"The SQL generated here is pretty complex so I\u0027ve included ample code comments"},{"line_number":16,"context_line":"showing the logic of how the join expressions are built up, one on top of the"},{"line_number":17,"context_line":"other."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"ff0f0b1f_5e8785b6","line":14,"in_reply_to":"1f013ff3_bc37fdb0","updated":"2017-05-22 13:23:37.000000000","message":"Done","commit_id":"d001582a46a5cb58767f3187a6a2979b93b39322"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cfff5230617b437eb7e58b7b3d10dd48e256b2f7","unresolved":false,"context_lines":[{"line_number":94,"context_line":"  ) OR"},{"line_number":95,"context_line":"    shared_disk_gb.aggregate_id IS NOT NULL"},{"line_number":96,"context_line":")"},{"line_number":97,"context_line":"GROUP BY rp.id"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"While the above may look hairy, the query plan produced is remarkably"},{"line_number":100,"context_line":"efficient.  Tested on a database populated with 10K compute nodes arranged in"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"1f013ff3_fcbfc5f8","line":97,"updated":"2017-05-16 14:09:09.000000000","message":"Reproducing the query in the commit message does nothing for me, so I think you should remove it.","commit_id":"d001582a46a5cb58767f3187a6a2979b93b39322"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"da4715c63da834fa08908047b35bd6a5ca2e9c45","unresolved":false,"context_lines":[{"line_number":94,"context_line":"  ) OR"},{"line_number":95,"context_line":"    shared_disk_gb.aggregate_id IS NOT NULL"},{"line_number":96,"context_line":")"},{"line_number":97,"context_line":"GROUP BY rp.id"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"While the above may look hairy, the query plan produced is remarkably"},{"line_number":100,"context_line":"efficient.  Tested on a database populated with 10K compute nodes arranged in"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"ff0f0b1f_3e9051ed","line":97,"in_reply_to":"1f013ff3_fcbfc5f8","updated":"2017-05-22 13:23:37.000000000","message":"Done","commit_id":"d001582a46a5cb58767f3187a6a2979b93b39322"}],"nova/objects/resource_provider.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8d42e0f7ca0bacf3377b5b2c61ce3f2df2a890e7","unresolved":false,"context_lines":[{"line_number":717,"context_line":""},{"line_number":718,"context_line":"    @staticmethod"},{"line_number":719,"context_line":"    @db_api.api_context_manager.reader"},{"line_number":720,"context_line":"    def _get_all_with_shared(ctx, resources):"},{"line_number":721,"context_line":"        \"\"\"Uses some more advanced SQL to find providers that either have the"},{"line_number":722,"context_line":"        requested resources \"locally\" or are associated with a provider that"},{"line_number":723,"context_line":"        shares those requested resources."}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_3fe0bafa","line":720,"updated":"2017-05-02 16:16:39.000000000","message":"I know you\u0027re mirroring the other methods here and in other objects, but because this is a billion lines long, I kinda feel like we should put this at module level instead of just blowing out the middle of this class. It makes it really hard to read the class at this point. I know there\u0027s another long one below, but I\u0027d have made the same argument about that one.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"fb973434079523f5b6066c48ccdadac11b7d05e5","unresolved":false,"context_lines":[{"line_number":720,"context_line":"    def _get_all_with_shared(ctx, resources):"},{"line_number":721,"context_line":"        \"\"\"Uses some more advanced SQL to find providers that either have the"},{"line_number":722,"context_line":"        requested resources \"locally\" or are associated with a provider that"},{"line_number":723,"context_line":"        shares those requested resources."},{"line_number":724,"context_line":""},{"line_number":725,"context_line":"        :param resources: Dict keyed by resource class integer ID of requested"},{"line_number":726,"context_line":"                          amounts of that resource"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_23866edd","line":723,"updated":"2017-05-02 12:42:47.000000000","message":"This description doesn\u0027t seem quite right to me. Below, we only choose to call this method if a search for rps that can solve for local-only has returned no results. If there are no results we call this to get those rps which can satisfy some-or-none of the resource request locally _and_ (not \"or\") one-or-more of the resource using an rp associated by aggregate.\n\nIt may be that you\u0027re using the term \"locally\" different than I am, in which case, can you expand a bit?\n\nWhatever the case may be, I think because this stuff is bound to cause confusion in the future we need to make sure that the description of the method is as crystal clear as we can possibly make it.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd2049c11d3fffbfd924e0e849fa2a0f2710010e","unresolved":false,"context_lines":[{"line_number":720,"context_line":"    def _get_all_with_shared(ctx, resources):"},{"line_number":721,"context_line":"        \"\"\"Uses some more advanced SQL to find providers that either have the"},{"line_number":722,"context_line":"        requested resources \"locally\" or are associated with a provider that"},{"line_number":723,"context_line":"        shares those requested resources."},{"line_number":724,"context_line":""},{"line_number":725,"context_line":"        :param resources: Dict keyed by resource class integer ID of requested"},{"line_number":726,"context_line":"                          amounts of that resource"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_b2fd26b3","line":723,"in_reply_to":"5ff73747_23866edd","updated":"2017-05-02 14:39:45.000000000","message":"No, I definitely meant \"or\", though perhaps I can define \"locally\" better, yes.\n\nThe OR is evident below on line 1025.\n\nWe need to check for resource providers that *either* have the resource locally *or* are associated by aggregate to a provider that shares those resources.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"3aa2928bda49f9497a1e81ad73d64abfb9cd8466","unresolved":false,"context_lines":[{"line_number":720,"context_line":"    def _get_all_with_shared(ctx, resources):"},{"line_number":721,"context_line":"        \"\"\"Uses some more advanced SQL to find providers that either have the"},{"line_number":722,"context_line":"        requested resources \"locally\" or are associated with a provider that"},{"line_number":723,"context_line":"        shares those requested resources."},{"line_number":724,"context_line":""},{"line_number":725,"context_line":"        :param resources: Dict keyed by resource class integer ID of requested"},{"line_number":726,"context_line":"                          amounts of that resource"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_a62607ba","line":723,"in_reply_to":"5ff73747_b2fd26b3","updated":"2017-05-02 14:57:48.000000000","message":"Yeah, I figured out that OR on the re-read mentioned in irc. It was unexpected, because of the duplicated effort that Ed\u0027s asked about.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"fb973434079523f5b6066c48ccdadac11b7d05e5","unresolved":false,"context_lines":[{"line_number":769,"context_line":"        #"},{"line_number":770,"context_line":"        # For resource classes that DO have shared providers, we also perform"},{"line_number":771,"context_line":"        # a \"butterfly join\" against two copies of the"},{"line_number":772,"context_line":"        # resource_provider_aggregates table."},{"line_number":773,"context_line":"        #"},{"line_number":774,"context_line":"        # The first part of the butterfly join is an outer join against a copy"},{"line_number":775,"context_line":"        # of the resource_provider_aggregates table in order to winnow results"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_c34d8ac5","line":772,"updated":"2017-05-02 12:42:47.000000000","message":"the famed butterfly join arrives after months of anticipation","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd2049c11d3fffbfd924e0e849fa2a0f2710010e","unresolved":false,"context_lines":[{"line_number":769,"context_line":"        #"},{"line_number":770,"context_line":"        # For resource classes that DO have shared providers, we also perform"},{"line_number":771,"context_line":"        # a \"butterfly join\" against two copies of the"},{"line_number":772,"context_line":"        # resource_provider_aggregates table."},{"line_number":773,"context_line":"        #"},{"line_number":774,"context_line":"        # The first part of the butterfly join is an outer join against a copy"},{"line_number":775,"context_line":"        # of the resource_provider_aggregates table in order to winnow results"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_b21646f1","line":772,"in_reply_to":"5ff73747_4cb407f3","updated":"2017-05-02 14:39:45.000000000","message":"Heh, it\u0027s a term I made up to describe what\u0027s going on here. It\u0027s easier to picture it on a whiteboard :)","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"41c9722c91d90c3e9ec768de3cb3e6e1d44dfe3f","unresolved":false,"context_lines":[{"line_number":769,"context_line":"        #"},{"line_number":770,"context_line":"        # For resource classes that DO have shared providers, we also perform"},{"line_number":771,"context_line":"        # a \"butterfly join\" against two copies of the"},{"line_number":772,"context_line":"        # resource_provider_aggregates table."},{"line_number":773,"context_line":"        #"},{"line_number":774,"context_line":"        # The first part of the butterfly join is an outer join against a copy"},{"line_number":775,"context_line":"        # of the resource_provider_aggregates table in order to winnow results"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_4cb407f3","line":772,"in_reply_to":"5ff73747_c34d8ac5","updated":"2017-05-02 14:13:02.000000000","message":"This is a new term to me.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"41c9722c91d90c3e9ec768de3cb3e6e1d44dfe3f","unresolved":false,"context_lines":[{"line_number":789,"context_line":"        # resource class has any shared providers."},{"line_number":790,"context_line":"        #"},{"line_number":791,"context_line":"        # For resource classes that DO NOT have any shared resource providers,"},{"line_number":792,"context_line":"        # the WHERE clause constructed filters out resource providers that have"},{"line_number":793,"context_line":"        # inventory for \"local\" resource providers:"},{"line_number":794,"context_line":"        #"},{"line_number":795,"context_line":"        # WHERE (COALESCE(usage_vcpu.used, 0) + $AMOUNT \u003c\u003d"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_2cae5b98","line":792,"range":{"start_line":792,"start_character":39,"end_line":792,"end_character":50},"updated":"2017-05-02 14:13:02.000000000","message":"\"filters for\"? \"filters out\" sounds like they are being removed from the result set.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd2049c11d3fffbfd924e0e849fa2a0f2710010e","unresolved":false,"context_lines":[{"line_number":789,"context_line":"        # resource class has any shared providers."},{"line_number":790,"context_line":"        #"},{"line_number":791,"context_line":"        # For resource classes that DO NOT have any shared resource providers,"},{"line_number":792,"context_line":"        # the WHERE clause constructed filters out resource providers that have"},{"line_number":793,"context_line":"        # inventory for \"local\" resource providers:"},{"line_number":794,"context_line":"        #"},{"line_number":795,"context_line":"        # WHERE (COALESCE(usage_vcpu.used, 0) + $AMOUNT \u003c\u003d"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_32e0f6fa","line":792,"range":{"start_line":792,"start_character":39,"end_line":792,"end_character":50},"in_reply_to":"5ff73747_2cae5b98","updated":"2017-05-02 14:39:45.000000000","message":"Yes, good catch. Fixed.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"41c9722c91d90c3e9ec768de3cb3e6e1d44dfe3f","unresolved":false,"context_lines":[{"line_number":986,"context_line":"            amount \u003d resources[rc_id]"},{"line_number":987,"context_line":""},{"line_number":988,"context_line":"            if join_chain is None:"},{"line_number":989,"context_line":"                rp_link \u003d rpt"},{"line_number":990,"context_line":"                jc \u003d rpt.c.id \u003d\u003d it.c.resource_provider_id"},{"line_number":991,"context_line":"            else:"},{"line_number":992,"context_line":"                rp_link \u003d join_chain"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_d71ae46d","line":989,"range":{"start_line":989,"start_character":26,"end_line":989,"end_character":29},"updated":"2017-05-02 14:13:02.000000000","message":"suggestion: you might want to make these names a little more descriptive.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"41c9722c91d90c3e9ec768de3cb3e6e1d44dfe3f","unresolved":false,"context_lines":[{"line_number":994,"context_line":""},{"line_number":995,"context_line":"            # We can do a more efficient INNER JOIN when we don\u0027t have shared"},{"line_number":996,"context_line":"            # resource providers for this resource class"},{"line_number":997,"context_line":"            joiner \u003d sa.join"},{"line_number":998,"context_line":"            if sps:"},{"line_number":999,"context_line":"                joiner \u003d sa.outerjoin"},{"line_number":1000,"context_line":"            inv_join \u003d joiner("}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_576ed4c9","line":997,"range":{"start_line":997,"start_character":12,"end_line":997,"end_character":28},"updated":"2017-05-02 14:13:02.000000000","message":"this doesn\u0027t seem to either save characters nor provide clarity.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"a869a1aabb27248340e71c9c53ebdcff953c9efa","unresolved":false,"context_lines":[{"line_number":994,"context_line":""},{"line_number":995,"context_line":"            # We can do a more efficient INNER JOIN when we don\u0027t have shared"},{"line_number":996,"context_line":"            # resource providers for this resource class"},{"line_number":997,"context_line":"            joiner \u003d sa.join"},{"line_number":998,"context_line":"            if sps:"},{"line_number":999,"context_line":"                joiner \u003d sa.outerjoin"},{"line_number":1000,"context_line":"            inv_join \u003d joiner("}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_66559ffa","line":997,"range":{"start_line":997,"start_character":12,"end_line":997,"end_character":28},"in_reply_to":"5ff73747_320c763a","updated":"2017-05-02 14:44:02.000000000","message":"Ah, ok, I see it now.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd2049c11d3fffbfd924e0e849fa2a0f2710010e","unresolved":false,"context_lines":[{"line_number":994,"context_line":""},{"line_number":995,"context_line":"            # We can do a more efficient INNER JOIN when we don\u0027t have shared"},{"line_number":996,"context_line":"            # resource providers for this resource class"},{"line_number":997,"context_line":"            joiner \u003d sa.join"},{"line_number":998,"context_line":"            if sps:"},{"line_number":999,"context_line":"                joiner \u003d sa.outerjoin"},{"line_number":1000,"context_line":"            inv_join \u003d joiner("}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_320c763a","line":997,"range":{"start_line":997,"start_character":12,"end_line":997,"end_character":28},"in_reply_to":"5ff73747_576ed4c9","updated":"2017-05-02 14:39:45.000000000","message":"It\u0027s not about saving characters.\n\nsee line 1000. we\u0027re either doing an INNER JOIN (sa.join) or a LEFT OUTER JOIN (sa.outerjoin) depending on whether there are shared resource providers for this resource or not.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"41c9722c91d90c3e9ec768de3cb3e6e1d44dfe3f","unresolved":false,"context_lines":[{"line_number":1010,"context_line":""},{"line_number":1011,"context_line":"            usage_cond \u003d sa.and_("},{"line_number":1012,"context_line":"                ("},{"line_number":1013,"context_line":"                (sql.func.coalesce(ut.c.used, 0) + amount) \u003c\u003d"},{"line_number":1014,"context_line":"                (it.c.total - it.c.reserved) * it.c.allocation_ratio"},{"line_number":1015,"context_line":"                ),"},{"line_number":1016,"context_line":"                it.c.min_unit \u003c\u003d amount,"},{"line_number":1017,"context_line":"                it.c.max_unit \u003e\u003d amount,"},{"line_number":1018,"context_line":"                amount % it.c.step_size \u003d\u003d 0,"},{"line_number":1019,"context_line":"            )"},{"line_number":1020,"context_line":"            if not sps:"},{"line_number":1021,"context_line":"                where_conds.append(usage_cond)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_57825402","line":1018,"range":{"start_line":1013,"start_character":0,"end_line":1018,"end_character":45},"updated":"2017-05-02 14:13:02.000000000","message":"we seem to use this construct often. Is there any way to make it generic?","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd2049c11d3fffbfd924e0e849fa2a0f2710010e","unresolved":false,"context_lines":[{"line_number":1010,"context_line":""},{"line_number":1011,"context_line":"            usage_cond \u003d sa.and_("},{"line_number":1012,"context_line":"                ("},{"line_number":1013,"context_line":"                (sql.func.coalesce(ut.c.used, 0) + amount) \u003c\u003d"},{"line_number":1014,"context_line":"                (it.c.total - it.c.reserved) * it.c.allocation_ratio"},{"line_number":1015,"context_line":"                ),"},{"line_number":1016,"context_line":"                it.c.min_unit \u003c\u003d amount,"},{"line_number":1017,"context_line":"                it.c.max_unit \u003e\u003d amount,"},{"line_number":1018,"context_line":"                amount % it.c.step_size \u003d\u003d 0,"},{"line_number":1019,"context_line":"            )"},{"line_number":1020,"context_line":"            if not sps:"},{"line_number":1021,"context_line":"                where_conds.append(usage_cond)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_f2360ee8","line":1018,"range":{"start_line":1013,"start_character":0,"end_line":1018,"end_character":45},"in_reply_to":"5ff73747_57825402","updated":"2017-05-02 14:39:45.000000000","message":"yeah, good point. I\u0027ll make it into a little utility func in a later patch.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"3aa2928bda49f9497a1e81ad73d64abfb9cd8466","unresolved":false,"context_lines":[{"line_number":1180,"context_line":"            sa.distinct(_INV_TBL.c.resource_class_id)) \u003d\u003d len(resources))"},{"line_number":1181,"context_line":""},{"line_number":1182,"context_line":"        rps \u003d query.all()"},{"line_number":1183,"context_line":"        if not rps:"},{"line_number":1184,"context_line":"            # NOTE(jaypipes): OK, so there are no resource providers that meet"},{"line_number":1185,"context_line":"            # all of the requested resources constraints. Here, we will go"},{"line_number":1186,"context_line":"            # further down the rabbit hole and look for resource providers that"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_2649d731","line":1183,"updated":"2017-05-02 14:57:48.000000000","message":"The new thing I noticed that may be an issue (and perhaps supports having just one query):\n\nWe can reach this line with \u0027uuid\u0027, \u0027member_of\u0027 and \u0027name\u0027 filters potentially set. It\u0027s unlikely but if someone passes in filters with one of those three and \u0027resources\u0027 set, and the resources fail to match, then we ought to be passing uuid, member_of and name into _get_all_with_shared.\n\nIt doesn\u0027t make a lot of sense to do it, but it is currently possible so we need to do something like one of the following:\n\n* decide that combos of those filters with \u0027resources\u0027 is a no go and defend against it\n* pass the filters along to both queries\n* use one query that does it all","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6341875d26034a637fb60f1e731447a64bfc91e3","unresolved":false,"context_lines":[{"line_number":1180,"context_line":"            sa.distinct(_INV_TBL.c.resource_class_id)) \u003d\u003d len(resources))"},{"line_number":1181,"context_line":""},{"line_number":1182,"context_line":"        rps \u003d query.all()"},{"line_number":1183,"context_line":"        if not rps:"},{"line_number":1184,"context_line":"            # NOTE(jaypipes): OK, so there are no resource providers that meet"},{"line_number":1185,"context_line":"            # all of the requested resources constraints. Here, we will go"},{"line_number":1186,"context_line":"            # further down the rabbit hole and look for resource providers that"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_06849bb0","line":1183,"in_reply_to":"5ff73747_2649d731","updated":"2017-05-02 15:07:45.000000000","message":"Yeah, noted.\n\nI think my main objective for these first patches in the series is to test the queries for handling shared provider inventories in isolation from the existing _get_all_by_filters() method, then gradually over a series of patches we can just migrate to using _get_all_with_shared() all the time if resources is set.\n\nPretty sure it will be more digestible if we keep this new stuff segregated into separate testable functions for the moment...","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"fb973434079523f5b6066c48ccdadac11b7d05e5","unresolved":false,"context_lines":[{"line_number":1181,"context_line":""},{"line_number":1182,"context_line":"        rps \u003d query.all()"},{"line_number":1183,"context_line":"        if not rps:"},{"line_number":1184,"context_line":"            # NOTE(jaypipes): OK, so there are no resource providers that meet"},{"line_number":1185,"context_line":"            # all of the requested resources constraints. Here, we will go"},{"line_number":1186,"context_line":"            # further down the rabbit hole and look for resource providers that"},{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_862670ba","line":1184,"updated":"2017-05-02 12:42:47.000000000","message":"The impression I got from IRC discussion yesterday was that the predisposition to consume local disk providing compute nodes before shared disk using compute nodes _might_ be an issue for some people.\n\nAs I\u0027m running no cloud of my own, I\u0027m not really in a position to comment on how much of an issue that is, but seems it should be mixed into the discussion.\n\nThere are plenty of ways we can probably think of to get around it (traits for example) if we want to allow people to control the behavior, but we should be aware this is introducing a pattern.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6341875d26034a637fb60f1e731447a64bfc91e3","unresolved":false,"context_lines":[{"line_number":1181,"context_line":""},{"line_number":1182,"context_line":"        rps \u003d query.all()"},{"line_number":1183,"context_line":"        if not rps:"},{"line_number":1184,"context_line":"            # NOTE(jaypipes): OK, so there are no resource providers that meet"},{"line_number":1185,"context_line":"            # all of the requested resources constraints. Here, we will go"},{"line_number":1186,"context_line":"            # further down the rabbit hole and look for resource providers that"},{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_26b0b78b","line":1184,"in_reply_to":"5ff73747_8608abd9","updated":"2017-05-02 15:07:45.000000000","message":"Sure, see above. Agree that\u0027s a good idea but I\u0027d like to isolate (for testing purposes) the changes introduced in these patches and then gradually merge the code together.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd2049c11d3fffbfd924e0e849fa2a0f2710010e","unresolved":false,"context_lines":[{"line_number":1181,"context_line":""},{"line_number":1182,"context_line":"        rps \u003d query.all()"},{"line_number":1183,"context_line":"        if not rps:"},{"line_number":1184,"context_line":"            # NOTE(jaypipes): OK, so there are no resource providers that meet"},{"line_number":1185,"context_line":"            # all of the requested resources constraints. Here, we will go"},{"line_number":1186,"context_line":"            # further down the rabbit hole and look for resource providers that"},{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_f2c60e4a","line":1184,"in_reply_to":"5ff73747_862670ba","updated":"2017-05-02 14:39:45.000000000","message":"Yes, I hear you on concerns about \"preferring\" the locally-sourced resources before looking for shared providers. And yes, there\u0027s a number of things we can do in future patches to address this if it is perceived as an issue by operators:\n\n1) We could put a little machine learning into the code here that would sense if we end up going into this more advanced SQL expression all the time (meaning all requests end up using a shared resource provider for something) and if so, pass along the other filters to _get_all_with_shared() and just default to running that method instead of trying the SQL strategy that requires the providers to have capacity for *all* resources at once.\n\n2) We could add a configuration setting to toggle the behaviour above in #1\n\n3) We could run some pre-SQL heuristics to identify whether a request is likely to need to query for shared capacity. This option is likely to introduce a bunch of one-off conditional code though that is specific to certain resource classes -- e.g. \"does this request disk? do I know there are shared storage providers? if yes, run _get_all_with_shared() else run the \"normal\" SQL.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"3aa2928bda49f9497a1e81ad73d64abfb9cd8466","unresolved":false,"context_lines":[{"line_number":1181,"context_line":""},{"line_number":1182,"context_line":"        rps \u003d query.all()"},{"line_number":1183,"context_line":"        if not rps:"},{"line_number":1184,"context_line":"            # NOTE(jaypipes): OK, so there are no resource providers that meet"},{"line_number":1185,"context_line":"            # all of the requested resources constraints. Here, we will go"},{"line_number":1186,"context_line":"            # further down the rabbit hole and look for resource providers that"},{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_8608abd9","line":1184,"in_reply_to":"5ff73747_f2c60e4a","updated":"2017-05-02 14:57:48.000000000","message":"Is there a 4th option which is \"use one query instead of two so all results are always returned\"?","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"fb973434079523f5b6066c48ccdadac11b7d05e5","unresolved":false,"context_lines":[{"line_number":1185,"context_line":"            # all of the requested resources constraints. Here, we will go"},{"line_number":1186,"context_line":"            # further down the rabbit hole and look for resource providers that"},{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"},{"line_number":1188,"context_line":"            # sharing provider that does have the needed capacity."},{"line_number":1189,"context_line":"            rps \u003d ResourceProviderList._get_all_with_shared(context, resources)"},{"line_number":1190,"context_line":"        return rps"},{"line_number":1191,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_8361c214","line":1188,"updated":"2017-05-02 12:42:47.000000000","message":"See my comment above about the disorentation about \"locally\" and \"OR\".","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6341875d26034a637fb60f1e731447a64bfc91e3","unresolved":false,"context_lines":[{"line_number":1185,"context_line":"            # all of the requested resources constraints. Here, we will go"},{"line_number":1186,"context_line":"            # further down the rabbit hole and look for resource providers that"},{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"},{"line_number":1188,"context_line":"            # sharing provider that does have the needed capacity."},{"line_number":1189,"context_line":"            rps \u003d ResourceProviderList._get_all_with_shared(context, resources)"},{"line_number":1190,"context_line":"        return rps"},{"line_number":1191,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_c6a6b354","line":1188,"in_reply_to":"5ff73747_26f797d7","updated":"2017-05-02 15:07:45.000000000","message":"lol :)","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"3aa2928bda49f9497a1e81ad73d64abfb9cd8466","unresolved":false,"context_lines":[{"line_number":1185,"context_line":"            # all of the requested resources constraints. Here, we will go"},{"line_number":1186,"context_line":"            # further down the rabbit hole and look for resource providers that"},{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"},{"line_number":1188,"context_line":"            # sharing provider that does have the needed capacity."},{"line_number":1189,"context_line":"            rps \u003d ResourceProviderList._get_all_with_shared(context, resources)"},{"line_number":1190,"context_line":"        return rps"},{"line_number":1191,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_26f797d7","line":1188,"in_reply_to":"5ff73747_8361c214","updated":"2017-05-02 14:57:48.000000000","message":"I take it all back.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"41c9722c91d90c3e9ec768de3cb3e6e1d44dfe3f","unresolved":false,"context_lines":[{"line_number":1186,"context_line":"            # further down the rabbit hole and look for resource providers that"},{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"},{"line_number":1188,"context_line":"            # sharing provider that does have the needed capacity."},{"line_number":1189,"context_line":"            rps \u003d ResourceProviderList._get_all_with_shared(context, resources)"},{"line_number":1190,"context_line":"        return rps"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_9729fcf3","line":1189,"updated":"2017-05-02 14:13:02.000000000","message":"Is this method so much slower that we avoid it at all costs? IOW, what if we just called this instead of only when there are no local-only providers? This would avoid favoring hosts with local disk over shared disk.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7512789abc5d746f08dd1061e08ac8013798d28e","unresolved":false,"context_lines":[{"line_number":1186,"context_line":"            # further down the rabbit hole and look for resource providers that"},{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"},{"line_number":1188,"context_line":"            # sharing provider that does have the needed capacity."},{"line_number":1189,"context_line":"            rps \u003d ResourceProviderList._get_all_with_shared(context, resources)"},{"line_number":1190,"context_line":"        return rps"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_8908bcd9","line":1189,"in_reply_to":"5ff73747_267557cd","updated":"2017-05-02 15:20:21.000000000","message":"I think what we need are tests that compare both approaches to make sure that they return the same results in the simple case (non-shared). If we are confident that works, I would prefer one single code path, which we can always tweak later if it turns out that performance is an issue.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"3aa2928bda49f9497a1e81ad73d64abfb9cd8466","unresolved":false,"context_lines":[{"line_number":1186,"context_line":"            # further down the rabbit hole and look for resource providers that"},{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"},{"line_number":1188,"context_line":"            # sharing provider that does have the needed capacity."},{"line_number":1189,"context_line":"            rps \u003d ResourceProviderList._get_all_with_shared(context, resources)"},{"line_number":1190,"context_line":"        return rps"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_462bc32f","line":1189,"in_reply_to":"5ff73747_32ea36ac","updated":"2017-05-02 14:57:48.000000000","message":"Perhaps yet another patch would merge it all into one after we\u0027ve decided we like the technique?\n\nDespite the complexity, the danger of changing one but not the other (for whatever reason) seems a greater concern.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6341875d26034a637fb60f1e731447a64bfc91e3","unresolved":false,"context_lines":[{"line_number":1186,"context_line":"            # further down the rabbit hole and look for resource providers that"},{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"},{"line_number":1188,"context_line":"            # sharing provider that does have the needed capacity."},{"line_number":1189,"context_line":"            rps \u003d ResourceProviderList._get_all_with_shared(context, resources)"},{"line_number":1190,"context_line":"        return rps"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_267557cd","line":1189,"in_reply_to":"5ff73747_462bc32f","updated":"2017-05-02 15:07:45.000000000","message":"heh, jinx.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd2049c11d3fffbfd924e0e849fa2a0f2710010e","unresolved":false,"context_lines":[{"line_number":1186,"context_line":"            # further down the rabbit hole and look for resource providers that"},{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"},{"line_number":1188,"context_line":"            # sharing provider that does have the needed capacity."},{"line_number":1189,"context_line":"            rps \u003d ResourceProviderList._get_all_with_shared(context, resources)"},{"line_number":1190,"context_line":"        return rps"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_32ea36ac","line":1189,"in_reply_to":"5ff73747_9729fcf3","updated":"2017-05-02 14:39:45.000000000","message":"I\u0027m not sure to be honest. I can run benchmarks, though.\n\nThe primary focus of the effort here was to minimize the amount of code change needed in the primary _get_all_by_filters() method.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"fb973434079523f5b6066c48ccdadac11b7d05e5","unresolved":false,"context_lines":[{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"},{"line_number":1188,"context_line":"            # sharing provider that does have the needed capacity."},{"line_number":1189,"context_line":"            rps \u003d ResourceProviderList._get_all_with_shared(context, resources)"},{"line_number":1190,"context_line":"        return rps"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"    @classmethod"},{"line_number":1193,"context_line":"    def get_all_by_filters(cls, context, filters\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_86e79047","line":1190,"updated":"2017-05-02 12:42:47.000000000","message":"Speaking of traits, at what stage of the querying had you imagined integrating filtering by traits? Would that be something that would be integrated into both of the two big queries (the new one you\u0027ve just created and the called by \u0027query.all()\u0027 in this method or we would filter the resulting rps list after they had both run?\n\nI\u0027m asking this here and now in case it might change the shape of things.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"3aa2928bda49f9497a1e81ad73d64abfb9cd8466","unresolved":false,"context_lines":[{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"},{"line_number":1188,"context_line":"            # sharing provider that does have the needed capacity."},{"line_number":1189,"context_line":"            rps \u003d ResourceProviderList._get_all_with_shared(context, resources)"},{"line_number":1190,"context_line":"        return rps"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"    @classmethod"},{"line_number":1193,"context_line":"    def get_all_by_filters(cls, context, filters\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_86d66b27","line":1190,"in_reply_to":"5ff73747_32bb5673","updated":"2017-05-02 14:57:48.000000000","message":"So, in the current state of affairs we would need to add this to both queries, right?","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6341875d26034a637fb60f1e731447a64bfc91e3","unresolved":false,"context_lines":[{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"},{"line_number":1188,"context_line":"            # sharing provider that does have the needed capacity."},{"line_number":1189,"context_line":"            rps \u003d ResourceProviderList._get_all_with_shared(context, resources)"},{"line_number":1190,"context_line":"        return rps"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"    @classmethod"},{"line_number":1193,"context_line":"    def get_all_by_filters(cls, context, filters\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_e67e4fae","line":1190,"in_reply_to":"5ff73747_86d66b27","updated":"2017-05-02 15:07:45.000000000","message":"yep.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd2049c11d3fffbfd924e0e849fa2a0f2710010e","unresolved":false,"context_lines":[{"line_number":1187,"context_line":"            # either have the resources \"locally\" OR they are associated with a"},{"line_number":1188,"context_line":"            # sharing provider that does have the needed capacity."},{"line_number":1189,"context_line":"            rps \u003d ResourceProviderList._get_all_with_shared(context, resources)"},{"line_number":1190,"context_line":"        return rps"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"    @classmethod"},{"line_number":1193,"context_line":"    def get_all_by_filters(cls, context, filters\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_32bb5673","line":1190,"in_reply_to":"5ff73747_86e79047","updated":"2017-05-02 14:39:45.000000000","message":"Great question. I was planning on passing the required traits collection to _get_all_with_shared() and using those traits in an additional filtering join against the resource_providers table. Something like this after the loop over resource classes on line 1047:\n\n tt \u003d sa.alias(_TRAIT_TBL, name\u003d\u0027tt\u0027)\n rpt \u003d sa.alias(_RP_TRAIT_TBL, name\u003d\u0027rpt\u0027)\n if traits:\n     rp_trait_join \u003d sa.join(\n         join_chain, rpt,\n         rp.c.resource_provider_id \u003d\u003d rpt.c.resource_provider_id,\n     )\n     trait_join \u003d sa.join(\n         rp_trait_join, tt,\n         rpt.c.trait_id \u003d\u003d tt.id\n     )\n     where_conds.append(tt.id.in_(traits))\n     join_chain \u003d trait_join","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"041c7cc983471a610f354acfdc97b2455b9d4e58","unresolved":false,"context_lines":[{"line_number":705,"context_line":"    return [r[0] for r in ctx.session.execute(sel)]"},{"line_number":706,"context_line":""},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"@db_api.api_context_manager.reader"},{"line_number":709,"context_line":"def _get_all_with_shared(ctx, resources):"},{"line_number":710,"context_line":"    \"\"\"Uses some more advanced SQL to find providers that either have the"},{"line_number":711,"context_line":"    requested resources \"locally\" or are associated with a provider that shares"}],"source_content_type":"text/x-python","patch_set":6,"id":"5ff73747_095b5f2e","line":708,"updated":"2017-05-03 18:36:47.000000000","message":"+1 on making them module level.\n\n-1 on having this (and _get_providers_with_shared_capacity() above) between the ResourceProvider class and the ResourceProviderList class definitions. I would prefer that they get moved up above the class definitions with the rest of the module-level code. to make this clearer, and to keep the class defs together.","commit_id":"5ebdddf5f08bf8ef522e4a26fd355fbbd6e60456"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9ca45169362aebaef689b3b947ea64c83371fbfa","unresolved":false,"context_lines":[{"line_number":705,"context_line":"    return [r[0] for r in ctx.session.execute(sel)]"},{"line_number":706,"context_line":""},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"@db_api.api_context_manager.reader"},{"line_number":709,"context_line":"def _get_all_with_shared(ctx, resources):"},{"line_number":710,"context_line":"    \"\"\"Uses some more advanced SQL to find providers that either have the"},{"line_number":711,"context_line":"    requested resources \"locally\" or are associated with a provider that shares"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_842651b7","line":708,"in_reply_to":"3f044301_d867a025","updated":"2017-05-04 14:30:12.000000000","message":"It\u0027s way too big because of comments. I\u0027d like to continue with our convention of keeping the query code close to the objects here. Is there any way we could cut these docs out into actual doc/ files and just refer to them here? _Most_ of the time people won\u0027t need the full SQL backstory when working on these files, so it seems reasonable to indirectly refer to the details.","commit_id":"5ebdddf5f08bf8ef522e4a26fd355fbbd6e60456"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"5193171424969db6bd20b810b873b3c55c9ca191","unresolved":false,"context_lines":[{"line_number":705,"context_line":"    return [r[0] for r in ctx.session.execute(sel)]"},{"line_number":706,"context_line":""},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"@db_api.api_context_manager.reader"},{"line_number":709,"context_line":"def _get_all_with_shared(ctx, resources):"},{"line_number":710,"context_line":"    \"\"\"Uses some more advanced SQL to find providers that either have the"},{"line_number":711,"context_line":"    requested resources \"locally\" or are associated with a provider that shares"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_d867a025","line":708,"in_reply_to":"5ff73747_095b5f2e","updated":"2017-05-04 13:14:52.000000000","message":"Yeah, may as well put this with the other private module-level SQL bits.\n\nAnd then once those are all in the same place move the entire chunk to another file. This file is way too big.","commit_id":"5ebdddf5f08bf8ef522e4a26fd355fbbd6e60456"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"e63ec4a28be98200fec5dca1e9a78f72a08b0719","unresolved":false,"context_lines":[{"line_number":706,"context_line":""},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"@db_api.api_context_manager.reader"},{"line_number":709,"context_line":"def _get_all_with_shared(ctx, resources):"},{"line_number":710,"context_line":"    \"\"\"Uses some more advanced SQL to find providers that either have the"},{"line_number":711,"context_line":"    requested resources \"locally\" or are associated with a provider that shares"},{"line_number":712,"context_line":"    those requested resources."}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_05d50ae5","line":709,"updated":"2017-05-12 08:20:22.000000000","message":"Am I correct the reason for that big SQL is we can not use a serializable transaction isolation level because Galera doesn\u0027t support it? \n\nAnd we don\u0027t want \"select for update\" because\nit\u0027s a pessimistic lock and the current logic provides an optimistic strategy based on RP updates with generations, right?\n\nSo the only way to get consistent data is to get it with one query, right?","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"97ccae8c08034bfbccad83e801f6d7105d6ebb74","unresolved":false,"context_lines":[{"line_number":706,"context_line":""},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"@db_api.api_context_manager.reader"},{"line_number":709,"context_line":"def _get_all_with_shared(ctx, resources):"},{"line_number":710,"context_line":"    \"\"\"Uses some more advanced SQL to find providers that either have the"},{"line_number":711,"context_line":"    requested resources \"locally\" or are associated with a provider that shares"},{"line_number":712,"context_line":"    those requested resources."}],"source_content_type":"text/x-python","patch_set":9,"id":"ff0f0b1f_134a26ab","line":709,"in_reply_to":"3f044301_05d50ae5","updated":"2017-05-22 13:17:17.000000000","message":"The big SQL doesn\u0027t have anything to do with transaction isolation level, no.\n\nWe don\u0027t want to use SELECT FOR UPDATE because we\u0027re not selecting for update here. We\u0027re just selecting. We handle concurrent updates to allocations and inventories using the generation field in the resource_providers table.\n\nWe do a big query here instead of multiple queries for efficiency reasons. If it turns out to be more efficient to do multiple queries after a certain number of resources (could very well happen, but I haven\u0027t seen it yet), we could break this into multiple queries, sure.","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"e63ec4a28be98200fec5dca1e9a78f72a08b0719","unresolved":false,"context_lines":[{"line_number":826,"context_line":"    # vCPU and RAM associated with the same aggregate as the provider"},{"line_number":827,"context_line":"    # representing the shared storage pool:"},{"line_number":828,"context_line":"    #"},{"line_number":829,"context_line":"    # SELECT rp.*"},{"line_number":830,"context_line":"    # FROM resource_providers AS rp"},{"line_number":831,"context_line":"    # INNER JOIN inventories AS inv_vcpu"},{"line_number":832,"context_line":"    #  ON rp.id \u003d inv_vcpu.resource_provider_id"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_e5f1fe6c","line":829,"updated":"2017-05-12 08:20:22.000000000","message":"nit: rp.id, maybe?","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"97ccae8c08034bfbccad83e801f6d7105d6ebb74","unresolved":false,"context_lines":[{"line_number":826,"context_line":"    # vCPU and RAM associated with the same aggregate as the provider"},{"line_number":827,"context_line":"    # representing the shared storage pool:"},{"line_number":828,"context_line":"    #"},{"line_number":829,"context_line":"    # SELECT rp.*"},{"line_number":830,"context_line":"    # FROM resource_providers AS rp"},{"line_number":831,"context_line":"    # INNER JOIN inventories AS inv_vcpu"},{"line_number":832,"context_line":"    #  ON rp.id \u003d inv_vcpu.resource_provider_id"}],"source_content_type":"text/x-python","patch_set":9,"id":"ff0f0b1f_fe9c19c1","line":829,"in_reply_to":"3f044301_e5f1fe6c","updated":"2017-05-22 13:17:17.000000000","message":"Done","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"e63ec4a28be98200fec5dca1e9a78f72a08b0719","unresolved":false,"context_lines":[{"line_number":828,"context_line":"    #"},{"line_number":829,"context_line":"    # SELECT rp.*"},{"line_number":830,"context_line":"    # FROM resource_providers AS rp"},{"line_number":831,"context_line":"    # INNER JOIN inventories AS inv_vcpu"},{"line_number":832,"context_line":"    #  ON rp.id \u003d inv_vcpu.resource_provider_id"},{"line_number":833,"context_line":"    #  AND inv_vcpu.resource_class_id \u003d $VCPU_ID"},{"line_number":834,"context_line":"    # LEFT JOIN ("},{"line_number":835,"context_line":"    #  SELECT resource_provider_id, SUM(used) AS used"},{"line_number":836,"context_line":"    #  FROM allocations"},{"line_number":837,"context_line":"    #  WHERE resource_class_id \u003d $VCPU_ID"},{"line_number":838,"context_line":"    #  GROUP BY resource_provider_id"},{"line_number":839,"context_line":"    # ) AS usage_vcpu"},{"line_number":840,"context_line":"    #  ON inv_vcpu.resource_provider_id \u003d \\"},{"line_number":841,"context_line":"    #       usage_vcpu.resource_provider_id"},{"line_number":842,"context_line":"    # INNER JOIN inventories AS inv_memory_mb"},{"line_number":843,"context_line":"    # ON inv_vcpu.resource_provider_id \u003d inv_memory_mb.resource_provider_id"},{"line_number":844,"context_line":"    # AND inv_memory_mb.resource_class_id \u003d $MEMORY_MB_ID"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_281a1d6e","line":841,"range":{"start_line":831,"start_character":0,"end_line":841,"end_character":43},"updated":"2017-05-12 08:20:22.000000000","message":"The size of SQL query depends on a number of resources we requested. So with mysql default max_allowed_packet (1Mb) it\u0027s not more than 1000 resources and speed of sending \nthis request is also affected.\n\nDo you consider to use different result \"table\" like:\n\n| resource provider | resource class | usage |\n\ninstead of \n\n| resource provider | vcpu usage | memory usage | ... |","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"97ccae8c08034bfbccad83e801f6d7105d6ebb74","unresolved":false,"context_lines":[{"line_number":828,"context_line":"    #"},{"line_number":829,"context_line":"    # SELECT rp.*"},{"line_number":830,"context_line":"    # FROM resource_providers AS rp"},{"line_number":831,"context_line":"    # INNER JOIN inventories AS inv_vcpu"},{"line_number":832,"context_line":"    #  ON rp.id \u003d inv_vcpu.resource_provider_id"},{"line_number":833,"context_line":"    #  AND inv_vcpu.resource_class_id \u003d $VCPU_ID"},{"line_number":834,"context_line":"    # LEFT JOIN ("},{"line_number":835,"context_line":"    #  SELECT resource_provider_id, SUM(used) AS used"},{"line_number":836,"context_line":"    #  FROM allocations"},{"line_number":837,"context_line":"    #  WHERE resource_class_id \u003d $VCPU_ID"},{"line_number":838,"context_line":"    #  GROUP BY resource_provider_id"},{"line_number":839,"context_line":"    # ) AS usage_vcpu"},{"line_number":840,"context_line":"    #  ON inv_vcpu.resource_provider_id \u003d \\"},{"line_number":841,"context_line":"    #       usage_vcpu.resource_provider_id"},{"line_number":842,"context_line":"    # INNER JOIN inventories AS inv_memory_mb"},{"line_number":843,"context_line":"    # ON inv_vcpu.resource_provider_id \u003d inv_memory_mb.resource_provider_id"},{"line_number":844,"context_line":"    # AND inv_memory_mb.resource_class_id \u003d $MEMORY_MB_ID"}],"source_content_type":"text/x-python","patch_set":9,"id":"ff0f0b1f_1e4fcd34","line":841,"range":{"start_line":831,"start_character":0,"end_line":841,"end_character":43},"in_reply_to":"3f044301_281a1d6e","updated":"2017-05-22 13:17:17.000000000","message":"I can run some experiments with a joint usage table and see if the query could be made more efficient. However, the WHERE clause will become really, really crazy and the way the join conditions line up in this SQL actually is extremely efficient and all of the joins are handled entirely via index conditions.","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"309913359173a626a81dbdc04ff2e08d1a59634a","unresolved":false,"context_lines":[{"line_number":862,"context_line":"    # ) AS usage_disk_gb"},{"line_number":863,"context_line":"    #  ON inv_disk_gb.resource_provider_id \u003d \\"},{"line_number":864,"context_line":"    #       usage_disk_gb.resource_provider_id"},{"line_number":865,"context_line":"    # LEFT JOIN resource_provider_aggregates AS shared_disk_gb"},{"line_number":866,"context_line":"    #  ON inv_memory_mb.resource_provider_id \u003d \\"},{"line_number":867,"context_line":"    #       shared_disk.resource_provider_id"},{"line_number":868,"context_line":"    # LEFT JOIN resource_provider_aggregates AS sharing_disk_gb"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_b325bcc4","line":865,"updated":"2017-05-09 09:49:55.000000000","message":"Do we care about the case: two compute nodes, one is in the shared storage pool, another one is not. But both of them have enough resources for the request.\n\nI write a test on this case quickly, looks like it doesn\u0027t work.","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"98e637f9f25de36e6f64a95644da9eb4391c7255","unresolved":false,"context_lines":[{"line_number":862,"context_line":"    # ) AS usage_disk_gb"},{"line_number":863,"context_line":"    #  ON inv_disk_gb.resource_provider_id \u003d \\"},{"line_number":864,"context_line":"    #       usage_disk_gb.resource_provider_id"},{"line_number":865,"context_line":"    # LEFT JOIN resource_provider_aggregates AS shared_disk_gb"},{"line_number":866,"context_line":"    #  ON inv_memory_mb.resource_provider_id \u003d \\"},{"line_number":867,"context_line":"    #       shared_disk.resource_provider_id"},{"line_number":868,"context_line":"    # LEFT JOIN resource_provider_aggregates AS sharing_disk_gb"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_0f8d59d2","line":865,"in_reply_to":"3f044301_7f8495b1","updated":"2017-05-10 03:43:58.000000000","message":"ok... found a bug in my own test :(","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"90d1636da0ba0580d02f2a7618103848fe0ce665","unresolved":false,"context_lines":[{"line_number":862,"context_line":"    # ) AS usage_disk_gb"},{"line_number":863,"context_line":"    #  ON inv_disk_gb.resource_provider_id \u003d \\"},{"line_number":864,"context_line":"    #       usage_disk_gb.resource_provider_id"},{"line_number":865,"context_line":"    # LEFT JOIN resource_provider_aggregates AS shared_disk_gb"},{"line_number":866,"context_line":"    #  ON inv_memory_mb.resource_provider_id \u003d \\"},{"line_number":867,"context_line":"    #       shared_disk.resource_provider_id"},{"line_number":868,"context_line":"    # LEFT JOIN resource_provider_aggregates AS sharing_disk_gb"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_7f8495b1","line":865,"in_reply_to":"3f044301_b325bcc4","updated":"2017-05-09 13:57:50.000000000","message":"It does indeed work. There is a functional test added after this patch that shows providers of \"local\" disk AND shared disk being returned.\n\nhttps://review.openstack.org/#/c/461899/5/nova/tests/functional/db/test_resource_provider.py\n\nThe OR condition in the WHERE clause below on 900-901 is what provides this behaviour.","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"f3b36e36cdfa20fbfeb2fa4a8f660581da32de3c","unresolved":false,"context_lines":[{"line_number":862,"context_line":"    # ) AS usage_disk_gb"},{"line_number":863,"context_line":"    #  ON inv_disk_gb.resource_provider_id \u003d \\"},{"line_number":864,"context_line":"    #       usage_disk_gb.resource_provider_id"},{"line_number":865,"context_line":"    # LEFT JOIN resource_provider_aggregates AS shared_disk_gb"},{"line_number":866,"context_line":"    #  ON inv_memory_mb.resource_provider_id \u003d \\"},{"line_number":867,"context_line":"    #       shared_disk.resource_provider_id"},{"line_number":868,"context_line":"    # LEFT JOIN resource_provider_aggregates AS sharing_disk_gb"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_b30edc38","line":865,"in_reply_to":"3f044301_b325bcc4","updated":"2017-05-09 09:51:16.000000000","message":"The test only returns the compute nodes in the shared storage pool. The expected result should be two compute nodes returned, right?","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"e63ec4a28be98200fec5dca1e9a78f72a08b0719","unresolved":false,"context_lines":[{"line_number":865,"context_line":"    # LEFT JOIN resource_provider_aggregates AS shared_disk_gb"},{"line_number":866,"context_line":"    #  ON inv_memory_mb.resource_provider_id \u003d \\"},{"line_number":867,"context_line":"    #       shared_disk.resource_provider_id"},{"line_number":868,"context_line":"    # LEFT JOIN resource_provider_aggregates AS sharing_disk_gb"},{"line_number":869,"context_line":"    #  ON shared_disk_gb.aggregate_id \u003d sharing_disk_gb.aggregate_id"},{"line_number":870,"context_line":"    # AND sharing_disk_gb.resource_provider_id IN ($RPS_SHARING_DISK)"},{"line_number":871,"context_line":"    # WHERE ("}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_88a7f10b","line":868,"updated":"2017-05-12 08:20:22.000000000","message":"Can we have several shared resources with the same resource class (say disk_gb) in one aggregate and which one will be chosen?","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"97ccae8c08034bfbccad83e801f6d7105d6ebb74","unresolved":false,"context_lines":[{"line_number":865,"context_line":"    # LEFT JOIN resource_provider_aggregates AS shared_disk_gb"},{"line_number":866,"context_line":"    #  ON inv_memory_mb.resource_provider_id \u003d \\"},{"line_number":867,"context_line":"    #       shared_disk.resource_provider_id"},{"line_number":868,"context_line":"    # LEFT JOIN resource_provider_aggregates AS sharing_disk_gb"},{"line_number":869,"context_line":"    #  ON shared_disk_gb.aggregate_id \u003d sharing_disk_gb.aggregate_id"},{"line_number":870,"context_line":"    # AND sharing_disk_gb.resource_provider_id IN ($RPS_SHARING_DISK)"},{"line_number":871,"context_line":"    # WHERE ("}],"source_content_type":"text/x-python","patch_set":9,"id":"ff0f0b1f_5ef785d4","line":868,"in_reply_to":"3f044301_88a7f10b","updated":"2017-05-22 13:17:17.000000000","message":"Which one is chosen is not the purpose of this query. The scheduler and/or compute node is what picks the specific resource provider to consume from. All this query does is return the resource providers that have capacity (or are associated with a sharing provider).","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"19eaa0a53593ab49fff4218692f441aeb27a56d5","unresolved":false,"context_lines":[{"line_number":867,"context_line":"    #       shared_disk.resource_provider_id"},{"line_number":868,"context_line":"    # LEFT JOIN resource_provider_aggregates AS sharing_disk_gb"},{"line_number":869,"context_line":"    #  ON shared_disk_gb.aggregate_id \u003d sharing_disk_gb.aggregate_id"},{"line_number":870,"context_line":"    # AND sharing_disk_gb.resource_provider_id IN ($RPS_SHARING_DISK)"},{"line_number":871,"context_line":"    # WHERE ("},{"line_number":872,"context_line":"    #   ("},{"line_number":873,"context_line":"    #     COALESCE(usage_vcpu.used, 0) + $AMOUNT_VCPU \u003c\u003d"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_35750233","line":870,"range":{"start_line":870,"start_character":50,"end_line":870,"end_character":69},"updated":"2017-05-10 06:37:52.000000000","message":"I\u0027m not giving up yet to challenge this huge sql yet....\n\nSo here the question, \u0027_get_providers_with_shared_capacity\u0027 will return the shared resource providers which have enough resource for the request. And then do this SQL to query the final result. So there should be a race case?  The usage of resources are changed after \"_get_providers_with_shared_capacity\". To avoid that race case, we can use \u0027select for update\u0027 or just do one DB call.\n\nThen I have a try for single DB call to query the usage both for shared and non-shared. I write a patch based yours https://review.openstack.org/#/c/463668/1, the test passed.\n\nBasically, the changes are as below:\n\nFirst, we change the \u0027_get_providers_with_shared_capacity\u0027 return all the shared resource RPs, and didn\u0027t care about whether the resource provider have enough resource or not.\n\nSecond, we need to add more joins:\n\nleft join inventories as sharing_inv_disk_gb\n\ton sharing_inv_disk_gb.resource_provider_id \u003d sharing_disk_gb.resource_provider_id\nleft join (\n\tselect resource_provider_id, SUM(used) as used\n\tfrom allocations\n\twhere resource_class_id \u003d 1\n\tgroup by resource_provider_id\n) as sharing_usage_disk\n\ton sharing_inv_disk_gb.resource_provider_id \u003d sharing_usage_disk.resource_provider_id\n\nThird, we need change in the \u0027where\u0027 clause, I write at line 901","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"97ccae8c08034bfbccad83e801f6d7105d6ebb74","unresolved":false,"context_lines":[{"line_number":867,"context_line":"    #       shared_disk.resource_provider_id"},{"line_number":868,"context_line":"    # LEFT JOIN resource_provider_aggregates AS sharing_disk_gb"},{"line_number":869,"context_line":"    #  ON shared_disk_gb.aggregate_id \u003d sharing_disk_gb.aggregate_id"},{"line_number":870,"context_line":"    # AND sharing_disk_gb.resource_provider_id IN ($RPS_SHARING_DISK)"},{"line_number":871,"context_line":"    # WHERE ("},{"line_number":872,"context_line":"    #   ("},{"line_number":873,"context_line":"    #     COALESCE(usage_vcpu.used, 0) + $AMOUNT_VCPU \u003c\u003d"}],"source_content_type":"text/x-python","patch_set":9,"id":"ff0f0b1f_3ec73167","line":870,"range":{"start_line":870,"start_character":50,"end_line":870,"end_character":69},"in_reply_to":"3f044301_35750233","updated":"2017-05-22 13:17:17.000000000","message":"See my response above on why SELECT FOR UPDATE isn\u0027t used here. We aren\u0027t writing records here, only selecting the providers that meet the requested resource conditions. The scheduler will eventually write allocations for providers. That allocation process ensures that inventory capacity is never exceeded for any provider involved in the allocation.","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"19eaa0a53593ab49fff4218692f441aeb27a56d5","unresolved":false,"context_lines":[{"line_number":898,"context_line":"    #     inv_disk_gb.max_unit \u003e\u003d $AMOUNT_DISK_GB AND"},{"line_number":899,"context_line":"    #     $AMOUNT_DISK_GB % inv_disk_gb.step_size \u003d\u003d 0"},{"line_number":900,"context_line":"    #   ) OR"},{"line_number":901,"context_line":"    #     shared_disk_gb.aggregate_id IS NOT NULL"},{"line_number":902,"context_line":"    # )"},{"line_number":903,"context_line":"    # GROUP BY rp.id"},{"line_number":904,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_35c3c23a","line":901,"updated":"2017-05-10 06:37:52.000000000","message":"follow the comment at line 870. We add more:\n\nshared_disk_gb.aggregate_id IS NOT NULL\nand\n(\n     (\n         COALESCE(sharing_usage_disk_gb.used, 0) + $AMOUNT_DISK_GB \u003c\u003d\n          (sharing_inv_disk_gb.total + sharing_inv_disk_gb.reserved)\n          * sharing_inv_disk_gb.allocation_ratio\n     )  AND\n     sharing_inv_disk_gb.min_unit \u003c\u003d $AMOUNT_DISK_GB AND\n     sharing_inv_disk_gb.max_unit \u003e\u003d $AMOUNT_DISK_GB AND\n     $AMOUNT_DISK_GB % sharing_inv_disk_gb.step_size \u003d\u003d 0\n)","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"97ccae8c08034bfbccad83e801f6d7105d6ebb74","unresolved":false,"context_lines":[{"line_number":898,"context_line":"    #     inv_disk_gb.max_unit \u003e\u003d $AMOUNT_DISK_GB AND"},{"line_number":899,"context_line":"    #     $AMOUNT_DISK_GB % inv_disk_gb.step_size \u003d\u003d 0"},{"line_number":900,"context_line":"    #   ) OR"},{"line_number":901,"context_line":"    #     shared_disk_gb.aggregate_id IS NOT NULL"},{"line_number":902,"context_line":"    # )"},{"line_number":903,"context_line":"    # GROUP BY rp.id"},{"line_number":904,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"ff0f0b1f_debef5b2","line":901,"in_reply_to":"3f044301_35c3c23a","updated":"2017-05-22 13:17:17.000000000","message":"There is no sharing_inv_disk_gb table, as you know. I\u0027m willing to accept a small race condition in between the call to _get_providers_with_shared_capacity() and here in order to simplify the query slightly. Alternately, we could return both the resource provider ID and the generation for the sharing providers and use that information to construct a temporary table of sharing providers at the generation we read from _get_providers_with_shared_capacity()?\n\nThat said, we\u0027re talking about a very small race condition here and not one that has a danger of introducing capacity exceeded, since the allocation cannot cause inventory to be exceeded.","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"54b07bbb8ccfa0e62ed549af1f742145cc84baaf","unresolved":false,"context_lines":[{"line_number":898,"context_line":"    #     inv_disk_gb.max_unit \u003e\u003d $AMOUNT_DISK_GB AND"},{"line_number":899,"context_line":"    #     $AMOUNT_DISK_GB % inv_disk_gb.step_size \u003d\u003d 0"},{"line_number":900,"context_line":"    #   ) OR"},{"line_number":901,"context_line":"    #     shared_disk_gb.aggregate_id IS NOT NULL"},{"line_number":902,"context_line":"    # )"},{"line_number":903,"context_line":"    # GROUP BY rp.id"},{"line_number":904,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"ff0f0b1f_da07f381","line":901,"in_reply_to":"ff0f0b1f_8cea770c","updated":"2017-05-23 15:18:05.000000000","message":"Well, the race is going to be only on shared resource provider consumption, and the chances of hitting that \"last slot of resources\" on a shared provider are, by nature, smaller than on a compute node which has fewer resources overall.\n\nBut, yes, your suggestion is noted and we\u0027ll discuss possibly merging the two functions into a single SQL call in a future patch (maybe yours!) :)","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"8ac61230e86c26fa88af7479ca2bfd135c0876c3","unresolved":false,"context_lines":[{"line_number":898,"context_line":"    #     inv_disk_gb.max_unit \u003e\u003d $AMOUNT_DISK_GB AND"},{"line_number":899,"context_line":"    #     $AMOUNT_DISK_GB % inv_disk_gb.step_size \u003d\u003d 0"},{"line_number":900,"context_line":"    #   ) OR"},{"line_number":901,"context_line":"    #     shared_disk_gb.aggregate_id IS NOT NULL"},{"line_number":902,"context_line":"    # )"},{"line_number":903,"context_line":"    # GROUP BY rp.id"},{"line_number":904,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"ff0f0b1f_8cea770c","line":901,"in_reply_to":"ff0f0b1f_debef5b2","updated":"2017-05-23 07:48:25.000000000","message":"The \u0027sharing_inv_disk_gb\u0027 table is in the comment at line 870.\n\nEmm... I\u0027m not sure how to measure whether this race condition is small or not. This can reduce some races and reduce some retries. Especially when we pack the instance to the compute node. Bascially, it is similar with the current nova-scheduler\u0027s race condition.","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"e63ec4a28be98200fec5dca1e9a78f72a08b0719","unresolved":false,"context_lines":[{"line_number":900,"context_line":"    #   ) OR"},{"line_number":901,"context_line":"    #     shared_disk_gb.aggregate_id IS NOT NULL"},{"line_number":902,"context_line":"    # )"},{"line_number":903,"context_line":"    # GROUP BY rp.id"},{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    rpt \u003d sa.alias(_RP_TBL, name\u003d\"rp\")"},{"line_number":906,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_483fa9b3","line":903,"updated":"2017-05-12 08:20:22.000000000","message":"Don\u0027t we want to add some limits here, for thousands of RPs and simple resource we can get all RPs?","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"97ccae8c08034bfbccad83e801f6d7105d6ebb74","unresolved":false,"context_lines":[{"line_number":900,"context_line":"    #   ) OR"},{"line_number":901,"context_line":"    #     shared_disk_gb.aggregate_id IS NOT NULL"},{"line_number":902,"context_line":"    # )"},{"line_number":903,"context_line":"    # GROUP BY rp.id"},{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    rpt \u003d sa.alias(_RP_TBL, name\u003d\"rp\")"},{"line_number":906,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"ff0f0b1f_1eeeadb3","line":903,"in_reply_to":"3f044301_483fa9b3","updated":"2017-05-22 13:17:17.000000000","message":"Meh, maybe in the future. For right now, I\u0027m not all that concerned. I\u0027ve benchmarked this schema with 10K compute nodes and hundreds of shared disk providers and the query is very quick, even returning thousands of providers at a time. The existing behaviour of the scheduler is to not get a limited number of providers, so I planned on keeping that behaviour at least for the time being.","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"7d67161766a588a45778e48b75c4931ee47b786e","unresolved":false,"context_lines":[{"line_number":905,"context_line":"    rpt \u003d sa.alias(_RP_TBL, name\u003d\"rp\")"},{"line_number":906,"context_line":""},{"line_number":907,"context_line":"    # Contains a set of resource provider IDs for each resource class requested"},{"line_number":908,"context_line":"    sharing_providers \u003d {"},{"line_number":909,"context_line":"        rc_id: _get_providers_with_shared_capacity(ctx, rc_id, amount)"},{"line_number":910,"context_line":"        for rc_id, amount in resources.items()"},{"line_number":911,"context_line":"    }"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"    # Short-circuit. If there\u0027s no providers sharing resources we need, no"},{"line_number":914,"context_line":"    # point going forward"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_ba95c1e6","line":911,"range":{"start_line":908,"start_character":0,"end_line":911,"end_character":5},"updated":"2017-05-05 16:01:45.000000000","message":"so, you\u0027re making a DB call for every RC. Why couldn\u0027t we pass a list of RC ids and return RPs that have at least one RC being shared ? That would only make one DB call","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b5dec8cba84b914053f226a787ae595e98f788ef","unresolved":false,"context_lines":[{"line_number":905,"context_line":"    rpt \u003d sa.alias(_RP_TBL, name\u003d\"rp\")"},{"line_number":906,"context_line":""},{"line_number":907,"context_line":"    # Contains a set of resource provider IDs for each resource class requested"},{"line_number":908,"context_line":"    sharing_providers \u003d {"},{"line_number":909,"context_line":"        rc_id: _get_providers_with_shared_capacity(ctx, rc_id, amount)"},{"line_number":910,"context_line":"        for rc_id, amount in resources.items()"},{"line_number":911,"context_line":"    }"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"    # Short-circuit. If there\u0027s no providers sharing resources we need, no"},{"line_number":914,"context_line":"    # point going forward"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_5f81d98e","line":911,"range":{"start_line":908,"start_character":0,"end_line":911,"end_character":5},"in_reply_to":"3f044301_ba95c1e6","updated":"2017-05-09 14:33:54.000000000","message":"Because the sets of resource providers that have shared resources are different. The way the query works looks for providers having some local inventory of a resource class OR are associated with an aggregate that contains a provider of shared resources of that resource class.\n\nThe WHERE clause contains an OR condition (see lines 889-901 above for the SQL and lines 1028-1035 below for the Python) which is specific to the set of shared providers *for a particular resource class*. That\u0027s why we need to get that list for each resource class here.","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"7d67161766a588a45778e48b75c4931ee47b786e","unresolved":false,"context_lines":[{"line_number":913,"context_line":"    # Short-circuit. If there\u0027s no providers sharing resources we need, no"},{"line_number":914,"context_line":"    # point going forward"},{"line_number":915,"context_line":"    if all(len(sp) \u003d\u003d 0 for sp in sharing_providers.values()):"},{"line_number":916,"context_line":"        return []"},{"line_number":917,"context_line":""},{"line_number":918,"context_line":"    name_map \u003d {"},{"line_number":919,"context_line":"        rc_id: _RC_CACHE.string_from_id(rc_id).lower()"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_daa64d54","line":916,"updated":"2017-05-05 16:01:45.000000000","message":"I\u0027m confused. That would return no RPs if none of them are sharing a RC, but I thought this method would also return RPs having those resources \"locally\"?","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b5dec8cba84b914053f226a787ae595e98f788ef","unresolved":false,"context_lines":[{"line_number":913,"context_line":"    # Short-circuit. If there\u0027s no providers sharing resources we need, no"},{"line_number":914,"context_line":"    # point going forward"},{"line_number":915,"context_line":"    if all(len(sp) \u003d\u003d 0 for sp in sharing_providers.values()):"},{"line_number":916,"context_line":"        return []"},{"line_number":917,"context_line":""},{"line_number":918,"context_line":"    name_map \u003d {"},{"line_number":919,"context_line":"        rc_id: _RC_CACHE.string_from_id(rc_id).lower()"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_bf3e9dd7","line":916,"in_reply_to":"3f044301_daa64d54","updated":"2017-05-09 14:33:54.000000000","message":"Yeah, this is leftover from a previous revision where I was using the original _get_all_by_filters() implementation and only calling this if the original \"get me providers that have ALL these resources\" failed. I\u0027ll remove the above short-circuit now.","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"309913359173a626a81dbdc04ff2e08d1a59634a","unresolved":false,"context_lines":[{"line_number":913,"context_line":"    # Short-circuit. If there\u0027s no providers sharing resources we need, no"},{"line_number":914,"context_line":"    # point going forward"},{"line_number":915,"context_line":"    if all(len(sp) \u003d\u003d 0 for sp in sharing_providers.values()):"},{"line_number":916,"context_line":"        return []"},{"line_number":917,"context_line":""},{"line_number":918,"context_line":"    name_map \u003d {"},{"line_number":919,"context_line":"        rc_id: _RC_CACHE.string_from_id(rc_id).lower()"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_70784a29","line":916,"in_reply_to":"3f044301_daa64d54","updated":"2017-05-09 09:49:55.000000000","message":"yea, I have same question.","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"0e22d2a5e4c5ba69580f8217d21aa0fa7fbc581a","unresolved":false,"context_lines":[{"line_number":868,"context_line":"    # LEFT JOIN resource_provider_aggregates AS sharing_disk_gb"},{"line_number":869,"context_line":"    #  ON shared_disk_gb.aggregate_id \u003d sharing_disk_gb.aggregate_id"},{"line_number":870,"context_line":"    # AND sharing_disk_gb.resource_provider_id IN ($RPS_SHARING_DISK)"},{"line_number":871,"context_line":"    # WHERE ("},{"line_number":872,"context_line":"    #   ("},{"line_number":873,"context_line":"    #     COALESCE(usage_vcpu.used, 0) + $AMOUNT_VCPU \u003c\u003d"},{"line_number":874,"context_line":"    #     (inv_vcpu.total + inv_vcpu.reserved)"},{"line_number":875,"context_line":"    #     * inv_vcpu.allocation_ratio"},{"line_number":876,"context_line":"    #   ) AND"},{"line_number":877,"context_line":"    #   inv_vcpu.min_unit \u003c\u003d $AMOUNT_VCPU AND"},{"line_number":878,"context_line":"    #   inv_vcpu.max_unit \u003e\u003d $AMOUNT_VCPU AND"},{"line_number":879,"context_line":"    #   $AMOUNT_VCPU % inv_vcpu.step_size \u003d\u003d 0"},{"line_number":880,"context_line":"    # ) AND ("},{"line_number":881,"context_line":"    #   ("},{"line_number":882,"context_line":"    #     COALESCE(usage_memory_mb.used, 0) + $AMOUNT_VCPU \u003c\u003d"},{"line_number":883,"context_line":"    #     (inv_memory_mb.total + inv_memory_mb.reserved)"},{"line_number":884,"context_line":"    #     * inv_memory_mb.allocation_ratio"},{"line_number":885,"context_line":"    #   ) AND"},{"line_number":886,"context_line":"    #   inv_memory_mb.min_unit \u003c\u003d $AMOUNT_MEMORY_MB AND"},{"line_number":887,"context_line":"    #   inv_memory_mb.max_unit \u003e\u003d $AMOUNT_MEMORY_MB AND"},{"line_number":888,"context_line":"    #   $AMOUNT_MEMORY_MB % inv_memory_mb.step_size \u003d\u003d 0"},{"line_number":889,"context_line":"    # ) AND ("},{"line_number":890,"context_line":"    #   inv_disk.resource_provider_id IS NOT NULL AND"},{"line_number":891,"context_line":"    #   ("},{"line_number":892,"context_line":"    #     ("},{"line_number":893,"context_line":"    #       COALESCE(usage_disk_gb.used, 0) + $AMOUNT_DISK_GB \u003c\u003d"},{"line_number":894,"context_line":"    #         (inv_disk_gb.total + inv_disk_gb.reserved)"},{"line_number":895,"context_line":"    #         * inv_disk_gb.allocation_ratio"},{"line_number":896,"context_line":"    #     ) AND"},{"line_number":897,"context_line":"    #     inv_disk_gb.min_unit \u003c\u003d $AMOUNT_DISK_GB AND"},{"line_number":898,"context_line":"    #     inv_disk_gb.max_unit \u003e\u003d $AMOUNT_DISK_GB AND"},{"line_number":899,"context_line":"    #     $AMOUNT_DISK_GB % inv_disk_gb.step_size \u003d\u003d 0"},{"line_number":900,"context_line":"    #   ) OR"},{"line_number":901,"context_line":"    #     shared_disk_gb.aggregate_id IS NOT NULL"},{"line_number":902,"context_line":"    # )"},{"line_number":903,"context_line":"    # GROUP BY rp.id"},{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    rpt \u003d sa.alias(_RP_TBL, name\u003d\"rp\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"ff0f0b1f_1ea21ffe","line":902,"range":{"start_line":871,"start_character":0,"end_line":902,"end_character":7},"updated":"2017-05-22 19:33:49.000000000","message":"Okay, given you\u0027re grouping only by RP UUIDs, then we only have one row by each of them, so the AND condition for checking each of the resources is working fine.\n\nCool with me.","commit_id":"951493a6e16c1e44f0467cfa5ad407aedf8223e1"}],"nova/tests/functional/db/test_resource_provider.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"fb973434079523f5b6066c48ccdadac11b7d05e5","unresolved":false,"context_lines":[{"line_number":1841,"context_line":"        with an aggregate. We then call the"},{"line_number":1842,"context_line":"        ResourceProviderList._get_all_with_shared() function to ensure that we"},{"line_number":1843,"context_line":"        get back the two compute node resource provider records."},{"line_number":1844,"context_line":"        \"\"\""},{"line_number":1845,"context_line":"        # The aggregate that will be associated to everything..."},{"line_number":1846,"context_line":"        agg_uuid \u003d uuidsentinel.agg"},{"line_number":1847,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_2612fc0a","line":1844,"updated":"2017-05-02 12:42:47.000000000","message":"Should there also be some cases where we set up some compute nodes that\n\n* are not in any aggregate\n* are in a different aggregate that has a shared provider that doesn\u0027t have enough inventory\n* are in the aggregate but don\u0027t have inventory\n\nI\u0027m sure there are a few cases and we probably can\u0027t cover all of the, but the SQL is bewildering enough that it would be great to have the nooks and crannies explored more completely; back at the end of newton we had issues with allocation handling because the test were only the happy path.\n\n(Perhaps you already plan to do this, in which case: awesome.)","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd2049c11d3fffbfd924e0e849fa2a0f2710010e","unresolved":false,"context_lines":[{"line_number":1841,"context_line":"        with an aggregate. We then call the"},{"line_number":1842,"context_line":"        ResourceProviderList._get_all_with_shared() function to ensure that we"},{"line_number":1843,"context_line":"        get back the two compute node resource provider records."},{"line_number":1844,"context_line":"        \"\"\""},{"line_number":1845,"context_line":"        # The aggregate that will be associated to everything..."},{"line_number":1846,"context_line":"        agg_uuid \u003d uuidsentinel.agg"},{"line_number":1847,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_b229a69c","line":1844,"in_reply_to":"5ff73747_2612fc0a","updated":"2017-05-02 14:39:45.000000000","message":"yes, totes. will add more detailed functional tests in the next patch. In this patch, I\u0027m going to remove the change that drops _get_all_by_filters() to _get_all_with_shared() if there are no results and just test _get_all_with_shared() separately like I did with _get_providers_with_shared_capacity() in the previous patch to this.\n\nI\u0027ll then add more functional tests for _get_all_with_shared() and finally will integrate it into the _get_all_by_filters() method.","commit_id":"d7f5529d3edcc022bcb509ea5108f1baf360e3b0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9ca45169362aebaef689b3b947ea64c83371fbfa","unresolved":false,"context_lines":[{"line_number":1952,"context_line":"        ss.set_aggregates([agg_uuid])"},{"line_number":1953,"context_line":""},{"line_number":1954,"context_line":"        # OK, now that has all been set up, let\u0027s verify that we get the ID of"},{"line_number":1955,"context_line":"        # the shared storage pool when we ask for 10 DISK_GB"},{"line_number":1956,"context_line":"        got_rps \u003d rp_obj._get_all_with_shared("},{"line_number":1957,"context_line":"            self.context,"},{"line_number":1958,"context_line":"            resources,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_e475159a","line":1955,"range":{"start_line":1955,"start_character":50,"end_line":1955,"end_character":52},"updated":"2017-05-04 14:30:12.000000000","message":"Again, you\u0027re asking for 100G not 10. I think it would be good to put \u0027resources\u0027 down here so it\u0027s clearer what we\u0027re asking for.\n\nAlso, I\u0027m not entirely sure how this is different from the test you added in the last patch. Seems almost identical?","commit_id":"5ebdddf5f08bf8ef522e4a26fd355fbbd6e60456"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"53f7567c1ab4bed261932b2e64e1a100d253f42c","unresolved":false,"context_lines":[{"line_number":1952,"context_line":"        ss.set_aggregates([agg_uuid])"},{"line_number":1953,"context_line":""},{"line_number":1954,"context_line":"        # OK, now that has all been set up, let\u0027s verify that we get the ID of"},{"line_number":1955,"context_line":"        # the shared storage pool when we ask for 10 DISK_GB"},{"line_number":1956,"context_line":"        got_rps \u003d rp_obj._get_all_with_shared("},{"line_number":1957,"context_line":"            self.context,"},{"line_number":1958,"context_line":"            resources,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_24765d08","line":1955,"range":{"start_line":1955,"start_character":50,"end_line":1955,"end_character":52},"in_reply_to":"3f044301_e475159a","updated":"2017-05-04 14:44:00.000000000","message":"Fixed the 100 vs 10 thing.\n\nThis test is different in that it adds in the aggregate association. Chris complained when I had the set_aggregates() stuff in the last patch\u0027s test when the code didn\u0027t actually touch the aggregates table. So this patch, since it *does* touch the aggregates table, adds in the set_aggregates() check.\n\nFollowing patches add more scenarios to this test...","commit_id":"5ebdddf5f08bf8ef522e4a26fd355fbbd6e60456"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"e63ec4a28be98200fec5dca1e9a78f72a08b0719","unresolved":false,"context_lines":[{"line_number":1757,"context_line":"        resources \u003d {"},{"line_number":1758,"context_line":"            VCPU_ID: 1,"},{"line_number":1759,"context_line":"            MEMORY_MB_ID: 64,"},{"line_number":1760,"context_line":"            DISK_GB_ID: 100,"},{"line_number":1761,"context_line":"        }"},{"line_number":1762,"context_line":"        return resources"},{"line_number":1763,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_c8d8597c","line":1760,"updated":"2017-05-12 08:20:22.000000000","message":"64Mb and 100Gb does such configuration can exist? )","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"e63ec4a28be98200fec5dca1e9a78f72a08b0719","unresolved":false,"context_lines":[{"line_number":1922,"context_line":"        disk_gb \u003d objects.Inventory("},{"line_number":1923,"context_line":"            resource_provider\u003dss,"},{"line_number":1924,"context_line":"            resource_class\u003dfields.ResourceClass.DISK_GB,"},{"line_number":1925,"context_line":"            total\u003d2000,"},{"line_number":1926,"context_line":"            reserved\u003d0,"},{"line_number":1927,"context_line":"            min_unit\u003d10,"},{"line_number":1928,"context_line":"            max_unit\u003d100,"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_88801148","line":1925,"updated":"2017-05-12 08:20:22.000000000","message":"What if we have total\u003d100 that it still returns two RPs but the only one can be served, is it right?","commit_id":"828f060996ebce9894bee4379db9ae83046f4a3d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cfff5230617b437eb7e58b7b3d10dd48e256b2f7","unresolved":false,"context_lines":[{"line_number":1958,"context_line":"            resources,"},{"line_number":1959,"context_line":"        )"},{"line_number":1960,"context_line":"        got_ids \u003d [rp.id for rp in got_rps]"},{"line_number":1961,"context_line":"        self.assertEqual([], got_ids)"},{"line_number":1962,"context_line":""},{"line_number":1963,"context_line":"        # Now associate the shared storage pool and both compute nodes with the"},{"line_number":1964,"context_line":"        # same aggregate"}],"source_content_type":"text/x-python","patch_set":10,"id":"1f013ff3_1ce27102","line":1961,"updated":"2017-05-16 14:09:09.000000000","message":"At first I was thinking you were missing tests for the non-shared conditionals in your query, but I think this is actually it. Question, though:\n\nI would assume that we can have RPs with disk that are also in an aggregate with shared disk, yes? Shouldn\u0027t we give one of the CNs some disk inventory and make sure that it\u0027s returned here? Otherwise we\u0027re not really sure if that part of the query is broken I think. The point is to move to using this query method for everything right?","commit_id":"d001582a46a5cb58767f3187a6a2979b93b39322"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"65d77547e7d9e38314e564653af64bd6b4739744","unresolved":false,"context_lines":[{"line_number":1958,"context_line":"            resources,"},{"line_number":1959,"context_line":"        )"},{"line_number":1960,"context_line":"        got_ids \u003d [rp.id for rp in got_rps]"},{"line_number":1961,"context_line":"        self.assertEqual([], got_ids)"},{"line_number":1962,"context_line":""},{"line_number":1963,"context_line":"        # Now associate the shared storage pool and both compute nodes with the"},{"line_number":1964,"context_line":"        # same aggregate"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff0f0b1f_7edc89e3","line":1961,"in_reply_to":"1f013ff3_1ce27102","updated":"2017-05-22 13:21:54.000000000","message":"I will add a test for that scenario on the end of this patch series, no problem Dan.","commit_id":"d001582a46a5cb58767f3187a6a2979b93b39322"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d90f2c42f7cba3b63985120bff981ff1cba809e1","unresolved":false,"context_lines":[{"line_number":1973,"context_line":"            resources,"},{"line_number":1974,"context_line":"        )"},{"line_number":1975,"context_line":"        got_ids \u003d [rp.id for rp in got_rps]"},{"line_number":1976,"context_line":"        self.assertEqual([cn1.id, cn2.id], got_ids)"}],"source_content_type":"text/x-python","patch_set":13,"id":"ff0f0b1f_e2d53a1d","line":1976,"updated":"2017-05-24 21:11:28.000000000","message":"Is the fact that the ordering of got_ids is the same as cn1 and cn2 reliable across whatever db backend and index structure might be involved here? Presumably cn1 and cn2\u0027s ids are in order because of the order they were created in, but maybe we should sort got_ids to be properly pedantic?\n\nIt probably doesn\u0027t matter but I saw it, so thought... eh","commit_id":"951493a6e16c1e44f0467cfa5ad407aedf8223e1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4a1c1d57cf35aba9ecb1f6d1e949db9dbba7ef58","unresolved":false,"context_lines":[{"line_number":1973,"context_line":"            resources,"},{"line_number":1974,"context_line":"        )"},{"line_number":1975,"context_line":"        got_ids \u003d [rp.id for rp in got_rps]"},{"line_number":1976,"context_line":"        self.assertEqual([cn1.id, cn2.id], got_ids)"}],"source_content_type":"text/x-python","patch_set":13,"id":"df140735_1de817b2","line":1976,"in_reply_to":"ff0f0b1f_e2d53a1d","updated":"2017-05-31 20:31:30.000000000","message":"Good note for when and if this test starts failing intermittently in the gate. :P","commit_id":"951493a6e16c1e44f0467cfa5ad407aedf8223e1"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"4040c64924b6cb2bec9bd0eec8feb8dd58b6c8d0","unresolved":false,"context_lines":[{"line_number":1973,"context_line":"            resources,"},{"line_number":1974,"context_line":"        )"},{"line_number":1975,"context_line":"        got_ids \u003d [rp.id for rp in got_rps]"},{"line_number":1976,"context_line":"        self.assertEqual([cn1.id, cn2.id], got_ids)"}],"source_content_type":"text/x-python","patch_set":13,"id":"ff0f0b1f_d448945d","line":1976,"in_reply_to":"ff0f0b1f_e2d53a1d","updated":"2017-05-25 18:06:24.000000000","message":"Yeah, I originally had the return as a set() for this exact reason but IIRC edleafe asked me to change it to a regular old list because, well, I don\u0027t remember why.","commit_id":"951493a6e16c1e44f0467cfa5ad407aedf8223e1"}]}