* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> 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.
Well, sure, which is why I didn't actually do that- but I did end up
having to make it resize when necessary, which isn't entirely ideal
either.
> Besides which, it's not like there is *no* cleanup for
> GetRunningTransactionData --- it has a lock that has to be released ...
That's true.. I guess my general feeling is that it'd be good to do
this all one way or the other- having it use a static variable into
which we stick the pointer to some reused space for one and then doing a
palloc for the other which needs to be pfree'd struck me as odd.
> 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?
Certainly good questions and better comments would have helped here. I
can go back and rework the patch either way.
Thanks,
Stephen