{"action":"created","issue":{"url":"https://api.github.com/repos/ezsystems/ezpublish-kernel/issues/2339","repository_url":"https://api.github.com/repos/ezsystems/ezpublish-kernel","labels_url":"https://api.github.com/repos/ezsystems/ezpublish-kernel/issues/2339/labels{/name}","comments_url":"https://api.github.com/repos/ezsystems/ezpublish-kernel/issues/2339/comments","events_url":"https://api.github.com/repos/ezsystems/ezpublish-kernel/issues/2339/events","html_url":"https://github.com/ezsystems/ezpublish-kernel/pull/2339","id":326143347,"number":2339,"title":"EZP-29206: MySQL deadlock errors happen when concurrently moving subtree","user":{"login":"alongosz","id":7099219,"avatar_url":"https://avatars1.githubusercontent.com/u/7099219?v=4","gravatar_id":"","url":"https://api.github.com/users/alongosz","html_url":"https://github.com/alongosz","followers_url":"https://api.github.com/users/alongosz/followers","following_url":"https://api.github.com/users/alongosz/following{/other_user}","gists_url":"https://api.github.com/users/alongosz/gists{/gist_id}","starred_url":"https://api.github.com/users/alongosz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alongosz/subscriptions","organizations_url":"https://api.github.com/users/alongosz/orgs","repos_url":"https://api.github.com/users/alongosz/repos","events_url":"https://api.github.com/users/alongosz/events{/privacy}","received_events_url":"https://api.github.com/users/alongosz/received_events","type":"User","site_admin":false},"labels":[{"id":430933236,"url":"https://api.github.com/repos/ezsystems/ezpublish-kernel/labels/Ready%20for%20QA","name":"Ready for QA","color":"fbca04","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2018-05-24T14:17:59Z","updated_at":"2018-05-28T08:38:18Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/ezsystems/ezpublish-kernel/pulls/2339","html_url":"https://github.com/ezsystems/ezpublish-kernel/pull/2339","diff_url":"https://github.com/ezsystems/ezpublish-kernel/pull/2339.diff","patch_url":"https://github.com/ezsystems/ezpublish-kernel/pull/2339.patch"},"body":"| Question           | Answer\r\n| ------------------ | ------------------\r\n| **JIRA issue**     | [EZP-29206](https://jira.ez.no/browse/EZP-29206)\r\n| **Bug/Improvement**| yes\r\n| **New feature**    | no\r\n| **Target version** | `6.7` and up.\r\n| **BC breaks**      | no\r\n| **Tests pass**     | yes\r\n| **Doc needed**     | no\r\n\r\nThis PR presents one of the possible solutions for database deadlocks that occur when concurrently moving subtree. The solution was suggested by the Customer and improved by us.\r\n\r\nThe solution splits update-where-id-in-select query into two separate queries to avoid locks. While possible deadlocks are documented for InnoDB and solution is to set proper indices, there's no solution for the case when update and select operate on different tables.\r\n\r\nTherefore obvious choice would be to split these operations. I was reluctant to do so because that introduces possible race condition between these select and update. However I haven't found so far real example where this would fail, so I'm presenting this as a possible solution.\r\n\r\n**TODO**:\r\n- [x] Split fetching content type Ids for update and update query into two separate queries\r\n- [x] Introduce additional database indices.\r\n- [x] Fix new code according to Coding Standards (`$ composer fix-cs`).\r\n- [ ] Ask for Code Review.\r\n"},"comment":{"url":"https://api.github.com/repos/ezsystems/ezpublish-kernel/issues/comments/392460636","html_url":"https://github.com/ezsystems/ezpublish-kernel/pull/2339#issuecomment-392460636","issue_url":"https://api.github.com/repos/ezsystems/ezpublish-kernel/issues/2339","id":392460636,"user":{"login":"alongosz","id":7099219,"avatar_url":"https://avatars1.githubusercontent.com/u/7099219?v=4","gravatar_id":"","url":"https://api.github.com/users/alongosz","html_url":"https://github.com/alongosz","followers_url":"https://api.github.com/users/alongosz/followers","following_url":"https://api.github.com/users/alongosz/following{/other_user}","gists_url":"https://api.github.com/users/alongosz/gists{/gist_id}","starred_url":"https://api.github.com/users/alongosz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alongosz/subscriptions","organizations_url":"https://api.github.com/users/alongosz/orgs","repos_url":"https://api.github.com/users/alongosz/repos","events_url":"https://api.github.com/users/alongosz/events{/privacy}","received_events_url":"https://api.github.com/users/alongosz/received_events","type":"User","site_admin":false},"created_at":"2018-05-28T08:38:18Z","updated_at":"2018-05-28T08:38:18Z","author_association":"MEMBER","body":"> Like we recently discussed on ezxmltext package, we can avoid having the quoting and make it more readable by doing something like:\r\n> ```php\r\n>    ->select('t.contentobject_id')\r\n>    ->from('ezcontentobject_tree', 't')\r\n> ```\r\n\r\n@andrerom ok, changed. I keep quoting for consistency with other parts of the system - the ones that work on names that could potentially be DBMS keywords. In such case quoting is needed at least on `from` no matter what. But indeed here there's no such risk.\r\n\r\n> same below\r\n\r\nRemoved quoting as well, but to clarify one thing: for updates such aliasing as the one above will not work. Not every DBMS supports that syntax (I only mention this because Doctrine allows it, which makes tests crash on sqlite :-) ).\r\n\r\n@glye \r\n> This also adds a return value to setSectionForSubtree() where previously there was none despite doc, a nice bonus.\r\n\r\nYep, I can't look at such things, I have to fix that :grinning:  The same goes for long array syntax. \r\nSome people have to wash dirty dishes when they see it, some people have to fix unrelated code ;)"}}
{"actor":{"display_login":"alongosz"}}