{"issue":{"pull_request":{"diff_url":"https://github.com/hpcc-systems/HPCC-Platform/pull/941.diff","html_url":"https://github.com/hpcc-systems/HPCC-Platform/pull/941","patch_url":"https://github.com/hpcc-systems/HPCC-Platform/pull/941.patch"},"state":"open","user":{"avatar_url":"https://secure.gravatar.com/avatar/9cdedd9001bac05ea2ab5823d32afc89?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-140.png","gravatar_id":"9cdedd9001bac05ea2ab5823d32afc89","url":"https://api.github.com/users/rengolin","login":"rengolin","id":1040968},"milestone":null,"assignee":null,"labels":[],"updated_at":"2011-11-10T19:49:24Z","comments":5,"closed_at":null,"url":"https://api.github.com/repos/hpcc-systems/HPCC-Platform/issues/941","body":"#919 added a regression, where superfile1.ecl would lock. The cause was the locking of sub-files, when superfiles had common sub-files.\r\n\r\nFixing the locking issue, there was another regression on the same file, where the deletion wouldn't delete the sub-file when that was expected, since the transaction existed, but was paused (no idea why yet).\r\n\r\nThe code is less elegant and should be refactored as soon as possible.\r\n\r\nNo regressions found on daregress nor runregress on hthor & thor suites.\r\n\r\nThis pull request fixes bug #927.","html_url":"https://github.com/hpcc-systems/HPCC-Platform/issues/941","title":"Fix 919 regression in superfile1.ecl","number":941,"id":2199824,"created_at":"2011-11-10T16:37:54Z"},"action":"created","comment":{"user":{"avatar_url":"https://secure.gravatar.com/avatar/8eff4f840b1605e58ce34b0c3b5eea5a?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-140.png","gravatar_id":"8eff4f840b1605e58ce34b0c3b5eea5a","url":"https://api.github.com/users/jakesmith","login":"jakesmith","id":902700},"updated_at":"2011-11-10T19:49:24Z","url":"https://api.github.com/repos/hpcc-systems/HPCC-Platform/issues/comments/2700148","body":"Not at PC right now, my \"fix\" to swapSuper, is simple and I suspect what \r\nit was supposed to be.\r\nI agree the transaction mechanism needs refactoring, but in the \r\nmeantime, we need to plug the gaps, e.g. leaky meta info. and avoiding \r\ncommitting early, avoiding deadlocks etc.\r\n\r\nAs I say the fix I added to swapSuper is relatively simple and we take \r\nthis until we have a fully refactored mechanism.\r\nI have tested the superfile*.ecl regression tests which involve various \r\ntransactions etc. and all looks well.\r\n\r\n-- \r\nJake.\r\n\r\n\r\nOn 10/11/2011 19:37, Renato Golin wrote:\r\n> Hi Jake,\r\n>\r\n> I had gone for a different strategy, that is to fix the problems in the current code, current style, and then refactor everything when less critical problems are present.\r\n>\r\n> If we're going to do things right from now on, than I don't think neither my quick fix nor your patch are correct. Adding the transaction object to more and more methods will only uncover more and more bugs that weren't counting on it on a multitude of library users (like ECL, ESP, DFUplus) and I'd prefer to have that trouble when doing it right, rather than quick-fixing.\r\n>\r\n> Again, your fix had changes in the Owned class to bestow ownership \"the right way\" (tm). Which highlights the problem I was referring to on my email earlier this week. I think we should try using the Linked class instead, and see how things follow from there, and would like to make transactions mandatory for every file action.\r\n>\r\n> If the two bugs (clean super owner and remove super files within transactions) can wait until we do this refactoring, than I'm glad to start right now. If not, I'd prefer to have a half-baked (and wrong) fix for the sake of it. I'm not sure how long this refactoring will take (I'm guessing multi-week period), but I wouldn't count on it for this year, just to be on the safe side.\r\n>\r\n> ---\r\n> Reply to this email directly or view it on GitHub:\r\n> https://github.com/hpcc-systems/HPCC-Platform/pull/941#issuecomment-2699994\r\n\r\nThis message (including any attachments) contains confidential information intended for a specific individual and purpose, and is protected by law.  If you are not the intended recipient, you should delete this message.  Any disclosure, copying, or distribution of this message, or the taking of any action based on it, is strictly prohibited.","id":2700148,"created_at":"2011-11-10T19:49:24Z"}}