)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"a713e14df5574062021e8c92c2a18b549c7a6dd7","unresolved":false,"context_lines":[{"line_number":18,"context_line":"It can be a placeholder in train/master then reverted once the"},{"line_number":19,"context_line":"DB objects make proper change to new_facade."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"The first change is critical, and adds a semaphore from Oslo\u0027s lockutils."},{"line_number":22,"context_line":"I have set external\u003dTrue because we can have multiple RPC workers"},{"line_number":23,"context_line":"and state report workers. With just this change, the easily"},{"line_number":24,"context_line":"reproducible q-reports-plugin growth/stuck DB queries go away."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"The other change is just an optimization. If the server loses"},{"line_number":27,"context_line":"connection to AMQP or DB, it won\u0027t process report_state RPCs."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3fa7e38b_6e4deac3","line":24,"range":{"start_line":21,"start_character":0,"end_line":24,"end_character":62},"updated":"2019-12-05 09:36:25.000000000","message":"Please correct me if I\u0027m mistaken, but that locking seems to work only for multiple workers running on the same host. A typical real world deployment would have multiple hosts, how would you avoid the issue there?","commit_id":"5fb323c890ddaba25650d4e5306271f48478b452"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"50270089d9ede3ea5572bec312fcdb063642b580","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Lockutils for Agent state report"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This is a temporary fix for https://bugs.launchpad.net/neutron/+bug/1853071"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"The issue is also resolved by https://review.opendev.org/#/c/697215/"},{"line_number":12,"context_line":"but hits a few failures in master, but most definitely won\u0027t merge"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3fa7e38b_c1f3be6c","line":9,"range":{"start_line":9,"start_character":28,"end_line":9,"end_character":75},"updated":"2019-12-19 15:48:09.000000000","message":"Nit: try to avoid exceeding the 72 columns limit for the commit message. In this case, yo can do that by re-stating this as:\n\nThis is a temporary fix for [0].\n\nAnd then, at the bottom of the commit message add:\n\n[0]https://bugs.launchpad.net/neutron/+bug/1853071","commit_id":"9fbe711ea836a01fc05b6f801dd51ee106d88555"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"50270089d9ede3ea5572bec312fcdb063642b580","unresolved":false,"context_lines":[{"line_number":19,"context_line":"DB objects make proper change to new_facade."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"This change adds a semaphore from Oslo\u0027s lockutils."},{"line_number":22,"context_line":"Please see my comment #5 on bug for exact blocked queries and transactions."},{"line_number":23,"context_line":"I suspect it may deal with issues with eventlet yields."},{"line_number":24,"context_line":"With just this change, the easily reproducible"},{"line_number":25,"context_line":"q-reports-plugin growth/stuck DB queries go away."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3fa7e38b_e1835aa9","line":22,"range":{"start_line":22,"start_character":72,"end_line":22,"end_character":75},"updated":"2019-12-19 15:48:09.000000000","message":"Same observation about the 72 columns here. Please note the dotted vertical line, which highlights the limit","commit_id":"9fbe711ea836a01fc05b6f801dd51ee106d88555"}],"neutron/db/agents_db.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3f2388919939857e892be6fcd2ae5f60c0ad2b57","unresolved":false,"context_lines":[{"line_number":370,"context_line":"                      \u0027agent_timestamp\u0027: agent_timestamp})"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"    @db_api.retry_if_session_inactive()"},{"line_number":373,"context_line":"    @lockutils.synchronized(\u0027agent-db\u0027, external\u003dTrue)"},{"line_number":374,"context_line":"    def create_or_update_agent(self, context, agent_state,"},{"line_number":375,"context_line":"                               agent_timestamp\u003dNone):"},{"line_number":376,"context_line":"        \"\"\"Registers new agent in the database or updates existing."}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_d19d9309","line":373,"updated":"2019-12-05 19:47:03.000000000","message":"Is this alone the temporary fix?  If so it should be in it\u0027s own change an the optimization in another.","commit_id":"5fb323c890ddaba25650d4e5306271f48478b452"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3f2388919939857e892be6fcd2ae5f60c0ad2b57","unresolved":false,"context_lines":[{"line_number":544,"context_line":""},{"line_number":545,"context_line":"        Method checks if the agent time is in sync with the server time"},{"line_number":546,"context_line":"        on start up. Ignores it, on subsequent re-connects."},{"line_number":547,"context_line":"        \"\"\""},{"line_number":548,"context_line":"        time_server_now \u003d timeutils.utcnow()"},{"line_number":549,"context_line":"        diff \u003d abs(timeutils.delta_seconds(time_server_now, agent_time))"},{"line_number":550,"context_line":"        if diff \u003e cfg.CONF.agent_down_time:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_1166cb2a","line":547,"updated":"2019-12-05 19:47:03.000000000","message":"Obviously the above comment is wrong now.\n\nAlso, by removing the \u0027start_flag\u0027 check, we\u0027re now running this code all the time, possibly generating lots of warnings.  That doesn\u0027t seem correct.","commit_id":"5fb323c890ddaba25650d4e5306271f48478b452"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"50270089d9ede3ea5572bec312fcdb063642b580","unresolved":false,"context_lines":[{"line_number":538,"context_line":""},{"line_number":539,"context_line":"    def _check_clock_sync_on_agent_start(self, agent_state, agent_time):"},{"line_number":540,"context_line":"        \"\"\"Checks if the server and the agent times are in sync."},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"        Method checks if the agent time is in sync with the server time"},{"line_number":543,"context_line":"        on start up. Ignores it, on subsequent re-connects."},{"line_number":544,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_41b6ee8b","side":"PARENT","line":541,"updated":"2019-12-19 15:48:09.000000000","message":"This a gratuitous change un related to the patch. Please don\u0027t do these changes because they are confusing to reviewers. A patch should focus on one change for ease of review","commit_id":"3a91398c14c504320e37047fc03313692a9e8182"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c6c8ad19a9cbf87a1474ea733e5f1f204cee596b","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                res[\u0027heartbeat_timestamp\u0027] \u003d current_time"},{"line_number":410,"context_line":"                if agent_state.get(\u0027start_flag\u0027):"},{"line_number":411,"context_line":"                    res[\u0027started_at\u0027] \u003d current_time"},{"line_number":412,"context_line":"                greenthread.sleep(0)"},{"line_number":413,"context_line":"                self._log_heartbeat(agent_state, agent, configurations_dict,"},{"line_number":414,"context_line":"                                    agent_timestamp)"},{"line_number":415,"context_line":"                agent.update_fields(res)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_e70daa36","line":412,"range":{"start_line":412,"start_character":16,"end_line":412,"end_character":36},"updated":"2019-12-19 16:48:25.000000000","message":"IMO, those (L412, L420, L426) the blockers in this method. Those sleeps were added 6 years ago as a workaround for two bugs.\n\nIf we stop the greenthread operation by adding a sleep, in heavy load systems we can make this thread to be hanged forever. A DB context (and especially if it is writing one), should finish ASAP.\n\nPlease, instead of adding another lock (we have a lot in the code), try to remove those, IMO, unneeded sleeps.","commit_id":"9fbe711ea836a01fc05b6f801dd51ee106d88555"}]}