I had the possibility to perform tests on 9.5, and can confirm the memory leak I was seeing is solved with the patch (and that's great :) )
Regards Marc On 18/04/2016 17:53, Julien Rouhaud wrote: > On 18/04/2016 16:33, Tom Lane wrote: >> I poked at this over the weekend, and got more unhappy the more I poked. >> Aside from the memory leakage issue, there are multiple coding-rule >> violations besides the one you noted about scope of the critical sections. >> One example is that in the page-split path for an inner page, we modify >> the contents of childbuf long before getting into the critical section. >> The number of out-of-date comments was depressingly large as well. >> >> I ended up deciding that the most reasonable fix was along the lines of >> my first alternative above. In the attached draft patches, the >> "placeToPage" method is split into two methods, "beginPlaceToPage" and >> "execPlaceToPage", where the second method is what's supposed to happen >> inside the critical section for a non-page-splitting update. Nothing >> that's done inside beginPlaceToPage is critical. >> >> I've tested this to the extent of verifying that it passes make >> check-world and stops the memory leak in your test case, but it could use >> more eyeballs on it. >> > Thanks! I also started working on it but it was very far from being > complete (and already much more ugly...). > > I couldn't find any problem in the patch. > > I wonder if asserting being in a critical section would be valuable in > the *execPlaceToPage functions, or that (leaf->walinfolen > 0) in > dataExecPlaceToPageLeaf(), since it's filled far from this function. > >> Attached are draft patches against HEAD and 9.5 (they're the same except >> for BufferGetPage calls). I think we'd also want to back-patch to 9.4, >> but that will take considerable additional work because of the XLOG API >> rewrite that happened in 9.5. I also noted that some of the coding-rule >> violations seem to be new in 9.5, so the problems may be less severe in >> 9.4 --- the memory leak definitely exists there, though. >> -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers