)]}'
{"cinder/volume/manager.py":[{"author":{"_account_id":170,"name":"Mike Perez","email":"thingee@gmail.com","username":"thingee"},"change_message_id":"d08cff560fa97862791955fb9612c82197db0e8e","unresolved":false,"context_lines":[{"line_number":1647,"context_line":"                except Exception:"},{"line_number":1648,"context_line":"                    LOG.exception(_LE(\"Error checking replication status for \""},{"line_number":1649,"context_line":"                                      \"volume %s\") % vol[\u0027id\u0027])"},{"line_number":1650,"context_line":"        else:"},{"line_number":1651,"context_line":"            LOG.debug(\"Replication is not enabled for driver %(driver)s\","},{"line_number":1652,"context_line":"                      {\u0027driver\u0027: self.driver.__class__.__name__})"},{"line_number":1653,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"da86d52c_bb4e7a6d","line":1650,"updated":"2015-02-11 02:13:23.000000000","message":"I don\u0027t think I need to see this in the logs everytime it runs.","commit_id":"0d4e704041aa3f07ef80b7b4689fee94568c66fc"},{"author":{"_account_id":10559,"name":"Steven Kaufer","email":"kaufer@us.ibm.com","username":"kaufer"},"change_message_id":"0f4a142d114de0b807c9fefcbe1d1c410c174c41","unresolved":false,"context_lines":[{"line_number":1647,"context_line":"                except Exception:"},{"line_number":1648,"context_line":"                    LOG.exception(_LE(\"Error checking replication status for \""},{"line_number":1649,"context_line":"                                      \"volume %s\") % vol[\u0027id\u0027])"},{"line_number":1650,"context_line":"        else:"},{"line_number":1651,"context_line":"            LOG.debug(\"Replication is not enabled for driver %(driver)s\","},{"line_number":1652,"context_line":"                      {\u0027driver\u0027: self.driver.__class__.__name__})"},{"line_number":1653,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"da86d52c_d6b785d6","line":1650,"in_reply_to":"da86d52c_bb4e7a6d","updated":"2015-02-11 03:19:49.000000000","message":"That\u0027s fair, I will remove the else block and the log message.","commit_id":"0d4e704041aa3f07ef80b7b4689fee94568c66fc"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"4a4929ddb10919af634f6f9d304d00ed2e0a8b84","unresolved":false,"context_lines":[{"line_number":1614,"context_line":"                       {\u0027model\u0027: model_update})"},{"line_number":1615,"context_line":"            raise exception.ReplicationError(reason\u003derr_msg,"},{"line_number":1616,"context_line":"                                             volume_id\u003dvolume_id)"},{"line_number":1617,"context_line":""},{"line_number":1618,"context_line":"    @periodic_task.periodic_task"},{"line_number":1619,"context_line":"    def _update_replication_relationship_status(self, ctxt):"},{"line_number":1620,"context_line":"        LOG.info(_LI(\u0027Updating volume replication status.\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"da86d52c_a697e338","line":1617,"updated":"2015-02-12 19:26:27.000000000","message":"This whole task shouldn\u0027t even be setup unless the host needs it.\n\nCan we wrap this method in the check to see if the driver supports replication or not?  I don\u0027t see the point of a periodic task that runs every N seconds and just does a check which will always fail and then exit.","commit_id":"9473f5912de73cf43bcfbe68e63d7c18a8d76ab3"},{"author":{"_account_id":10559,"name":"Steven Kaufer","email":"kaufer@us.ibm.com","username":"kaufer"},"change_message_id":"d1c466fa0546368ab05835207ca45c7e785514a5","unresolved":false,"context_lines":[{"line_number":1614,"context_line":"                       {\u0027model\u0027: model_update})"},{"line_number":1615,"context_line":"            raise exception.ReplicationError(reason\u003derr_msg,"},{"line_number":1616,"context_line":"                                             volume_id\u003dvolume_id)"},{"line_number":1617,"context_line":""},{"line_number":1618,"context_line":"    @periodic_task.periodic_task"},{"line_number":1619,"context_line":"    def _update_replication_relationship_status(self, ctxt):"},{"line_number":1620,"context_line":"        LOG.info(_LI(\u0027Updating volume replication status.\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"da86d52c_d95ba8f4","line":1617,"in_reply_to":"da86d52c_95bf63cf","updated":"2015-02-13 20:47:16.000000000","message":"Seems like a newer version of the periodic task code is oslo supports dynamically adding a periodic task:\n\nhttps://github.com/openstack/oslo-incubator/blob/master/openstack/common/periodic_task.py#L192\n\nAdded in commit:\n\nhttps://review.openstack.org/#/c/147567/\n\nI have prototyped this and it appears that the following would work:\n\n1. Add something like this to the end of the \"init_host\" function:\n\n             # collect and publish service capabilities\n             self.publish_service_capabilities(ctxt)\n    +\n    +        # contionally run replication status task\n    +        if self.driver._stats.get(\u0027replication\u0027, False):\n    +\n    +            @periodic_task.periodic_task\n    +            def run_replication_task(self, ctxt):\n    +                self._update_replication_relationship_status(ctxt)\n    +\n    +            self.add_periodic_task(run_replication_task)\n    \n         def create_volume(self, context, volume_id, request_spec\u003dNone,\n\n2. Remove the decorator from the existing \"_update_replication_relationship_status\" function:\n\n    -    @periodic_task.periodic_task\n         def _update_replication_relationship_status(self, ctxt):\n             LOG.info(_LI(\u0027Updating volume replication status.\u0027))\n\nThoughts?","commit_id":"9473f5912de73cf43bcfbe68e63d7c18a8d76ab3"},{"author":{"_account_id":10559,"name":"Steven Kaufer","email":"kaufer@us.ibm.com","username":"kaufer"},"change_message_id":"339bebc95aaacb1db329b308d34beb8a37270bcc","unresolved":false,"context_lines":[{"line_number":1614,"context_line":"                       {\u0027model\u0027: model_update})"},{"line_number":1615,"context_line":"            raise exception.ReplicationError(reason\u003derr_msg,"},{"line_number":1616,"context_line":"                                             volume_id\u003dvolume_id)"},{"line_number":1617,"context_line":""},{"line_number":1618,"context_line":"    @periodic_task.periodic_task"},{"line_number":1619,"context_line":"    def _update_replication_relationship_status(self, ctxt):"},{"line_number":1620,"context_line":"        LOG.info(_LI(\u0027Updating volume replication status.\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"da86d52c_95bf63cf","line":1617,"in_reply_to":"da86d52c_a697e338","updated":"2015-02-12 21:35:58.000000000","message":"Walter, can you go into more details on how this would be implemented? We cannot do something like\n\n  @periodic_task.periodic_task(enabled\u003dself.driver._stats.get(\u0027replication\u0027, False))\n\nsince \"self\" is not available when the decorator is processed. I cannot find any examples of how to conditionally add a periodic task with this type of logic.\n\nAny recommendations?","commit_id":"9473f5912de73cf43bcfbe68e63d7c18a8d76ab3"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"34a0dc371fe3fe666dca8768ccbad0c4fc522e6f","unresolved":false,"context_lines":[{"line_number":1614,"context_line":"                       {\u0027model\u0027: model_update})"},{"line_number":1615,"context_line":"            raise exception.ReplicationError(reason\u003derr_msg,"},{"line_number":1616,"context_line":"                                             volume_id\u003dvolume_id)"},{"line_number":1617,"context_line":""},{"line_number":1618,"context_line":"    @periodic_task.periodic_task"},{"line_number":1619,"context_line":"    def _update_replication_relationship_status(self, ctxt):"},{"line_number":1620,"context_line":"        LOG.info(_LI(\u0027Updating volume replication status.\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"da86d52c_33d68fc4","line":1617,"in_reply_to":"da86d52c_d95ba8f4","updated":"2015-02-15 02:37:05.000000000","message":"I have pushed up a new review with the change referenced above:  https://review.openstack.org/156011","commit_id":"9473f5912de73cf43bcfbe68e63d7c18a8d76ab3"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"357764fe4159f5a88c11dfbd36f232c85db5ba32","unresolved":false,"context_lines":[{"line_number":1633,"context_line":"                         \u0027config_group\u0027: config_group})"},{"line_number":1634,"context_line":"            return"},{"line_number":1635,"context_line":""},{"line_number":1636,"context_line":"        if self.driver._stats.get(\u0027replication\u0027, False):"},{"line_number":1637,"context_line":"            volumes \u003d self.db.volume_get_all_by_host(ctxt, self.host)"},{"line_number":1638,"context_line":"            for vol in volumes:"},{"line_number":1639,"context_line":"                model_update \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"da86d52c_277189a8","line":1636,"updated":"2015-02-11 05:22:41.000000000","message":"Honestly this doesn\u0027t really improve much IMO, but what would be an improvement is if we did a query here on volumes that have replicaton_status set.  What do you think?","commit_id":"9473f5912de73cf43bcfbe68e63d7c18a8d76ab3"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"33e7526034b2676d1ce4a7e69a6a4bce04431f4c","unresolved":false,"context_lines":[{"line_number":1633,"context_line":"                         \u0027config_group\u0027: config_group})"},{"line_number":1634,"context_line":"            return"},{"line_number":1635,"context_line":""},{"line_number":1636,"context_line":"        if self.driver._stats.get(\u0027replication\u0027, False):"},{"line_number":1637,"context_line":"            volumes \u003d self.db.volume_get_all_by_host(ctxt, self.host)"},{"line_number":1638,"context_line":"            for vol in volumes:"},{"line_number":1639,"context_line":"                model_update \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"da86d52c_b20ac2cf","line":1636,"in_reply_to":"da86d52c_277189a8","updated":"2015-02-12 18:55:18.000000000","message":"To clarify a bit, yes; your idea of two phases to this is good, and adding the db query is an improvement.\n\nThe problem I have is still you\u0027re doing this check/update for EVERY volume every minute.  That doesn\u0027t scale very well.\n\nThe other question is *why*?  My impression earlier was that this was so you could update provider fields etc in the background, but as others pointed out we\u0027re not actually doing that anyway, so why do we even need/want this polling here?\n\nIf somebody wants the status of replication on a replicated Volume why don\u0027t we just give them a call to do so?\n\nAgain, given that IBM is the only driver that implements this, and has a very specific update object I\u0027m not sure why this is here and polling every volume on every backend that supports replication every minute?","commit_id":"9473f5912de73cf43bcfbe68e63d7c18a8d76ab3"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"434883b1bb8202a8fbfad75c2a2bd6c4e55f1bc2","unresolved":false,"context_lines":[{"line_number":347,"context_line":"        # collect and publish service capabilities"},{"line_number":348,"context_line":"        self.publish_service_capabilities(ctxt)"},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"        # contionally run replication status task"},{"line_number":351,"context_line":"        if self.driver._stats.get(\u0027replication\u0027, False):"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"            @periodic_task.periodic_task"}],"source_content_type":"text/x-python","patch_set":3,"id":"da86d52c_f36900f9","line":350,"updated":"2015-02-17 21:31:01.000000000","message":"contionally -\u003e conditionally?","commit_id":"27af1e7b389d179e58cc04dab905ad888fea3ea7"},{"author":{"_account_id":10559,"name":"Steven Kaufer","email":"kaufer@us.ibm.com","username":"kaufer"},"change_message_id":"215cdae1fd504222f1b4557406e364f9f190ab37","unresolved":false,"context_lines":[{"line_number":347,"context_line":"        # collect and publish service capabilities"},{"line_number":348,"context_line":"        self.publish_service_capabilities(ctxt)"},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"        # contionally run replication status task"},{"line_number":351,"context_line":"        if self.driver._stats.get(\u0027replication\u0027, False):"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"            @periodic_task.periodic_task"}],"source_content_type":"text/x-python","patch_set":3,"id":"da86d52c_0ee907b7","line":350,"in_reply_to":"da86d52c_f36900f9","updated":"2015-02-17 21:44:05.000000000","message":"Done","commit_id":"27af1e7b389d179e58cc04dab905ad888fea3ea7"},{"author":{"_account_id":10559,"name":"Steven Kaufer","email":"kaufer@us.ibm.com","username":"kaufer"},"change_message_id":"be21679a255cde1e1ebbc6968edec034564a35f6","unresolved":false,"context_lines":[{"line_number":345,"context_line":"                    self.delete_volume(ctxt, volume[\u0027id\u0027])"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"        # collect and publish service capabilities"},{"line_number":348,"context_line":"        self.publish_service_capabilities(ctxt)"},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"        # conditionally run replication status task"},{"line_number":351,"context_line":"        if self.driver._stats.get(\u0027replication\u0027, False):"}],"source_content_type":"text/x-python","patch_set":4,"id":"da86d52c_4d005b66","line":348,"updated":"2015-02-18 04:49:11.000000000","message":"John, so this function sparks the chain that sets the \u0027replication\u0027 value on the volume stats.  Here\u0027s the flow:\n\nThe \"publish_service_capabilities\" function here invokes \"report_driver_status\":\n\nhttps://github.com/openstack/cinder/blob/master/cinder/volume/manager.py#L1272\n\nwhich then gets the driver stats for the driver:\n\nhttps://github.com/openstack/cinder/blob/master/cinder/volume/manager.py#L1247\n\nAnd this implementation in the storwize driver eventually sets \u0027replication\u0027\u003dTrue\n\nhttps://github.com/openstack/cinder/blob/master/cinder/volume/drivers/ibm/storwize_svc/__init__.py#L1005\n\nhttps://github.com/openstack/cinder/blob/master/cinder/volume/drivers/ibm/storwize_svc/__init__.py#L1123\n\nhttps://github.com/openstack/cinder/blob/master/cinder/volume/drivers/ibm/storwize_svc/replication.py#L184\n\nIn summary, after the \"publish_service_capabilities\" function is invoked here then the _stats will be set on the driver.\n\nNow that I\u0027ve traced all this, I feel that this path is too fragile and I will update the check in line 350 to the following:\n\n        # conditionally run replication status task\n        stats \u003d self.driver.get_volume_stats()\n\tif stats and stats.get(\u0027replication\u0027, False):","commit_id":"f99c4740c559a74596c1130bbcab3d1c343b72ae"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"43b87d2e98d3fd204f9ece760f967220bcd2fcbe","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            def run_replication_task(self, ctxt):"},{"line_number":355,"context_line":"                self._update_replication_relationship_status(ctxt)"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"            self.add_periodic_task(run_replication_task)"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"    def create_volume(self, context, volume_id, request_spec\u003dNone,"},{"line_number":360,"context_line":"                      filter_properties\u003dNone, allow_reschedule\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":4,"id":"da86d52c_7b02420d","line":357,"updated":"2015-02-17 23:31:03.000000000","message":"So I\u0027ve never seen this used before, if I follow correctly it will make the run_replication_task IFF it get\u0027s replication not False in the stats?  Pretty clever.","commit_id":"f99c4740c559a74596c1130bbcab3d1c343b72ae"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"3130dc15fb3ec7f2451bcee235315d12d8a8141b","unresolved":false,"context_lines":[{"line_number":1626,"context_line":""},{"line_number":1627,"context_line":"    def _update_replication_relationship_status(self, ctxt):"},{"line_number":1628,"context_line":"        LOG.info(_LI(\u0027Updating volume replication status.\u0027))"},{"line_number":1629,"context_line":"        volumes \u003d self.db.volume_get_all_by_host(ctxt, self.host)"},{"line_number":1630,"context_line":"        for vol in volumes:"},{"line_number":1631,"context_line":"            model_update \u003d None"},{"line_number":1632,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"da86d52c_e74a1082","line":1629,"updated":"2015-02-18 02:49:50.000000000","message":"So, is there a way to do a sql call that only returns volumes that are replicated instead of fetching every volume for a host?   This seems like it could be pulling a boatload of data every 60 seconds from the database.","commit_id":"f99c4740c559a74596c1130bbcab3d1c343b72ae"},{"author":{"_account_id":10559,"name":"Steven Kaufer","email":"kaufer@us.ibm.com","username":"kaufer"},"change_message_id":"be21679a255cde1e1ebbc6968edec034564a35f6","unresolved":false,"context_lines":[{"line_number":1626,"context_line":""},{"line_number":1627,"context_line":"    def _update_replication_relationship_status(self, ctxt):"},{"line_number":1628,"context_line":"        LOG.info(_LI(\u0027Updating volume replication status.\u0027))"},{"line_number":1629,"context_line":"        volumes \u003d self.db.volume_get_all_by_host(ctxt, self.host)"},{"line_number":1630,"context_line":"        for vol in volumes:"},{"line_number":1631,"context_line":"            model_update \u003d None"},{"line_number":1632,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"da86d52c_2ab30d9b","line":1629,"in_reply_to":"da86d52c_e74a1082","updated":"2015-02-18 04:49:11.000000000","message":"This is not possible until this BP is approved [1] and the associated code is merged [2].\n\nI was hoping to address this bug in 2 phases: (1) the updates here (to only execute the periodic task for drivers that support replication) and then (2) push another change to use the new DB APIs once the filter support on the \u0027volume_get_all_by_host\u0027 function is available.\n\n[1]: https://review.openstack.org/#/c/155074/\n\n[2]: https://review.openstack.org/#/c/155343/","commit_id":"f99c4740c559a74596c1130bbcab3d1c343b72ae"}]}