Hi~ Thanks for looking.
> DropRelationFiles() is also called by FinishPreparedTransaction(). At > first I thought that might be a problem too, but looking a bit more > closely and trying it out... if a prepared transaction dropped a > table, then it called RelationDropStorage(), RelationCloseSmgr(), > smgrunpin(), and I can't immediately think of any way to repin it > while the relation is locked, so you can't break the assertion about > that in smgrdestroy(), where we make sure there are no Relation > objects with dangling references. It's already left unpinned or never > pinned and later destroyed in both DROP; PREPARE TRANSACTION; ... > COMMIT PREPARED; and CREATE; PREPARE TRANSACTION; ... ROLLBACK > PREPARED sequences, with different details. Another call of DropRelationFiles seems to have been handled by AtEOXact_SMgr, so there is no leak w/o patch. I have pgbench'ed in one connection for 10 min with CREATE; PREPARE TRANSACTION; ... ROLLBACK PREPARED; sequence and there is no rising of RES. > Now I'm left wondering if two-phase commit should do this explicitly > or not. For the isRedo case it seems clear, it was the intention of > 21d9c3ee to destroy it on commit/abort, which must be here I think. > The following description from the commit message *probably* also fits > the two-phase case, even though it already doesn't leak without it. > It would be good to get a comment explaining the new smgrdestroy() > call, and point to where our tests exercise all relevant cases. Explicit call seems to be a duplication if AtEOXact_SMgr can already handle? Should be do it only for redo? Some comment has been added to smgrdestroy() to define when it should be called inside v2 patch. > it's not immediately obvious how to introspect the cached state to > verify that the memory isn't leaked. It sure is...The way I'm using is benching for a long time to see if RES is rising… — Regards, Jingtang
v2-0001-Fix-SMgrRelation-object-memory-leak-in-DropRelationF.patch
Description: Binary data
