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

On Wed, Feb 22, 2023 at 4:35 PM John Naylor
<john(dot)naylor(at)enterprisedb(dot)com> wrote:
>
>
> On Wed, Feb 22, 2023 at 1:16 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Mon, Feb 20, 2023 at 2:56 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > >
> > > Yeah, I did a similar thing in an earlier version of tidstore patch.
>
> Okay, if you had checks against the old array lookup in development, that gives us better confidence.
>
> > > Since we're trying to introduce two new components: radix tree and
> > > tidstore, I sometimes find it hard to investigate failures happening
> > > during lazy (parallel) vacuum due to a bug either in tidstore or radix
> > > tree. If there is a bug in lazy vacuum, we cannot even do initdb. So
> > > it might be a good idea to do such checks in USE_ASSERT_CHECKING (or
> > > with another macro say DEBUG_TIDSTORE) builds. For example, TidStore
> > > stores tids to both the radix tree and array, and checks if the
> > > results match when lookup or iteration. It will use more memory but it
> > > would not be a big problem in USE_ASSERT_CHECKING builds. It would
> > > also be great if we can enable such checks on some bf animals.
> >
> > I've tried this idea. Enabling this check on all debug builds (i.e.,
> > with USE_ASSERT_CHECKING macro) seems not a good idea so I use a
> > special macro for that, TIDSTORE_DEBUG. I think we can define this
> > macro on some bf animals (or possibly a new bf animal).
>
> I don't think any vacuum calls in regression tests would stress any of this code very much, so it's not worth carrying the old way forward. I was thinking of only doing this as a short-time sanity check for testing a real-world workload.

I guess that It would also be helpful at least until the GA release.
People will be able to test them easily on their workloads or their
custom test scenarios.

Regards,

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