)]}'
{"ironic/conductor/manager.py":[{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"fd5a91c332135b3b860721385f7911d66f7e9090","unresolved":false,"context_lines":[{"line_number":561,"context_line":""},{"line_number":562,"context_line":"            try:"},{"line_number":563,"context_line":"                # prevent nodes in DEPLOYWAIT state from locking"},{"line_number":564,"context_line":"                if provision_state \u003d\u003d states.DEPLOYWAIT:"},{"line_number":565,"context_line":"                    continue"},{"line_number":566,"context_line":"                with task_manager.acquire(context, node_id) as task:"},{"line_number":567,"context_line":"                    self._do_sync_power_state(task)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAWH%2F9od8%3D","line":564,"updated":"2014-04-03 08:06:03.000000000","message":"There is race condition possible, node.provision_state can be changed after dbapi.get_nodeinfo_list because this loop takes some time if we have many nodes.","commit_id":"66a8a6339d8ac35d3157ff0a9b92f81bf4cbfac3"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"ac9aac8835524fab69ddcfa228c74adbb71008ac","unresolved":false,"context_lines":[{"line_number":561,"context_line":""},{"line_number":562,"context_line":"            try:"},{"line_number":563,"context_line":"                # prevent nodes in DEPLOYWAIT state from locking"},{"line_number":564,"context_line":"                if provision_state \u003d\u003d states.DEPLOYWAIT:"},{"line_number":565,"context_line":"                    continue"},{"line_number":566,"context_line":"                with task_manager.acquire(context, node_id) as task:"},{"line_number":567,"context_line":"                    self._do_sync_power_state(task)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAW3%2F%2F35g%3D","line":564,"in_reply_to":"AAAAWH%2F9ass%3D","updated":"2014-04-09 19:07:18.000000000","message":"Having the deploy ramdisk retry will certainly be possible within the ironic-python-agent project. In the current implementation... we could use a for loop or something equally brutish, but that doesn\u0027t really solve the problem. It just reduces the likelyhood of a failure since multiple attempted callbacks would have to all fail.\n\nI agree that the race condition previously existed, but this is making the window significantly longer. Instead of O(one db query), it\u0027s O(length of periodic task).\n\nGiven the potential duration of this periodic task, I think we need to refresh the node before proceeding. And given that, it\u0027s probably reasonable to just remove all the filters early on, fetch all the node\u0027s that map to this conductor, and cycle through them, with a greenthread yield between each.\n\nEg,\n\n  for ...:\n    node \u003d self.dbapi.get_node(node_uuid)\n    if node.provision_state not in [states.ACTIVE, states.NONE]:\n      continue\n    if node.maintenance:\n      continue\n    with acquire...:\n      do_sync_power_state\n\n    sleep(0)\n\nThis will avoid the current race entirely because the state before a transition to WAITCALLBACK is neither of those states.\n\nFurthermore, I think we only want to be forcing the power state of a node that is unassociated (states.NONE) or associated (states.ACTIVE) but nothing in between (all the prepare/teardown stuff).","commit_id":"66a8a6339d8ac35d3157ff0a9b92f81bf4cbfac3"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"14c3b26d28c7badd858a7155c5c043f1796fb1a0","unresolved":false,"context_lines":[{"line_number":561,"context_line":""},{"line_number":562,"context_line":"            try:"},{"line_number":563,"context_line":"                # prevent nodes in DEPLOYWAIT state from locking"},{"line_number":564,"context_line":"                if provision_state \u003d\u003d states.DEPLOYWAIT:"},{"line_number":565,"context_line":"                    continue"},{"line_number":566,"context_line":"                with task_manager.acquire(context, node_id) as task:"},{"line_number":567,"context_line":"                    self._do_sync_power_state(task)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAWH%2F9ass%3D","line":564,"in_reply_to":"AAAAWH%2F9n1o%3D","updated":"2014-04-04 16:46:26.000000000","message":"Ok.. seems like we need some additional synchronization then or deploy to retry if locked. I realize this increases the potential for the race.. but I\u0027m just arguing the race existed before.","commit_id":"66a8a6339d8ac35d3157ff0a9b92f81bf4cbfac3"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"f3c62e848e34fa8c9c24f0dd27147de25c27cb65","unresolved":false,"context_lines":[{"line_number":561,"context_line":""},{"line_number":562,"context_line":"            try:"},{"line_number":563,"context_line":"                # prevent nodes in DEPLOYWAIT state from locking"},{"line_number":564,"context_line":"                if provision_state \u003d\u003d states.DEPLOYWAIT:"},{"line_number":565,"context_line":"                    continue"},{"line_number":566,"context_line":"                with task_manager.acquire(context, node_id) as task:"},{"line_number":567,"context_line":"                    self._do_sync_power_state(task)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAWH%2F9n1o%3D","line":564,"in_reply_to":"AAAAWH%2F9oLM%3D","updated":"2014-04-03 09:40:22.000000000","message":"\u003eif it wasn\u0027t DEPLOYWAIT here and it happens to change to DEPLOYWAIT, we\u0027ll get  a lock and try to sync the power state. I\u0027m not sure if this is a problem or not\n\nThis is a problem because deploy will be failed if node locked and callback will come via vendor_passthru.","commit_id":"66a8a6339d8ac35d3157ff0a9b92f81bf4cbfac3"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"f16cea748cf16c8c30736d6f133b784257c778ee","unresolved":false,"context_lines":[{"line_number":561,"context_line":""},{"line_number":562,"context_line":"            try:"},{"line_number":563,"context_line":"                # prevent nodes in DEPLOYWAIT state from locking"},{"line_number":564,"context_line":"                if provision_state \u003d\u003d states.DEPLOYWAIT:"},{"line_number":565,"context_line":"                    continue"},{"line_number":566,"context_line":"                with task_manager.acquire(context, node_id) as task:"},{"line_number":567,"context_line":"                    self._do_sync_power_state(task)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAWH%2F9oLM%3D","line":564,"in_reply_to":"AAAAWH%2F9od8%3D","updated":"2014-04-03 08:55:11.000000000","message":"There was a race before, as it can always flip states immediately after the DB query. But, sure, it can be a larger window now. If the state happens to no longer really be DEPLOYWAIT, it\u0027s no big deal, because it\u0027ll get checked on the next run. However, if it wasn\u0027t DEPLOYWAIT here and it happens to change to DEPLOYWAIT, we\u0027ll get a lock and try to sync the power state. I\u0027m not sure if this is a problem or not. But if it is, there should be a different fix, like maybe checking the state after we acquire the lock.\n\nThis actually points out a separate issue here. We probably should yield the greenthread every so often in here so this periodic task doesn\u0027t starve other work. Of course, that\u0027ll make the window even larger. :)","commit_id":"66a8a6339d8ac35d3157ff0a9b92f81bf4cbfac3"},{"author":{"_account_id":10626,"name":"Russell Haering","email":"russellhaering@gmail.com","username":"russell_h"},"change_message_id":"e5906cb115e8d8c1967bce6ecd6a5150638f3964","unresolved":false,"context_lines":[{"line_number":585,"context_line":"                return"},{"line_number":586,"context_line":"            if not self._mapped_to_this_conductor(node.uuid, node.driver):"},{"line_number":587,"context_line":"                return"},{"line_number":588,"context_line":"            with task_manager.acquire(context, node.id) as task:"},{"line_number":589,"context_line":"                self._do_sync_power_state(task)"},{"line_number":590,"context_line":""},{"line_number":591,"context_line":"        # NOTE(comstud): Just grab the ids for all of the nodes to"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAW3%2F%2FYr4%3D","line":588,"updated":"2014-04-15 00:57:50.000000000","message":"If you checked for maintenance, reservation or DEPLOYWAIT right before calling self._do_sync_power_state(task) you could kill that FIXME. Mappings can change even while you have the lock so there\u0027s no point worrying about that.","commit_id":"028b914873add7a4990e532cee29f48e9ee04a15"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"71546603ba6c00747bbfdf4731724bbd1f9075b1","unresolved":false,"context_lines":[{"line_number":585,"context_line":"                return"},{"line_number":586,"context_line":"            if not self._mapped_to_this_conductor(node.uuid, node.driver):"},{"line_number":587,"context_line":"                return"},{"line_number":588,"context_line":"            with task_manager.acquire(context, node.id) as task:"},{"line_number":589,"context_line":"                self._do_sync_power_state(task)"},{"line_number":590,"context_line":""},{"line_number":591,"context_line":"        # NOTE(comstud): Just grab the ids for all of the nodes to"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAW3%2F%2FYHI%3D","line":588,"in_reply_to":"AAAAW3%2F%2FYr4%3D","updated":"2014-04-15 02:11:42.000000000","message":"The problem is that we don\u0027t want to even lock for DEPLOYWAIT... so what I really need is a way to specify some constraints with acquire().  I plan on addressing this in the future.\n\nI\u0027m not sure about the maintenance check. Probably no harm in locking for it... so I can just move the check to here.","commit_id":"028b914873add7a4990e532cee29f48e9ee04a15"},{"author":{"_account_id":10626,"name":"Russell Haering","email":"russellhaering@gmail.com","username":"russell_h"},"change_message_id":"e5906cb115e8d8c1967bce6ecd6a5150638f3964","unresolved":false,"context_lines":[{"line_number":597,"context_line":"        #                need to yield so that we\u0027re not stealing too"},{"line_number":598,"context_line":"        #                much time from other greenthreads."},{"line_number":599,"context_line":"        for (node_id, node_uuid) in self.dbapi.get_nodeinfo_list("},{"line_number":600,"context_line":"                columns\u003d[\u0027id\u0027, \u0027uuid\u0027]):"},{"line_number":601,"context_line":"            try:"},{"line_number":602,"context_line":"                node \u003d self.dbapi.get_node(node_id)"},{"line_number":603,"context_line":"                _sync_power_state()"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAW3%2F%2FYbs%3D","line":600,"updated":"2014-04-15 00:57:50.000000000","message":"If you checked if the node is mapped to this conductor right here, and skipped it inside _sync_power_state, you could skip a lot of reads.","commit_id":"028b914873add7a4990e532cee29f48e9ee04a15"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"71546603ba6c00747bbfdf4731724bbd1f9075b1","unresolved":false,"context_lines":[{"line_number":597,"context_line":"        #                need to yield so that we\u0027re not stealing too"},{"line_number":598,"context_line":"        #                much time from other greenthreads."},{"line_number":599,"context_line":"        for (node_id, node_uuid) in self.dbapi.get_nodeinfo_list("},{"line_number":600,"context_line":"                columns\u003d[\u0027id\u0027, \u0027uuid\u0027]):"},{"line_number":601,"context_line":"            try:"},{"line_number":602,"context_line":"                node \u003d self.dbapi.get_node(node_id)"},{"line_number":603,"context_line":"                _sync_power_state()"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAW3%2F%2FYIY%3D","line":600,"in_reply_to":"AAAAW3%2F%2FYbs%3D","updated":"2014-04-15 02:11:42.000000000","message":"Yeah, I think it should be moved back...","commit_id":"028b914873add7a4990e532cee29f48e9ee04a15"}]}