Re: mem context is not reset between extended stats

On 9/21/21 3:37 AM, Justin Pryzby wrote:
> On Tue, Sep 21, 2021 at 02:15:45AM +0200, Tomas Vondra wrote:
>> On 9/15/21 10:09 PM, Justin Pryzby wrote:
>>> Memory allocation appeared be O(1) WRT the number of statistics objects, which
>>> was not expected to me. This is true in v13 (and probably back to v10).
>
> Of course I meant to say that it's O(N) and not O(1) :)
>

Sure, I got that ;-)

>> In principle we don't expect too many extended statistics on a single table,
>
> Yes, but note that expression statistics make it more reasonable to have
> multiple extended stats objects. I noticed this while testing a patch to build
> (I think) 7 stats objects on each of our current month's partitions.
> autovacuum was repeatedly killed on this vm after using using 2+GB RAM,
> probably in part because there were multiple autovacuum workers handling the
> most recent batch of inserted tables.
>
> First, I tried to determine what specifically was leaking so badly, and
> eventually converged to this patch. Maybe there's additional subcontexts which
> would be useful, but the minimum is to reset between objects.
>

Agreed.

I don't think there's much we could release, given the current design,
because we evaluate (and process) all expressions at once. We might
evaluate/process them one by one (and release the memory), but only when
no other statistics kinds are requested. That seems pretty futile.

>> These issues exist pretty much since PG10, which is where extended stats
>> were introduced, so we'll have to backpatch it. But there's no rush and I
>> don't want to interfere with rc1 at the moment.
>
> Ack that. It'd be *nice* if if the fix were included in v14.0, but I don't
> know the rules about what can change after rc1.
>

IMO this is a bugfix, and I'll get it into 14.0 (and backpatch). But I
don't want to interfere with the rc1 tagging and release, so I'll do
that later this week.

>> Attached are two patches - 0001 is your patch (seems fine, but I looked only
>> very briefly) and 0002 is the context reset I proposed.
>
> I noticed there seems to be a 3rd patch available, which might either be junk
> for testing or a cool new feature I'll hear about later ;)
>

Haha! Nope, that was just an experiment with doubling the repalloc()
sizes in functional dependencies, instead of growing them in tiny
chunks. but it does not make a measurable difference, so I haven't
included that.

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company