Re: pgsql: Add TIDStore, to store sets of TIDs (ItemPointerData) efficientl

On Tue, Mar 26, 2024 at 7:16 AM David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Thu, 21 Mar 2024 at 14:10, Masahiko Sawada <msawada@postgresql.org> wrote:
> >
> > Add TIDStore, to store sets of TIDs (ItemPointerData) efficiently.
>
> > src/backend/access/common/tidstore.c               | 463 +++++++++++++++++++++
>
> I was looking at this code and I saw the following:
>
> /* choose the maxBlockSize to be no larger than 1/16 of max_bytes */
> while (16 * maxBlockSize > max_bytes * 1024L)
>     maxBlockSize >>= 1;
>
> I saw the "* 1024L" and was confused by it.  "max_bytes * 1024L"
> converts the number of bytes into "millibytes" ([1]), which I don't
> think is correct.
>
> Either "max_bytes" is a bad name for this variable or the * 1024L
> should be removed.
>

Right. We discussed it on the original thread too[1]. Since we're
going to change the create API of TidStore, I'm going to fix it along
with the API changes[2]. Even if we don't make the API change, I'll
fix it individually.

Regards,

[1] https://www.postgresql.org/message-id/CANWCAZZTE-14ofsucofTuhFsfuDGBNf%3DNZb22TMYT8bxA41oQQ%40mail.gmail.com
[2] https://www.postgresql.org/message-id/CAD21AoAxXGf7FbuA9jvx%2BXUFrEJUz9JNOmkxF8SLppOOOvhBtg%40mail.gmail.com
Please see v79-0003-Rethink-create-and-attach-APIs-of-shared-TidStor.patch.

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