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
