Stephen Frost <sfrost@snowman.net> writes:
> * Andres Freund (andres@2ndquadrant.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