Re: [PoC] Improve dead tuple storage for lazy vacuum

On Mon, Dec 12, 2022 at 7:14 PM John Naylor
<john(dot)naylor(at)enterprisedb(dot)com> wrote:
>
>
> On Fri, Dec 9, 2022 at 8:33 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Fri, Dec 9, 2022 at 5:53 PM John Naylor <john(dot)naylor(at)enterprisedb(dot)com> wrote:
> > >
>
> > > I don't think that'd be very controversial, but I'm also not sure why we'd need 4MB -- can you explain in more detail what exactly we'd need so that the feature would work? (The minimum doesn't have to work *well* IIUC, just do some useful work and not fail).
> >
> > The minimum requirement is 2MB. In PoC patch, TIDStore checks how big
> > the radix tree is using dsa_get_total_size(). If the size returned by
> > dsa_get_total_size() (+ some memory used by TIDStore meta information)
> > exceeds maintenance_work_mem, lazy vacuum starts to do index vacuum
> > and heap vacuum. However, when allocating DSA memory for
> > radix_tree_control at creation, we allocate 1MB
> > (DSA_INITIAL_SEGMENT_SIZE) DSM memory and use memory required for
> > radix_tree_control from it. das_get_total_size() returns 1MB even if
> > there is no TID collected.
>
> 2MB makes sense.
>
> If the metadata is small, it seems counterproductive to count it towards the total. We want the decision to be driven by blocks allocated. I have an idea on that below.
>
> > > Remember when we discussed how we might approach parallel pruning? I envisioned a local array of a few dozen kilobytes to reduce contention on the tidstore. We could use such an array even for a single worker (always doing the same thing is simpler anyway). When the array fills up enough so that the next heap page *could* overflow it: Stop, insert into the store, and check the store's memory usage before continuing.
> >
> > Right, I think it's no problem in slab cases. In DSA cases, the new
> > segment size follows a geometric series that approximately doubles the
> > total storage each time we create a new segment. This behavior comes
> > from the fact that the underlying DSM system isn't designed for large
> > numbers of segments.
>
> And taking a look, the size of a new segment can get quite large. It seems we could test if the total DSA area allocated is greater than half of maintenance_work_mem. If that parameter is a power of two (common) and >=8MB, then the area will contain just under a power of two the last time it passes the test. The next segment will bring it to about 3/4 full, like this:
>
> maintenance work mem = 256MB, so stop if we go over 128MB:
>
> 2*(1+2+4+8+16+32) = 126MB -> keep going
> 126MB + 64 = 190MB -> stop
>
> That would be a simple way to be conservative with the memory limit. The unfortunate aspect is that the last segment would be mostly wasted, but it's paradise compared to the pessimistically-sized single array we have now (even with Peter G.'s VM snapshot informing the allocation size, I imagine).

Right. In this case, even if we allocate 64MB, we will use only 2088
bytes at maximum. So I think the memory space used for vacuum is
practically limited to half.

>
> And as for minimum possible maintenance work mem, I think this would work with 2MB, if the community is okay with technically going over the limit by a few bytes of overhead if a buildfarm animal set to that value. I imagine it would never go over the limit for realistic (and even most unrealistic) values. Even with a VM snapshot page in memory and small local arrays of TIDs, I think with this scheme we'll be well under the limit.

Looking at other code using DSA such as tidbitmap.c and nodeHash.c, it
seems that they look at only memory that are actually dsa_allocate'd.
To be exact, we estimate the number of hash buckets based on work_mem
(and hash_mem_multiplier) and use it as the upper limit. So I've
confirmed that the result of dsa_get_total_size() could exceed the
limit. I'm not sure it's a known and legitimate usage. If we can
follow such usage, we can probably track how much dsa_allocate'd
memory is used in the radix tree. Templating whether or not to count
the memory usage might help avoid the overheads.

> After this feature is complete, I think we should consider a follow-on patch to get rid of vacuum_work_mem, since it would no longer be needed.

I think you meant autovacuum_work_mem. Yes, I also think we can get rid of it.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com