Horton
Menu

Re: brininsert optimization opportunity

Soumyadeep Chakraborty <soumyadeep2007@gmail.com>

View thread
From: Soumyadeep Chakraborty <soumyadeep2007@gmail.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, pgsql-hackers <pgsql-hackers@postgresql.org>, Ashwin Agrawal <ashwinstar@gmail.com>
Date: 2023-07-05T18:57:58Z
Lists: pgsql-hackers

Attachments

On Wed, Jul 5, 2023 at 3:16 AM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:

> > I'll try this out and introduce a couple of new index AM callbacks. I
> > think it's best to do it before releasing the locks - otherwise it
> > might be weird
> > to manipulate buffers of an index relation, without having some sort of lock on
> > it. I'll think about it some more.
> >
>
> I don't understand why would this need more than just a callback to
> release the cache.

We wouldn't. I thought that it would be slightly cleaner and slightly more
performant if we moved the (if !state) branches out of the XXXinsert()
functions.
But I guess, let's minimize the changes here. One cleanup callback is enough.

> > PS: It should be possible to make GIN and GiST use the new index AM APIs
> > as well.
> >
>
> Why should GIN/GiST use the new API? I think it's perfectly sensible to
> only require the "cleanup callback" when just pfree() is not enough.

Yeah no need.

Attached v3 of the patch w/ a single index AM callback.

Regards,
Soumyadeep (VMware)

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add missing index_insert_cleanup calls

  2. Fix a couple typos in BRIN code

  3. Check if ii_AmCache is NULL in aminsertcleanup

  4. Use fipshash in brin_multi test

  5. Reuse BrinDesc and BrinRevmap in brininsert

  6. Consider fillfactor when estimating relation size

  7. Postpone some stuff out of ExecInitModifyTable.