| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | Stephen Frost <sfrost(at)snowman(dot)net> |
| Cc: | Andres Freund <andres(at)2ndquadrant(dot)com>, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, Naoya Anzai <anzai-naoya(at)mxu(dot)nes(dot)nec(dot)co(dot)jp>, pgsql-bugs(at)postgresql(dot)org, Akio Iwaasa <iwaasa(at)mxs(dot)nes(dot)nec(dot)co(dot)jp> |
| Subject: | Re: Memory-leak in BackgroundWriter(and Checkpointer) |
| Date: | 2013-06-04 17:53:03 |
| Message-ID: | 28228.1370368383@sss.pgh.pa.us |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Andres Freund (andres(at)2ndquadrant(dot)com) wrote:
>> Seems more consistent with the rest of the code too. But anyway, I am
>> fine with fixing it either way.
> And this is really the other point- having LogStandbySnapshot() need to
> clean up after GetRunningTransactionLocks() but not
> GetRunningTransactionData() would strike me as very odd.
Meh. I'm not impressed with permanently allocating an array large
enough to hold all the locks GetRunningTransactionLocks
might return --- that's potentially much larger than the other array,
and in fact I don't think we have a hard limit on its size at all.
Besides which, it's not like there is *no* cleanup for
GetRunningTransactionData --- it has a lock that has to be released ...
I think the proposed fix is fine code-wise; the real problem here is
crummy commenting. GetRunningTransactionLocks isn't documented as
returning a palloc'd array, and why the heck do we have a long comment
about its implementation in LogStandbySnapshot?
regards, tom lane
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Stephen Frost | 2013-06-04 18:02:29 | Re: Memory-leak in BackgroundWriter(and Checkpointer) |
| Previous Message | Joshua Berry | 2013-06-04 15:48:33 | Re: Segmentation Fault in Postgres server when using psqlODBC |