Re: Memory leak of SMgrRelation object on standby

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