)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"43efdfcf4c3b7250bf00ccf46f0b695bcdfb68f9","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Race conditions may cause DBDuplicateEntry exceptions that"},{"line_number":10,"context_line":"require a transaction to be rollbacked back, and yet make"},{"line_number":11,"context_line":"the whole operation succeed. A classic example is what has"},{"line_number":12,"context_line":"been solved in commit 5575f3b."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"However, if the rollback is done in a nested transaction,"},{"line_number":15,"context_line":"the above mentioned exception is raised. To address the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"baada198_da25e8bf","line":12,"updated":"2014-08-08 21:38:19.000000000","message":"5575f3b is the merge commit where it merged.  Unfortunately, gerrit doesn\u0027t find the commit when I search on this.\n\nfbc6b991a79a147bf1411b643f5c304062e5956a is the commit with the changes and gerrit finds that.  Gerrit also finds fbc6b99.  So, you could use that here.","commit_id":"6691331cdeb83e2ec44c1197c62c02aecae419cb"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"787386ea8863bacac8f3d1ce90245bb4a4b4faf2","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Race conditions may cause DBDuplicateEntry exceptions that"},{"line_number":10,"context_line":"require a transaction to be rollbacked back, and yet make"},{"line_number":11,"context_line":"the whole operation succeed. A classic example is what has"},{"line_number":12,"context_line":"been solved in commit 5575f3b."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"However, if the rollback is done in a nested transaction,"},{"line_number":15,"context_line":"the above mentioned exception is raised. To address the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"baada198_6587b4b6","line":12,"in_reply_to":"baada198_da25e8bf","updated":"2014-08-11 14:20:17.000000000","message":"Done","commit_id":"6691331cdeb83e2ec44c1197c62c02aecae419cb"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"43efdfcf4c3b7250bf00ccf46f0b695bcdfb68f9","unresolved":false,"context_lines":[{"line_number":22,"context_line":"that certain DBMS, or certain DB backend configurations,"},{"line_number":23,"context_line":"support savepoints."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"That said, this specific case can be addressed by removing"},{"line_number":26,"context_line":"the extra nesting (which is effectively redundant), thus"},{"line_number":27,"context_line":"making the change rather trivial."},{"line_number":28,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"baada198_b5168548","line":25,"updated":"2014-08-08 21:38:19.000000000","message":"Could this paragraph go first so that you start the commit message by describing the actual fix this patch makes?  Then, you can add your paragraphs about race conditions and begin_nested.","commit_id":"6691331cdeb83e2ec44c1197c62c02aecae419cb"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"787386ea8863bacac8f3d1ce90245bb4a4b4faf2","unresolved":false,"context_lines":[{"line_number":22,"context_line":"that certain DBMS, or certain DB backend configurations,"},{"line_number":23,"context_line":"support savepoints."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"That said, this specific case can be addressed by removing"},{"line_number":26,"context_line":"the extra nesting (which is effectively redundant), thus"},{"line_number":27,"context_line":"making the change rather trivial."},{"line_number":28,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"baada198_858208a3","line":25,"in_reply_to":"baada198_b5168548","updated":"2014-08-11 14:20:17.000000000","message":"Done","commit_id":"6691331cdeb83e2ec44c1197c62c02aecae419cb"}],"neutron/scheduler/l3_agent_scheduler.py":[{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"17734e4dbae445198baa8c4ce061ea7f42d94c57","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":180,"context_line":"                binding \u003d l3_agentschedulers_db.RouterL3AgentBinding()"},{"line_number":181,"context_line":"                binding.l3_agent \u003d chosen_agent"},{"line_number":182,"context_line":"                binding.router_id \u003d router_id"},{"line_number":183,"context_line":"                context.session.add(binding)"},{"line_number":184,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":185,"context_line":"            LOG.debug(\u0027Router %(router_id)s has already been scheduled \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_65c25487","line":182,"updated":"2014-08-11 14:04:14.000000000","message":"I was actually thinking about preventing auto rollback here, by issuing session.flush() and wrapping it with try-except:\n with (...) :\n   ...\n   try:\n       context.session.flush()\n   except db_exc.DBDuplicateEntry:\n       ...\n\nThe reason is that bind_router is used under transaction in several places where transactions actually make sense.\nSo applying fix here we avoid issues in other places as well.","commit_id":"b4eaa0520990f30519aaa073f352541f17b0577b"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"34a62e6abf6e46ef4c994979da5909ae523ab7b1","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":180,"context_line":"                binding \u003d l3_agentschedulers_db.RouterL3AgentBinding()"},{"line_number":181,"context_line":"                binding.l3_agent \u003d chosen_agent"},{"line_number":182,"context_line":"                binding.router_id \u003d router_id"},{"line_number":183,"context_line":"                context.session.add(binding)"},{"line_number":184,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":185,"context_line":"            LOG.debug(\u0027Router %(router_id)s has already been scheduled \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_5080a9f5","line":182,"in_reply_to":"baada198_051a787b","updated":"2014-08-11 15:41:06.000000000","message":"the with should be inside the try except to ensure the with context is aware when a db exception is raised to perform a rollback. \n\nEnikanorov proposal reverses them which implies if a DBDuplicateEntry is raised (in such case the transaction is broken), then the except DBDuplicateEntry MUST reraise an error otherwise the with context will not \"see\" the exception and try to commit a broken transaction \u003d\u003d\u003e fail","commit_id":"b4eaa0520990f30519aaa073f352541f17b0577b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"62ee6747a249f0d00fefd84d1e5119ff43bd1ac0","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":180,"context_line":"                binding \u003d l3_agentschedulers_db.RouterL3AgentBinding()"},{"line_number":181,"context_line":"                binding.l3_agent \u003d chosen_agent"},{"line_number":182,"context_line":"                binding.router_id \u003d router_id"},{"line_number":183,"context_line":"                context.session.add(binding)"},{"line_number":184,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":185,"context_line":"            LOG.debug(\u0027Router %(router_id)s has already been scheduled \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_4b1f17b8","line":182,"in_reply_to":"baada198_2bdbc37d","updated":"2014-08-11 19:12:41.000000000","message":"I gave this another whirl: adding a flush() in almost any combination does not address the issue; I ran out of bullets. Besides the begin_nested(), the only fix is removing one level of nesting, which I still think it\u0027s redundant. But I would love to be proved wrong.","commit_id":"b4eaa0520990f30519aaa073f352541f17b0577b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3b4e0e23ad0fc6f455df6be186f015869d2658eb","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":180,"context_line":"                binding \u003d l3_agentschedulers_db.RouterL3AgentBinding()"},{"line_number":181,"context_line":"                binding.l3_agent \u003d chosen_agent"},{"line_number":182,"context_line":"                binding.router_id \u003d router_id"},{"line_number":183,"context_line":"                context.session.add(binding)"},{"line_number":184,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":185,"context_line":"            LOG.debug(\u0027Router %(router_id)s has already been scheduled \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_2bdbc37d","line":182,"in_reply_to":"baada198_4b1b9e6e","updated":"2014-08-11 19:00:25.000000000","message":"enikanorov:  Can you elaborate on the contexts where it still makes sense to start a transaction in schedule?  Are you saying that they\u0027re not effectively redundant as the commit message claims?","commit_id":"b4eaa0520990f30519aaa073f352541f17b0577b"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"4fcbc7d997e8fc50715201feb3aeca04abf5e314","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":180,"context_line":"                binding \u003d l3_agentschedulers_db.RouterL3AgentBinding()"},{"line_number":181,"context_line":"                binding.l3_agent \u003d chosen_agent"},{"line_number":182,"context_line":"                binding.router_id \u003d router_id"},{"line_number":183,"context_line":"                context.session.add(binding)"},{"line_number":184,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":185,"context_line":"            LOG.debug(\u0027Router %(router_id)s has already been scheduled \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_62ba2372","line":182,"in_reply_to":"baada198_4b1f17b8","updated":"2014-08-12 10:51:24.000000000","message":"When a DBDuplicateEntry error is raised in bind_router, it brokes the current transaction and PARENT transactions. The current transaction is correctly ended by a rollback done by the context but the parent transaction defined in \"schedule\" method is not because no error is raised and the with in \"schedule\" tries to perform a session.commit on a broken transaction \u003d\u003d\u003e crash.\n\nSo the with in schedule should be removed or bind_router should use a nested subtransaction (currently not working with pysqlite without a patch in oslo.db under review).","commit_id":"b4eaa0520990f30519aaa073f352541f17b0577b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"756f1b6781fab276807d2887305189923e2eba33","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":180,"context_line":"                binding \u003d l3_agentschedulers_db.RouterL3AgentBinding()"},{"line_number":181,"context_line":"                binding.l3_agent \u003d chosen_agent"},{"line_number":182,"context_line":"                binding.router_id \u003d router_id"},{"line_number":183,"context_line":"                context.session.add(binding)"},{"line_number":184,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":185,"context_line":"            LOG.debug(\u0027Router %(router_id)s has already been scheduled \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_4b1b9e6e","line":182,"in_reply_to":"baada198_5080a9f5","updated":"2014-08-11 15:51:02.000000000","message":"I tried flush(), by keeping the structure of the code as it is, and I was still unable to address the stack trace.","commit_id":"b4eaa0520990f30519aaa073f352541f17b0577b"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"c67535372ddfd6137fa141fee93fece5696dc3fe","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":180,"context_line":"                binding \u003d l3_agentschedulers_db.RouterL3AgentBinding()"},{"line_number":181,"context_line":"                binding.l3_agent \u003d chosen_agent"},{"line_number":182,"context_line":"                binding.router_id \u003d router_id"},{"line_number":183,"context_line":"                context.session.add(binding)"},{"line_number":184,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":185,"context_line":"            LOG.debug(\u0027Router %(router_id)s has already been scheduled \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_9ee5cfa4","line":182,"in_reply_to":"baada198_62ba2372","updated":"2014-08-12 12:14:57.000000000","message":"After a discussion with Armando, I agree that the best approach will be to remove the parent transaction, as this patch suggests, but in addition to that, auto_schedule_routers should also be fixed, because that method has the same issue as schedule()\n\nThat, however, might be a matter of separate patch","commit_id":"b4eaa0520990f30519aaa073f352541f17b0577b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"0b9440bd6cc2ba5267b08f096a4268cfed4d92ad","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":180,"context_line":"                binding \u003d l3_agentschedulers_db.RouterL3AgentBinding()"},{"line_number":181,"context_line":"                binding.l3_agent \u003d chosen_agent"},{"line_number":182,"context_line":"                binding.router_id \u003d router_id"},{"line_number":183,"context_line":"                context.session.add(binding)"},{"line_number":184,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":185,"context_line":"            LOG.debug(\u0027Router %(router_id)s has already been scheduled \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_051a787b","line":182,"in_reply_to":"baada198_65aed430","updated":"2014-08-11 14:26:15.000000000","message":"Not sure how this is different from Eugene\u0027s comment? We should not flip the order of try/with as it currently is, should we?","commit_id":"b4eaa0520990f30519aaa073f352541f17b0577b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"1ca7f863b613d9a6ed9b1e3ae575a5adb7a2e6a4","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":180,"context_line":"                binding \u003d l3_agentschedulers_db.RouterL3AgentBinding()"},{"line_number":181,"context_line":"                binding.l3_agent \u003d chosen_agent"},{"line_number":182,"context_line":"                binding.router_id \u003d router_id"},{"line_number":183,"context_line":"                context.session.add(binding)"},{"line_number":184,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":185,"context_line":"            LOG.debug(\u0027Router %(router_id)s has already been scheduled \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_25077c71","line":182,"in_reply_to":"baada198_65c25487","updated":"2014-08-11 14:16:36.000000000","message":"I did try flush but it did not help, I was still getting the same stack trace in my repro environment. I would love to be proved wrong though.","commit_id":"b4eaa0520990f30519aaa073f352541f17b0577b"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"65485777b98a17e43e95482606cb6d29742b9ed6","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":180,"context_line":"                binding \u003d l3_agentschedulers_db.RouterL3AgentBinding()"},{"line_number":181,"context_line":"                binding.l3_agent \u003d chosen_agent"},{"line_number":182,"context_line":"                binding.router_id \u003d router_id"},{"line_number":183,"context_line":"                context.session.add(binding)"},{"line_number":184,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":185,"context_line":"            LOG.debug(\u0027Router %(router_id)s has already been scheduled \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_65aed430","line":182,"in_reply_to":"baada198_65c25487","updated":"2014-08-11 14:20:40.000000000","message":"if so, you should do:\n\n  try: \n    with (...):\n      ...flush...      \n  except db_exc.DBDuplicateEntry:\n    ...\n\notherwise the with context is not aware that an error was raised and try a session.commit instead of a session.rollback","commit_id":"b4eaa0520990f30519aaa073f352541f17b0577b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"8e11684c7eb5f89b2f8130791d2f8d115e292e99","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":180,"context_line":"                binding \u003d l3_agentschedulers_db.RouterL3AgentBinding()"},{"line_number":181,"context_line":"                binding.l3_agent \u003d chosen_agent"},{"line_number":182,"context_line":"                binding.router_id \u003d router_id"},{"line_number":183,"context_line":"                context.session.add(binding)"},{"line_number":184,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":185,"context_line":"            LOG.debug(\u0027Router %(router_id)s has already been scheduled \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_681afa6b","line":182,"in_reply_to":"baada198_9ee5cfa4","updated":"2014-08-12 15:07:38.000000000","message":"right; I\u0027d rather see the failure show up and fix it then (possibly unifying the two approaches).","commit_id":"b4eaa0520990f30519aaa073f352541f17b0577b"}]}