Julien Rouhaud <julien.rouh...@dalibo.com> writes: > After some digging, the leak comes from walbufbegin palloc in > registerLeafRecompressWALData(). > IIUC, walbufbegin isn't pfree-d and can't be before XLogInsert() is > called, which happens in ginPlaceToPage().
Hmm. > I don't see a simple way to fix that. My first idea would be to change > GinBtreeData's placeToPage (and all other needed) functions signature to > pass a pointer to pfree in ginPlaceToPage() if needed, but it doesn't > really seem appealing. In any case, I'd be happy to work on a patch if > needed. > Also, in dataPlaceToPageLeaf() and ginVacuumPostingTreeLeaf(), shouldn't > the START_CRIT_SECTION() calls be placed before the xlog code? Yeah, they should. Evidently somebody kluged it to avoid doing a palloc inside a critical section, while ignoring every other rule about where to place critical sections. (Maybe we should put an assert about being within a critical section into XLogBeginInsert.) This code is pretty fundamentally broken, isn't it. elog() calls inside a critical section? Really? Even worse, a MemoryContextDelete, which hardly seems like something that should be assumed error-proof. Not to mention the unbelievable ugliness of a function that sometimes returns with an open critical section (and WAL insertion started) and sometimes doesn't. It kinda looks like this was hacked up without bothering to keep the comment block in ginPlaceToPage in sync with reality, either. I think this needs to be redesigned so that the critical section and WAL insertion calls all happen within a single straight-line piece of code. We could try making that place be ginPlaceToPage(). So far as registerLeafRecompressWALData is concerned, that does not look that hard; it could palloc and fill the WAL-data buffer the same as it's doing now, then pass it back up to be registered (and eventually pfreed) in ginPlaceToPage. However, that would also mean postponing the call of dataPlaceToPageLeafRecompress(), which seems much messier. I assume the data it's working from is mostly in the tmpCtx that dataPlaceToPageLeaf wants to free before returning. Maybe we could move creation/destruction of the tmpCtx out to ginPlaceToPage, as well? The other line of thought is to move the WAL work that ginPlaceToPage does down into the subroutines. That would probably result in some duplication of code, but it might end up being a cleaner solution. Anyway, I think memory leakage is just the start of what's broken here, and fixing it won't be a very small patch. regards, tom lane -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers