Re: [PoC] Improve dead tuple storage for lazy vacuum
Masahiko Sawada <sawada.mshk@gmail.com>
On Mon, Dec 12, 2022 at 7:14 PM John Naylor <john.naylor@enterprisedb.com> wrote: > > > On Fri, Dec 9, 2022 at 8:33 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > > > On Fri, Dec 9, 2022 at 5:53 PM John Naylor <john.naylor@enterprisedb.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
Commits
-
radixtree: Fix SIGSEGV at update of embeddable value to non-embeddable.
- bb7f195ff788 17.0 landed
-
Get rid of anonymous struct
- bf183f168c44 17.0 landed
-
Teach radix tree to embed values at runtime
- 0fe5f64367bc 17.0 landed
-
Teach TID store to skip bitmap for small numbers of offsets
- f35bd9bf359d 17.0 landed
-
Use bump context for TID bitmaps stored by vacuum
- 8a1b31e6e596 17.0 landed
-
Fix alignment of stack variable
- 0ea51bac3802 17.0 landed
-
Use TidStore for dead tuple TIDs storage during lazy vacuum.
- 667e65aac354 17.0 landed
-
Rethink create and attach APIs of shared TidStore.
- 2d8f56dabbfd 17.0 landed
-
Fix inconsistent function prototypes with function definitions.
- a0e22ef9114b 17.0 landed
-
Fix a calculation in TidStoreCreate().
- 4edb37e322a6 17.0 landed
-
Fix potential integer handling issue in radixtree.h.
- 80d5d4937c16 17.0 landed
-
Add TIDStore, to store sets of TIDs (ItemPointerData) efficiently.
- 30e144287a72 17.0 landed
-
Fix link error for test_radixtree module on Windows
- ab6ae6260372 17.0 landed
- 9552e3ace317 17.0 landed
-
Blind attempt to fix ODR violations
- 1f1d73a8b83f 17.0 landed
-
Fix incorrect format specifier for int64
- e444ebcb85c0 17.0 landed
-
Fix redefinition of typedefs
- ac234e6377dd 17.0 landed
-
Add template for adaptive radix tree
- ee1b30f128d8 17.0 landed
-
Fix signedness error in 9f225e992 for gcc
- de7c6fe8347a 17.0 landed
-
Introduce helper SIMD functions for small byte arrays
- 9f225e992bed 17.0 landed
-
Optimize vacuuming of relations with no indexes.
- c120550edb86 17.0 cited
-
Add bound check before bsearch() for performance
- bbaf315309ed 14.0 cited
-
Allocate consecutive blocks during parallel seqscans
- 56788d2156fc 14.0 cited