{"action":"created","issue":{"url":"https://api.github.com/repos/neo4j/neo4j/issues/5817","labels_url":"https://api.github.com/repos/neo4j/neo4j/issues/5817/labels{/name}","comments_url":"https://api.github.com/repos/neo4j/neo4j/issues/5817/comments","events_url":"https://api.github.com/repos/neo4j/neo4j/issues/5817/events","html_url":"https://github.com/neo4j/neo4j/pull/5817","id":114769848,"number":5817,"title":"Ensures all lock clients closed during role switch","user":{"login":"tinwelint","id":187292,"avatar_url":"https://avatars.githubusercontent.com/u/187292?v=3","gravatar_id":"","url":"https://api.github.com/users/tinwelint","html_url":"https://github.com/tinwelint","followers_url":"https://api.github.com/users/tinwelint/followers","following_url":"https://api.github.com/users/tinwelint/following{/other_user}","gists_url":"https://api.github.com/users/tinwelint/gists{/gist_id}","starred_url":"https://api.github.com/users/tinwelint/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tinwelint/subscriptions","organizations_url":"https://api.github.com/users/tinwelint/orgs","repos_url":"https://api.github.com/users/tinwelint/repos","events_url":"https://api.github.com/users/tinwelint/events{/privacy}","received_events_url":"https://api.github.com/users/tinwelint/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/neo4j/neo4j/labels/2.2","name":"2.2","color":"bfd4f2"},{"url":"https://api.github.com/repos/neo4j/neo4j/labels/kernel","name":"kernel","color":"0052cc"}],"state":"open","locked":false,"assignee":{"login":"lutovich","id":1638145,"avatar_url":"https://avatars.githubusercontent.com/u/1638145?v=3","gravatar_id":"","url":"https://api.github.com/users/lutovich","html_url":"https://github.com/lutovich","followers_url":"https://api.github.com/users/lutovich/followers","following_url":"https://api.github.com/users/lutovich/following{/other_user}","gists_url":"https://api.github.com/users/lutovich/gists{/gist_id}","starred_url":"https://api.github.com/users/lutovich/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lutovich/subscriptions","organizations_url":"https://api.github.com/users/lutovich/orgs","repos_url":"https://api.github.com/users/lutovich/repos","events_url":"https://api.github.com/users/lutovich/events{/privacy}","received_events_url":"https://api.github.com/users/lutovich/received_events","type":"User","site_admin":false},"milestone":null,"comments":1,"created_at":"2015-11-03T08:48:44Z","updated_at":"2015-11-03T09:53:52Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/neo4j/neo4j/pulls/5817","html_url":"https://github.com/neo4j/neo4j/pull/5817","diff_url":"https://github.com/neo4j/neo4j/pull/5817.diff","patch_url":"https://github.com/neo4j/neo4j/pull/5817.patch"},"body":"Observed issue:\r\nAfter an instance switching role in HA, for some reason some transactions\r\njust wouldn't respect the locks that other transactions held. This would\r\nlead to some transactions making changes on top of stale data, effectively\r\noverwriting changes of other transactions.\r\n\r\nCause(s):\r\nAmong other things a new lock manager instance is created during role\r\nswitching. Transaction instances pooled and created with a locks client\r\ninstance which is kept throughout its life. While there may be transactions\r\nstill executing at the time of switching role, all open transactions are\r\nmarked for termination and transactions pool cleared. Transactions marked\r\nfor termination cannot commit, but when closed they were happily returned\r\nto the transaction pool and would be reused again and again.\r\nThese transactions would have its termination flag reset at the point of\r\nbeing reused and would still contain a locks client for the previous\r\nlock manager. These transaction instances would forever disrespect the\r\nlocks that all other transactions respected and would continue to do so until\r\nthe next role switch, where they at least would have a chance of being\r\nproperly disposed of. Although the next role switch would have a chance of\r\nintroducing new rogue transactions as well...\r\n\r\nSolution(s)\r\nMany small issues here and there caused this to happen. There are a number\r\nof changes that will prevent this from happening. Some of them are\r\nbelt-and-suspenders additions, although at virtually no cost:\r\n\r\n- Disposing kernel transactions as part of switching to pending.\r\n  Previously they were disposed of as late as after creating the new ones,\r\n  which would increase the chance of transactions using locks clients from\r\n  the old lock manager to be used and cause harm.\r\n- Mark _all_ transactions, not just open transactions, for termination\r\n  when switching role. No transactions referring to the previous lock manager\r\n  can be around after the role switch.\r\n- Do not return transaction (KernelTransactionImplementation) instances\r\n  that are marked for termination to the pool.\r\n- Have Locks instances know when they are closed and refuse to hand out new\r\n  clients if closed.\r\n\r\nOn top of those changes, with accompanied unit tests, there's an added\r\nstress test `TransactionThroughMasterSwitchStressIT` which quite\r\ndeterministically could reproduce the problem. It can be set to run for a\r\nlonger period of time (default 30s in the main build) using\r\nsystem property, like:\r\n\r\n  `-Dorg.neo4j.kernel.ha.transaction.TransactionThroughMasterSwitchStressIT.duration=10m`\r\n\r\nCurrently that stress test provokes only one role switch scenario:\r\nmaster --> pending --> master, although more scenarios could be added\r\nlater on for an even broader net to catch these sorts of problems."},"comment":{"url":"https://api.github.com/repos/neo4j/neo4j/issues/comments/153302149","html_url":"https://github.com/neo4j/neo4j/pull/5817#issuecomment-153302149","issue_url":"https://api.github.com/repos/neo4j/neo4j/issues/5817","id":153302149,"user":{"login":"chrisvest","id":7993,"avatar_url":"https://avatars.githubusercontent.com/u/7993?v=3","gravatar_id":"","url":"https://api.github.com/users/chrisvest","html_url":"https://github.com/chrisvest","followers_url":"https://api.github.com/users/chrisvest/followers","following_url":"https://api.github.com/users/chrisvest/following{/other_user}","gists_url":"https://api.github.com/users/chrisvest/gists{/gist_id}","starred_url":"https://api.github.com/users/chrisvest/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chrisvest/subscriptions","organizations_url":"https://api.github.com/users/chrisvest/orgs","repos_url":"https://api.github.com/users/chrisvest/repos","events_url":"https://api.github.com/users/chrisvest/events{/privacy}","received_events_url":"https://api.github.com/users/chrisvest/received_events","type":"User","site_admin":false},"created_at":"2015-11-03T09:53:52Z","updated_at":"2015-11-03T09:53:52Z","body":"Really nice commit message."}}