On Wed, Feb 22, 2023 at 1:16 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Mon, Feb 20, 2023 at 2:56 PM Masahiko Sawada <sawada.m...@gmail.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. -- John Naylor EDB: http://www.enterprisedb.com