)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"db56dd98c9e214ebcbdb6b89eaae151ef953a11c","unresolved":false,"context_lines":[{"line_number":13,"context_line":"using zookeeper for coordination"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Icf60381e30f3baf986cf9e008e133287765d9827"},{"line_number":16,"context_line":"Closes-Bug: #1558735"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"ba0121b8_937f72aa","line":16,"updated":"2016-03-31 09:01:30.000000000","message":"plz add a DocImpact tag such as:\nDocImpact: adds two new options for coordination","commit_id":"6efab202b56c5b7f8557a44f28002550a77d07bf"}],"ceilometer/coordination.py":[{"author":{"_account_id":17289,"name":"Su Zhengwei","email":"su.zhengwei@zte.com.cn","username":"su-zhengwei"},"change_message_id":"4a04b31451337a13d2fcab8c1cb90b57532f7ff8","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                 default\u003d10.0,"},{"line_number":41,"context_line":"                 help\u003d\u0027Number of seconds between checks to see if group \u0027"},{"line_number":42,"context_line":"                      \u0027membership has changed\u0027),"},{"line_number":43,"context_line":"    cfg.FloatOpt(\u0027retry_interval\u0027,"},{"line_number":44,"context_line":"                 default\u003d10.0,"},{"line_number":45,"context_line":"                 help\u003d\u0027Number of Seconds between retry to join partitioning \u0027"},{"line_number":46,"context_line":"                      \u0027group\u0027),"},{"line_number":47,"context_line":"    cfg.IntOpt(\u0027max_retries\u0027,"},{"line_number":48,"context_line":"               default\u003d30,"},{"line_number":49,"context_line":"               help\u003d\u0027Maximum number of retries before we give up to join \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"1af94dfe_717f37ab","line":46,"range":{"start_line":43,"start_character":0,"end_line":46,"end_character":31},"updated":"2016-03-18 01:58:18.000000000","message":"Here using \"cfg.IntOpt\" type will be better.","commit_id":"7d868b270effb564d8db06a8f0b6d2e999173478"},{"author":{"_account_id":11549,"name":"Atul Aggarwal","email":"atula@hp.com","username":"aaggarw3"},"change_message_id":"064e15aa01c9ab6deb19a26380de7f82d90df61f","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                 default\u003d10.0,"},{"line_number":41,"context_line":"                 help\u003d\u0027Number of seconds between checks to see if group \u0027"},{"line_number":42,"context_line":"                      \u0027membership has changed\u0027),"},{"line_number":43,"context_line":"    cfg.FloatOpt(\u0027retry_interval\u0027,"},{"line_number":44,"context_line":"                 default\u003d10.0,"},{"line_number":45,"context_line":"                 help\u003d\u0027Number of Seconds between retry to join partitioning \u0027"},{"line_number":46,"context_line":"                      \u0027group\u0027),"},{"line_number":47,"context_line":"    cfg.IntOpt(\u0027max_retries\u0027,"},{"line_number":48,"context_line":"               default\u003d30,"},{"line_number":49,"context_line":"               help\u003d\u0027Maximum number of retries before we give up to join \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"1af94dfe_75dc1e46","line":46,"range":{"start_line":43,"start_character":0,"end_line":46,"end_character":31},"in_reply_to":"1af94dfe_2439eaa4","updated":"2016-03-18 16:44:39.000000000","message":"Done","commit_id":"7d868b270effb564d8db06a8f0b6d2e999173478"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"24f06ac544cb1dd209835b21d6d4e5a9de3ba830","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                 default\u003d10.0,"},{"line_number":41,"context_line":"                 help\u003d\u0027Number of seconds between checks to see if group \u0027"},{"line_number":42,"context_line":"                      \u0027membership has changed\u0027),"},{"line_number":43,"context_line":"    cfg.FloatOpt(\u0027retry_interval\u0027,"},{"line_number":44,"context_line":"                 default\u003d10.0,"},{"line_number":45,"context_line":"                 help\u003d\u0027Number of Seconds between retry to join partitioning \u0027"},{"line_number":46,"context_line":"                      \u0027group\u0027),"},{"line_number":47,"context_line":"    cfg.IntOpt(\u0027max_retries\u0027,"},{"line_number":48,"context_line":"               default\u003d30,"},{"line_number":49,"context_line":"               help\u003d\u0027Maximum number of retries before we give up to join \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"1af94dfe_2439eaa4","line":46,"range":{"start_line":43,"start_character":0,"end_line":46,"end_character":31},"in_reply_to":"1af94dfe_717f37ab","updated":"2016-03-18 06:39:56.000000000","message":"agree, it is common to use int value for inerval","commit_id":"7d868b270effb564d8db06a8f0b6d2e999173478"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"c61523d7540bdab1ce1b8cc90a1e6c247407c515","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"class ErrorJoiningPartitioningGroup(Exception):"},{"line_number":56,"context_line":"    def __init__(self, group_id):"},{"line_number":57,"context_line":"        super(ErrorJoiningPartitioningGroup, self).__init__(_LE("},{"line_number":58,"context_line":"            \u0027Coordination join_group Error joining partitioning group\u0027))"},{"line_number":59,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1af94dfe_a7d05c20","line":56,"range":{"start_line":56,"start_character":23,"end_line":56,"end_character":31},"updated":"2016-03-18 06:48:58.000000000","message":"define a positional arg that never used?","commit_id":"7d868b270effb564d8db06a8f0b6d2e999173478"},{"author":{"_account_id":11549,"name":"Atul Aggarwal","email":"atula@hp.com","username":"aaggarw3"},"change_message_id":"064e15aa01c9ab6deb19a26380de7f82d90df61f","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"class ErrorJoiningPartitioningGroup(Exception):"},{"line_number":56,"context_line":"    def __init__(self, group_id):"},{"line_number":57,"context_line":"        super(ErrorJoiningPartitioningGroup, self).__init__(_LE("},{"line_number":58,"context_line":"            \u0027Coordination join_group Error joining partitioning group\u0027))"},{"line_number":59,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1af94dfe_b5cd0668","line":56,"range":{"start_line":56,"start_character":23,"end_line":56,"end_character":31},"in_reply_to":"1af94dfe_a7d05c20","updated":"2016-03-18 16:44:39.000000000","message":"When I started on this change, I thought it would be a good idea to output the group id. But if I use wrap_exception true then original exception does it anyways. So I will remove this positional arg.","commit_id":"7d868b270effb564d8db06a8f0b6d2e999173478"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"24f06ac544cb1dd209835b21d6d4e5a9de3ba830","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        if self._coordinator:"},{"line_number":142,"context_line":"            self._coordinator.run_watchers()"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    @retrying.retry(stop_max_attempt_number\u003dcfg.CONF.coordination.max_retries,"},{"line_number":145,"context_line":"                    wait_fixed\u003dcfg.CONF.coordination.retry_interval*1000,"},{"line_number":146,"context_line":"                    retry_on_exception\u003dretry_on_error_joining_partition,"},{"line_number":147,"context_line":"                    wrap_exception\u003dTrue)"},{"line_number":148,"context_line":"    def join_group(self, group_id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1af94dfe_04d78e4b","line":145,"range":{"start_line":144,"start_character":0,"end_line":145,"end_character":73},"updated":"2016-03-18 06:39:56.000000000","message":"use cfg.CONF option in definition time is danger, because it may always use the default value, you can add an unit test check it","commit_id":"7d868b270effb564d8db06a8f0b6d2e999173478"},{"author":{"_account_id":17289,"name":"Su Zhengwei","email":"su.zhengwei@zte.com.cn","username":"su-zhengwei"},"change_message_id":"af2d1abbec128e68fa3e74996328a5ade5d2b5cc","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        if self._coordinator:"},{"line_number":142,"context_line":"            self._coordinator.run_watchers()"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    @retrying.retry(stop_max_attempt_number\u003dcfg.CONF.coordination.max_retries,"},{"line_number":145,"context_line":"                    wait_fixed\u003dcfg.CONF.coordination.retry_interval*1000,"},{"line_number":146,"context_line":"                    retry_on_exception\u003dretry_on_error_joining_partition,"},{"line_number":147,"context_line":"                    wrap_exception\u003dTrue)"},{"line_number":148,"context_line":"    def join_group(self, group_id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1af94dfe_4a019d69","line":145,"range":{"start_line":144,"start_character":0,"end_line":145,"end_character":73},"in_reply_to":"1af94dfe_04d78e4b","updated":"2016-03-18 07:36:42.000000000","message":"I think the owner just want to define a global variable which  can be easy modified,but not open to users.","commit_id":"7d868b270effb564d8db06a8f0b6d2e999173478"},{"author":{"_account_id":11549,"name":"Atul Aggarwal","email":"atula@hp.com","username":"aaggarw3"},"change_message_id":"064e15aa01c9ab6deb19a26380de7f82d90df61f","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        if self._coordinator:"},{"line_number":142,"context_line":"            self._coordinator.run_watchers()"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    @retrying.retry(stop_max_attempt_number\u003dcfg.CONF.coordination.max_retries,"},{"line_number":145,"context_line":"                    wait_fixed\u003dcfg.CONF.coordination.retry_interval*1000,"},{"line_number":146,"context_line":"                    retry_on_exception\u003dretry_on_error_joining_partition,"},{"line_number":147,"context_line":"                    wrap_exception\u003dTrue)"},{"line_number":148,"context_line":"    def join_group(self, group_id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1af94dfe_78f25307","line":145,"range":{"start_line":144,"start_character":0,"end_line":145,"end_character":73},"in_reply_to":"1af94dfe_4a019d69","updated":"2016-03-18 16:44:39.000000000","message":"As ZTE-Zhengwei Su said, my initial thought is to have a global variables and when you want to expose these variables out to user we can do that. but wasn\u0027t the scope of my review. also the way I could do that was to do something like \ninitialize cfg.CONF in self.__init__ of the class and then here do this \n\ndef join_group(self,group_id):\n  retryinterval\u003dself.retryinterval\n  @retrying.retry(stop_max_attempt_number\u003dmx_retries...\n                  ...)\n   def inner()\n           Original Function Definition\n\n   return inner()\n\nBut then I realized that may be over work then what I intended to do that. I can do it if that looks better?","commit_id":"7d868b270effb564d8db06a8f0b6d2e999173478"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"c61523d7540bdab1ce1b8cc90a1e6c247407c515","unresolved":false,"context_lines":[{"line_number":144,"context_line":"    @retrying.retry(stop_max_attempt_number\u003dcfg.CONF.coordination.max_retries,"},{"line_number":145,"context_line":"                    wait_fixed\u003dcfg.CONF.coordination.retry_interval*1000,"},{"line_number":146,"context_line":"                    retry_on_exception\u003dretry_on_error_joining_partition,"},{"line_number":147,"context_line":"                    wrap_exception\u003dTrue)"},{"line_number":148,"context_line":"    def join_group(self, group_id):"},{"line_number":149,"context_line":"        if (not self._coordinator or not self._coordinator.is_started"},{"line_number":150,"context_line":"                or not group_id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1af94dfe_c7ec88f5","line":147,"range":{"start_line":147,"start_character":20,"end_line":147,"end_character":34},"updated":"2016-03-18 06:48:58.000000000","message":"if we set wrap exception to True, when retry finally fail, it will raise retrying.RetryError instead of origin error","commit_id":"7d868b270effb564d8db06a8f0b6d2e999173478"},{"author":{"_account_id":11549,"name":"Atul Aggarwal","email":"atula@hp.com","username":"aaggarw3"},"change_message_id":"064e15aa01c9ab6deb19a26380de7f82d90df61f","unresolved":false,"context_lines":[{"line_number":144,"context_line":"    @retrying.retry(stop_max_attempt_number\u003dcfg.CONF.coordination.max_retries,"},{"line_number":145,"context_line":"                    wait_fixed\u003dcfg.CONF.coordination.retry_interval*1000,"},{"line_number":146,"context_line":"                    retry_on_exception\u003dretry_on_error_joining_partition,"},{"line_number":147,"context_line":"                    wrap_exception\u003dTrue)"},{"line_number":148,"context_line":"    def join_group(self, group_id):"},{"line_number":149,"context_line":"        if (not self._coordinator or not self._coordinator.is_started"},{"line_number":150,"context_line":"                or not group_id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1af94dfe_9bc8f956","line":147,"range":{"start_line":147,"start_character":20,"end_line":147,"end_character":34},"in_reply_to":"1af94dfe_c7ec88f5","updated":"2016-03-18 16:44:39.000000000","message":"I think it does both. So you get retrying.RetryError when retry finally fail but you don\u0027t get back the original error if wrap_exception is not set. That is why I set it to true so you get original error as part of retry error.\nSomething like this\n retrying.RetryError\n      something something .....\n   ERROR coordination join_group Error joining partitioning group central-global, re-trying\n\nBut that is what I understand .","commit_id":"7d868b270effb564d8db06a8f0b6d2e999173478"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"24f06ac544cb1dd209835b21d6d4e5a9de3ba830","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        except tooz.coordination.MemberAlreadyExist:"},{"line_number":157,"context_line":"            return"},{"line_number":158,"context_line":"        except tooz.coordination.GroupNotCreated:"},{"line_number":159,"context_line":"            create_grp_req \u003d self._coordinator.create_group(group_id)"},{"line_number":160,"context_line":"            try:"},{"line_number":161,"context_line":"                create_grp_req.get()"},{"line_number":162,"context_line":"            except tooz.coordination.GroupAlreadyExist:"},{"line_number":163,"context_line":"                pass"},{"line_number":164,"context_line":"        except tooz.coordination.ToozError:"},{"line_number":165,"context_line":"            LOG.exception(_LE(\u0027Error joining partitioning group %s,\u0027"},{"line_number":166,"context_line":"                              \u0027 re-trying\u0027), group_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1af94dfe_671e5460","line":163,"range":{"start_line":159,"start_character":12,"end_line":163,"end_character":20},"updated":"2016-03-18 06:39:56.000000000","message":"need to trigger a retry after group is created","commit_id":"7d868b270effb564d8db06a8f0b6d2e999173478"},{"author":{"_account_id":11549,"name":"Atul Aggarwal","email":"atula@hp.com","username":"aaggarw3"},"change_message_id":"a3ace7000c612ac3d2d0c28bcb37379c14c9df9a","unresolved":false,"context_lines":[{"line_number":162,"context_line":"            except tooz.coordination.GroupAlreadyExist:"},{"line_number":163,"context_line":"                join_group(group_id)"},{"line_number":164,"context_line":"        except tooz.coordination.ToozError:"},{"line_number":165,"context_line":"            LOG.exception(_LE(\u0027Error joining partitioning group %s,\u0027"},{"line_number":166,"context_line":"                              \u0027 re-trying\u0027), group_id)"},{"line_number":167,"context_line":"            raise ErrorJoiningPartitioningGroup()"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        self._groups.add(group_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1af94dfe_0dd30161","line":166,"range":{"start_line":165,"start_character":0,"end_line":166,"end_character":54},"updated":"2016-03-18 22:34:22.000000000","message":"This gives the information on what went wrong along with the retry exception and this output is already part of the existing unit test : ceilometer.tests.unit.test_coordination.TestPartitioning.test_coordination_backend_connection_fail_on_join","commit_id":"de68e4dc05ebfb2f10f62a176ee21fe514a066d9"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"137f303cd4a702a8a7a982c5e4f00634f21f31cb","unresolved":false,"context_lines":[{"line_number":44,"context_line":"               default\u003d10,"},{"line_number":45,"context_line":"               help\u003d\u0027Number of Seconds between retry to join partitioning \u0027"},{"line_number":46,"context_line":"                    \u0027group\u0027),"},{"line_number":47,"context_line":"    cfg.IntOpt(\u0027max_retries\u0027,"},{"line_number":48,"context_line":"               default\u003d30,"},{"line_number":49,"context_line":"               help\u003d\u0027Maximum number of retries before we give up to join \u0027"},{"line_number":50,"context_line":"                    \u0027partitioning group\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1af94dfe_106f91fc","line":47,"updated":"2016-03-19 09:02:41.000000000","message":"I don\u0027t think this is a good idea, what the expected daemon state after this 30 seconds. What happen after ?\n\nThe pollsters will retry after printing backtraces, but the notification agent will be in an unexpected state.","commit_id":"ab29c0de189459c0af8ebe24a545663dc1037a21"},{"author":{"_account_id":11549,"name":"Atul Aggarwal","email":"atula@hp.com","username":"aaggarw3"},"change_message_id":"91f8d3642d50dbaaf1267241d37b6e6131f0fd9e","unresolved":false,"context_lines":[{"line_number":44,"context_line":"               default\u003d10,"},{"line_number":45,"context_line":"               help\u003d\u0027Number of Seconds between retry to join partitioning \u0027"},{"line_number":46,"context_line":"                    \u0027group\u0027),"},{"line_number":47,"context_line":"    cfg.IntOpt(\u0027max_retries\u0027,"},{"line_number":48,"context_line":"               default\u003d30,"},{"line_number":49,"context_line":"               help\u003d\u0027Maximum number of retries before we give up to join \u0027"},{"line_number":50,"context_line":"                    \u0027partitioning group\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa0719c6_bab951ef","line":47,"in_reply_to":"1af94dfe_106f91fc","updated":"2016-03-20 00:30:59.000000000","message":"This is not 30 seconds. either of pollsters or notification agent are expected to retry for 5 mins (30 retries after 10 seconds apart) with this proposed change. After that they should give up and shutdown as far as I understand and reasonable than to keep wasting cpu cycles and resources on a broken component as it was the case before.","commit_id":"ab29c0de189459c0af8ebe24a545663dc1037a21"},{"author":{"_account_id":13560,"name":"Rohit Jaiswal","email":"rohit.jaiswal@gmail.com","username":"rjaiswal"},"change_message_id":"40b0f78a5aad90cb6c827aba975166d19d8f74c5","unresolved":false,"context_lines":[{"line_number":44,"context_line":"               default\u003d10,"},{"line_number":45,"context_line":"               help\u003d\u0027Number of Seconds between retry to join partitioning \u0027"},{"line_number":46,"context_line":"                    \u0027group\u0027),"},{"line_number":47,"context_line":"    cfg.IntOpt(\u0027max_retries\u0027,"},{"line_number":48,"context_line":"               default\u003d30,"},{"line_number":49,"context_line":"               help\u003d\u0027Maximum number of retries before we give up to join \u0027"},{"line_number":50,"context_line":"                    \u0027partitioning group\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa0719c6_edfe3f6a","line":47,"in_reply_to":"fa0719c6_bab951ef","updated":"2016-03-21 19:04:45.000000000","message":"I think it depends on the scenario. If this happens when the polling agent and n agent are starting, then the pollsters will not get a chance to setup and the behavior should be the same for both. If it happens later ( if a node goes down that triggers the join_group), then i think the pollsters will continue polling across the un-partitioned resource set whereas the n agent should stop consuming any data, but still keep running.\n\nBut what we are trying to do here is better than what we have currently - an infinite loop in the join_group method. Having an explicit retry on join_group will also retry when the join_group is called before any partitioning is done (before any extract_my_subset is called.)","commit_id":"ab29c0de189459c0af8ebe24a545663dc1037a21"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"137f303cd4a702a8a7a982c5e4f00634f21f31cb","unresolved":false,"context_lines":[{"line_number":142,"context_line":"            self._coordinator.run_watchers()"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    @retrying.retry(stop_max_attempt_number\u003dcfg.CONF.coordination.max_retries,"},{"line_number":145,"context_line":"                    wait_fixed\u003dcfg.CONF.coordination.retry_interval*1000,"},{"line_number":146,"context_line":"                    retry_on_exception\u003dretry_on_error_joining_partition,"},{"line_number":147,"context_line":"                    wrap_exception\u003dTrue)"},{"line_number":148,"context_line":"    def join_group(self, group_id):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1af94dfe_507599c8","line":145,"updated":"2016-03-19 09:02:41.000000000","message":"I think an exponential backoff with a maximum starting from some milliseconds will be better.","commit_id":"ab29c0de189459c0af8ebe24a545663dc1037a21"},{"author":{"_account_id":11549,"name":"Atul Aggarwal","email":"atula@hp.com","username":"aaggarw3"},"change_message_id":"91f8d3642d50dbaaf1267241d37b6e6131f0fd9e","unresolved":false,"context_lines":[{"line_number":142,"context_line":"            self._coordinator.run_watchers()"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    @retrying.retry(stop_max_attempt_number\u003dcfg.CONF.coordination.max_retries,"},{"line_number":145,"context_line":"                    wait_fixed\u003dcfg.CONF.coordination.retry_interval*1000,"},{"line_number":146,"context_line":"                    retry_on_exception\u003dretry_on_error_joining_partition,"},{"line_number":147,"context_line":"                    wrap_exception\u003dTrue)"},{"line_number":148,"context_line":"    def join_group(self, group_id):"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa0719c6_9abe0dd3","line":145,"in_reply_to":"1af94dfe_507599c8","updated":"2016-03-20 00:30:59.000000000","message":"Didn\u0027t get this, but intent was to have configured number of retries (30 in this proposal) at configured number of seconds apart (10 seconds in this proposal)","commit_id":"ab29c0de189459c0af8ebe24a545663dc1037a21"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"137f303cd4a702a8a7a982c5e4f00634f21f31cb","unresolved":false,"context_lines":[{"line_number":160,"context_line":"            try:"},{"line_number":161,"context_line":"                create_grp_req.get()"},{"line_number":162,"context_line":"            except tooz.coordination.GroupAlreadyExist:"},{"line_number":163,"context_line":"                self.join_group(group_id)"},{"line_number":164,"context_line":"        except tooz.coordination.ToozError:"},{"line_number":165,"context_line":"            LOG.exception(_LE(\u0027Error joining partitioning group %s,\u0027"},{"line_number":166,"context_line":"                              \u0027 re-trying\u0027), group_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1af94dfe_50ae392e","line":163,"updated":"2016-03-19 09:02:41.000000000","message":"This create a recursive loop, and will fail on the 255 tries.\n\nI guess we should raise retrying.RetryError instead.","commit_id":"ab29c0de189459c0af8ebe24a545663dc1037a21"},{"author":{"_account_id":11549,"name":"Atul Aggarwal","email":"atula@hp.com","username":"aaggarw3"},"change_message_id":"91f8d3642d50dbaaf1267241d37b6e6131f0fd9e","unresolved":false,"context_lines":[{"line_number":160,"context_line":"            try:"},{"line_number":161,"context_line":"                create_grp_req.get()"},{"line_number":162,"context_line":"            except tooz.coordination.GroupAlreadyExist:"},{"line_number":163,"context_line":"                self.join_group(group_id)"},{"line_number":164,"context_line":"        except tooz.coordination.ToozError:"},{"line_number":165,"context_line":"            LOG.exception(_LE(\u0027Error joining partitioning group %s,\u0027"},{"line_number":166,"context_line":"                              \u0027 re-trying\u0027), group_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa0719c6_7aadf987","line":163,"in_reply_to":"1af94dfe_50ae392e","updated":"2016-03-20 00:30:59.000000000","message":"I don\u0027t think its a always recursive loop. it is retrying to join group when group was not created before and created as part of the exception handler above (tooz.coordination.GroupNotCreated). If it gets to the exception not group not created even when group is existing and created then the bigger problem lies in tooz side. and if it is truly not created group and it is created just now, it will try to join the group again. this time it should not reach this code. And it will try configured number of retries to join the group. So I think it is ok to have it here.\nIf I am wrong, can you please elaborate on your concern?\n\nRegarding raising retrying.RetryError I think I would let retrying module do that rather then to enforce it here (even if it is 255 according to your calculation) to get back the original exception traceback in the logs. Since I don\u0027t think it is a infinite recursive loop and based on the explanation above.","commit_id":"ab29c0de189459c0af8ebe24a545663dc1037a21"},{"author":{"_account_id":11549,"name":"Atul Aggarwal","email":"atula@hp.com","username":"aaggarw3"},"change_message_id":"b07206ab95ee9737e71dbf38df83a5dcb7361f25","unresolved":false,"context_lines":[{"line_number":160,"context_line":"            try:"},{"line_number":161,"context_line":"                create_grp_req.get()"},{"line_number":162,"context_line":"            except tooz.coordination.GroupAlreadyExist:"},{"line_number":163,"context_line":"                self.join_group(group_id)"},{"line_number":164,"context_line":"        except tooz.coordination.ToozError:"},{"line_number":165,"context_line":"            LOG.exception(_LE(\u0027Error joining partitioning group %s,\u0027"},{"line_number":166,"context_line":"                              \u0027 re-trying\u0027), group_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa0719c6_a1f8cbe6","line":163,"in_reply_to":"fa0719c6_18b01343","updated":"2016-03-21 21:51:46.000000000","message":"Done. Understood Rohit. Thanks! will add the ErrorJoiningPartitioningGroup  exception and will update the retry decorator","commit_id":"ab29c0de189459c0af8ebe24a545663dc1037a21"},{"author":{"_account_id":13560,"name":"Rohit Jaiswal","email":"rohit.jaiswal@gmail.com","username":"rjaiswal"},"change_message_id":"40b0f78a5aad90cb6c827aba975166d19d8f74c5","unresolved":false,"context_lines":[{"line_number":160,"context_line":"            try:"},{"line_number":161,"context_line":"                create_grp_req.get()"},{"line_number":162,"context_line":"            except tooz.coordination.GroupAlreadyExist:"},{"line_number":163,"context_line":"                self.join_group(group_id)"},{"line_number":164,"context_line":"        except tooz.coordination.ToozError:"},{"line_number":165,"context_line":"            LOG.exception(_LE(\u0027Error joining partitioning group %s,\u0027"},{"line_number":166,"context_line":"                              \u0027 re-trying\u0027), group_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa0719c6_18b01343","line":163,"in_reply_to":"fa0719c6_7aadf987","updated":"2016-03-21 19:04:45.000000000","message":"I think there is a slight chance that 2 distinct processes find that group is not created and try to create the group, but one does it before the other, so the other one fails with the GroupAlreadyExist exception so that concern is valid. However, chances of facing the same race condition everytime are minimal, and to minimize such cases, retrying randomly is best i think, maybe we can use the wait_random_min and wait_random_max parameter in the retry decorator. \n\nInstead of raising the retryError, maybe we can raise the ErrorJoiningPartitioningGroup error to force the retry.","commit_id":"ab29c0de189459c0af8ebe24a545663dc1037a21"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"06b220889fda1f7b33ce7e9b4db0bab9d0593dfd","unresolved":false,"context_lines":[{"line_number":44,"context_line":"               default\u003d10,"},{"line_number":45,"context_line":"               help\u003d\u0027Number of Seconds between retry to join partitioning \u0027"},{"line_number":46,"context_line":"                    \u0027group\u0027),"},{"line_number":47,"context_line":"    cfg.IntOpt(\u0027max_retries\u0027,"},{"line_number":48,"context_line":"               default\u003d30,"},{"line_number":49,"context_line":"               help\u003d\u0027Maximum number of retries before we give up to join \u0027"},{"line_number":50,"context_line":"                    \u0027partitioning group\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa0719c6_6f50ce32","line":47,"updated":"2016-03-24 08:38:52.000000000","message":"I still disagree on this, stopping the notification agent after a while just because we can\u0027t join redis/zookeeper it not acceptable...","commit_id":"40a5a10534dba70df50f090d0573b7fd2e4044b8"},{"author":{"_account_id":11549,"name":"Atul Aggarwal","email":"atula@hp.com","username":"aaggarw3"},"change_message_id":"aebf095a93494c0f8758e9e927227e3efa821215","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    cfg.IntOpt(\u0027retry_interval\u0027,"},{"line_number":44,"context_line":"               default\u003d10,"},{"line_number":45,"context_line":"               help\u003d\u0027Number of Seconds between retry to join partitioning \u0027"},{"line_number":46,"context_line":"                    \u0027group\u0027)"},{"line_number":47,"context_line":"]"},{"line_number":48,"context_line":"cfg.CONF.register_opts(OPTS, group\u003d\u0027coordination\u0027)"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba0121b8_05944335","line":46,"range":{"start_line":46,"start_character":0,"end_line":46,"end_character":20},"updated":"2016-03-29 06:05:52.000000000","message":"Apparently I should not have used max_retries setting after all. Reason, max retries does lead to stop trying after configured number of times. but when it fails Polling agent doesn\u0027t die and hence leave the polling agent in a really bad state where it is not retrying and process still running (so basically frozen unless restarted forcefully). This is due to the fact the polling agent runs as oslo servivce and handler to the process launcher is not accessible after it is launched. Hence, I removed the max retries logic. Now the this proposal follows the previous logic of keep retrying but a little enhancement.\n\nI still think it is worth to add retry interval because it allows user to control the time in between two consecutive retrial and not wasting cpu cycles continuously for no reason when zookeeper is in bad shape. also number of log entries can be controlled and reduce to a great extent by adding this small logic.","commit_id":"0d72e1e4bb33d829a8eafa8ff01da3bf781ea1dd"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"543e275b2359fd8d426bfff61b3fc82ea705d131","unresolved":false,"context_lines":[{"line_number":150,"context_line":"            wait_random_max\u003dmax_retry_interval,"},{"line_number":151,"context_line":"            retry_on_exception\u003dretry_on_error_joining_partition,"},{"line_number":152,"context_line":"            wrap_exception\u003dTrue)"},{"line_number":153,"context_line":"        def _inner():"},{"line_number":154,"context_line":"            try:"},{"line_number":155,"context_line":"                join_req \u003d self._coordinator.join_group(group_id)"},{"line_number":156,"context_line":"                join_req.get()"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba0121b8_0568c3f4","line":153,"updated":"2016-03-29 06:20:59.000000000","message":"oslo.db and oslo.messaging uses exponential backoff for this kind of stuff, I guess we should do the same to be homogeneous for deployer.","commit_id":"0d72e1e4bb33d829a8eafa8ff01da3bf781ea1dd"},{"author":{"_account_id":11549,"name":"Atul Aggarwal","email":"atula@hp.com","username":"aaggarw3"},"change_message_id":"21c13a3ed90b0108c5cf22faeb3db8772bcfe755","unresolved":false,"context_lines":[{"line_number":150,"context_line":"            wait_random_max\u003dmax_retry_interval,"},{"line_number":151,"context_line":"            retry_on_exception\u003dretry_on_error_joining_partition,"},{"line_number":152,"context_line":"            wrap_exception\u003dTrue)"},{"line_number":153,"context_line":"        def _inner():"},{"line_number":154,"context_line":"            try:"},{"line_number":155,"context_line":"                join_req \u003d self._coordinator.join_group(group_id)"},{"line_number":156,"context_line":"                join_req.get()"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba0121b8_c9dc0115","line":153,"in_reply_to":"ba0121b8_0568c3f4","updated":"2016-03-29 17:48:14.000000000","message":"Any References? Thanks!","commit_id":"0d72e1e4bb33d829a8eafa8ff01da3bf781ea1dd"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"f036f381acf7064c650a46852a5b3442e00168af","unresolved":false,"context_lines":[{"line_number":150,"context_line":"            wait_random_max\u003dmax_retry_interval,"},{"line_number":151,"context_line":"            retry_on_exception\u003dretry_on_error_joining_partition,"},{"line_number":152,"context_line":"            wrap_exception\u003dTrue)"},{"line_number":153,"context_line":"        def _inner():"},{"line_number":154,"context_line":"            try:"},{"line_number":155,"context_line":"                join_req \u003d self._coordinator.join_group(group_id)"},{"line_number":156,"context_line":"                join_req.get()"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba0121b8_6327b712","line":153,"in_reply_to":"ba0121b8_c9dc0115","updated":"2016-03-30 07:58:52.000000000","message":"It\u0027s retry parameters: wait_exponential_multiplier\u003d1000, wait_exponential_max\u003d10000\n\noslo.msg: https://github.com/openstack/oslo.messaging/blob/master/oslo_messaging/_drivers/impl_rabbit.py#L754\n\n\nBut I was wrong for oslo.db, that is not exponential backoff : https://github.com/openstack/oslo.db/blob/7a52dcf17f94c5df0a64caeedfb90b4c2c4ad255/oslo_db/sqlalchemy/engines.py#L108\n\nSo if we are not already homogeneous, I don\u0027t really care :p","commit_id":"0d72e1e4bb33d829a8eafa8ff01da3bf781ea1dd"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"db56dd98c9e214ebcbdb6b89eaae151ef953a11c","unresolved":false,"context_lines":[{"line_number":42,"context_line":"                      \u0027membership has changed\u0027),"},{"line_number":43,"context_line":"    cfg.IntOpt(\u0027retry_backoff\u0027,"},{"line_number":44,"context_line":"               default\u003d1,"},{"line_number":45,"context_line":"               help\u003d\u0027How frequently to retry connecting with coordination \u0027"},{"line_number":46,"context_line":"                    \u0027backend\u0027),"},{"line_number":47,"context_line":"    cfg.IntOpt(\u0027max_retry_interval\u0027,"},{"line_number":48,"context_line":"               default\u003d30,"},{"line_number":49,"context_line":"               help\u003d\u0027Maximum Number of Seconds between retry to join \u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba0121b8_337b062a","line":46,"range":{"start_line":45,"start_character":21,"end_line":46,"end_character":28},"updated":"2016-03-31 09:01:30.000000000","message":"I think this is not the precise retry interval, is just a factor","commit_id":"6efab202b56c5b7f8557a44f28002550a77d07bf"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"db56dd98c9e214ebcbdb6b89eaae151ef953a11c","unresolved":false,"context_lines":[{"line_number":46,"context_line":"                    \u0027backend\u0027),"},{"line_number":47,"context_line":"    cfg.IntOpt(\u0027max_retry_interval\u0027,"},{"line_number":48,"context_line":"               default\u003d30,"},{"line_number":49,"context_line":"               help\u003d\u0027Maximum Number of Seconds between retry to join \u0027"},{"line_number":50,"context_line":"                    \u0027partitioning group\u0027)"},{"line_number":51,"context_line":"]"},{"line_number":52,"context_line":"cfg.CONF.register_opts(OPTS, group\u003d\u0027coordination\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba0121b8_b3955634","line":49,"range":{"start_line":49,"start_character":29,"end_line":49,"end_character":46},"updated":"2016-03-31 09:01:30.000000000","message":"number of seconds, lower case","commit_id":"6efab202b56c5b7f8557a44f28002550a77d07bf"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"db56dd98c9e214ebcbdb6b89eaae151ef953a11c","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                or not group_id):"},{"line_number":147,"context_line":"            return"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        retry_backoff \u003d cfg.CONF.coordination.retry_backoff*1000"},{"line_number":150,"context_line":"        max_retry_interval \u003d cfg.CONF.coordination.max_retry_interval*1000"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        @retrying.retry("}],"source_content_type":"text/x-python","patch_set":11,"id":"ba0121b8_d3de0a10","line":149,"range":{"start_line":149,"start_character":59,"end_line":149,"end_character":60},"updated":"2016-03-31 09:01:30.000000000","message":"need space around operator, it is strange that pep8 doesn\u0027t detect it","commit_id":"6efab202b56c5b7f8557a44f28002550a77d07bf"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"db56dd98c9e214ebcbdb6b89eaae151ef953a11c","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            return"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        retry_backoff \u003d cfg.CONF.coordination.retry_backoff*1000"},{"line_number":150,"context_line":"        max_retry_interval \u003d cfg.CONF.coordination.max_retry_interval*1000"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        @retrying.retry("},{"line_number":153,"context_line":"            wait_exponential_multiplier\u003dretry_backoff,"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba0121b8_53d31a04","line":150,"range":{"start_line":150,"start_character":69,"end_line":150,"end_character":70},"updated":"2016-03-31 09:01:30.000000000","message":"ditto","commit_id":"6efab202b56c5b7f8557a44f28002550a77d07bf"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"db56dd98c9e214ebcbdb6b89eaae151ef953a11c","unresolved":false,"context_lines":[{"line_number":161,"context_line":"                LOG.info(_LI(\u0027Joined partitioning group %s\u0027), group_id)"},{"line_number":162,"context_line":"            except tooz.coordination.MemberAlreadyExist:"},{"line_number":163,"context_line":"                return"},{"line_number":164,"context_line":"            except tooz.coordination.GroupNotCreated:"},{"line_number":165,"context_line":"                create_grp_req \u003d self._coordinator.create_group(group_id)"},{"line_number":166,"context_line":"                try:"},{"line_number":167,"context_line":"                    create_grp_req.get()"},{"line_number":168,"context_line":"                except tooz.coordination.GroupAlreadyExist:"},{"line_number":169,"context_line":"                    raise ErrorJoiningPartitioningGroup()"},{"line_number":170,"context_line":"            except tooz.coordination.ToozError:"},{"line_number":171,"context_line":"                LOG.exception(_LE(\u0027Error joining partitioning group %s,\u0027"},{"line_number":172,"context_line":"                                  \u0027 re-trying\u0027), group_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba0121b8_13b24206","line":169,"range":{"start_line":164,"start_character":12,"end_line":169,"end_character":57},"updated":"2016-03-31 09:01:30.000000000","message":"-1\n\nIf I understand this block correctly, when group is not created, we create it, then if creation succeed, we do nothing but return, then how this member join this new created group?\n\nI think no matter the group exist or not, after we create it, we both need to raise an ErrorJoiningPartitioningGroup exception","commit_id":"6efab202b56c5b7f8557a44f28002550a77d07bf"},{"author":{"_account_id":11549,"name":"Atul Aggarwal","email":"atula@hp.com","username":"aaggarw3"},"change_message_id":"3d0813ef2bada875c962474673fef8e8fd259c08","unresolved":false,"context_lines":[{"line_number":161,"context_line":"                LOG.info(_LI(\u0027Joined partitioning group %s\u0027), group_id)"},{"line_number":162,"context_line":"            except tooz.coordination.MemberAlreadyExist:"},{"line_number":163,"context_line":"                return"},{"line_number":164,"context_line":"            except tooz.coordination.GroupNotCreated:"},{"line_number":165,"context_line":"                create_grp_req \u003d self._coordinator.create_group(group_id)"},{"line_number":166,"context_line":"                try:"},{"line_number":167,"context_line":"                    create_grp_req.get()"},{"line_number":168,"context_line":"                except tooz.coordination.GroupAlreadyExist:"},{"line_number":169,"context_line":"                    raise ErrorJoiningPartitioningGroup()"},{"line_number":170,"context_line":"            except tooz.coordination.ToozError:"},{"line_number":171,"context_line":"                LOG.exception(_LE(\u0027Error joining partitioning group %s,\u0027"},{"line_number":172,"context_line":"                                  \u0027 re-trying\u0027), group_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba0121b8_4ab97f2d","line":169,"range":{"start_line":164,"start_character":12,"end_line":169,"end_character":57},"in_reply_to":"ba0121b8_13b24206","updated":"2016-03-31 17:57:47.000000000","message":"I think you are missing the line #174. It won\u0027t simply return, it will add the group id to _self.groups after it creates it successfully. line #167 is just just checking that group was indeed created successfully that is all, if I understand it correctly. if there is exception then we retry.\nLet me know if that make sense?","commit_id":"6efab202b56c5b7f8557a44f28002550a77d07bf"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"ecdf035417415539ee4c94436fbe379054472f1d","unresolved":false,"context_lines":[{"line_number":161,"context_line":"                LOG.info(_LI(\u0027Joined partitioning group %s\u0027), group_id)"},{"line_number":162,"context_line":"            except tooz.coordination.MemberAlreadyExist:"},{"line_number":163,"context_line":"                return"},{"line_number":164,"context_line":"            except tooz.coordination.GroupNotCreated:"},{"line_number":165,"context_line":"                create_grp_req \u003d self._coordinator.create_group(group_id)"},{"line_number":166,"context_line":"                try:"},{"line_number":167,"context_line":"                    create_grp_req.get()"},{"line_number":168,"context_line":"                except tooz.coordination.GroupAlreadyExist:"},{"line_number":169,"context_line":"                    raise ErrorJoiningPartitioningGroup()"},{"line_number":170,"context_line":"            except tooz.coordination.ToozError:"},{"line_number":171,"context_line":"                LOG.exception(_LE(\u0027Error joining partitioning group %s,\u0027"},{"line_number":172,"context_line":"                                  \u0027 re-trying\u0027), group_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba0121b8_31b910cd","line":169,"range":{"start_line":164,"start_character":12,"end_line":169,"end_character":57},"in_reply_to":"ba0121b8_4ab97f2d","updated":"2016-03-31 23:55:13.000000000","message":"Hi Atul, The problem I described is that tooz create_group is asynchronous, so if two agents invoke this step, and the time delta is very low, then one will success, another will raise GroupAlreadyExist exception, then the first agent will fail to join group because it will directly return (only add to the set is meanless, we need to call tooz join_group), the second one will successfully join group because it will trigger another retry. IIUC","commit_id":"6efab202b56c5b7f8557a44f28002550a77d07bf"},{"author":{"_account_id":13560,"name":"Rohit Jaiswal","email":"rohit.jaiswal@gmail.com","username":"rjaiswal"},"change_message_id":"b2fcb2e73dde2601afb5bf2656c70b4464533427","unresolved":false,"context_lines":[{"line_number":161,"context_line":"                LOG.info(_LI(\u0027Joined partitioning group %s\u0027), group_id)"},{"line_number":162,"context_line":"            except tooz.coordination.MemberAlreadyExist:"},{"line_number":163,"context_line":"                return"},{"line_number":164,"context_line":"            except tooz.coordination.GroupNotCreated:"},{"line_number":165,"context_line":"                create_grp_req \u003d self._coordinator.create_group(group_id)"},{"line_number":166,"context_line":"                try:"},{"line_number":167,"context_line":"                    create_grp_req.get()"},{"line_number":168,"context_line":"                except tooz.coordination.GroupAlreadyExist:"},{"line_number":169,"context_line":"                    raise ErrorJoiningPartitioningGroup()"},{"line_number":170,"context_line":"            except tooz.coordination.ToozError:"},{"line_number":171,"context_line":"                LOG.exception(_LE(\u0027Error joining partitioning group %s,\u0027"},{"line_number":172,"context_line":"                                  \u0027 re-trying\u0027), group_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba0121b8_4db9962d","line":169,"range":{"start_line":164,"start_character":12,"end_line":169,"end_character":57},"in_reply_to":"ba0121b8_4ab97f2d","updated":"2016-03-31 19:11:09.000000000","message":"agree with ZQ, the exception should be raised regardless, else the group will be added to the _groups set, but not joined it in Tooz(line 159).","commit_id":"6efab202b56c5b7f8557a44f28002550a77d07bf"}]}