Re: Parallel CREATE INDEX for BRIN indexes

From: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Parallel CREATE INDEX for BRIN indexes
Date: 2024-04-13 21:04:33
Message-ID: 1df00a66-db5a-4e66-809a-99b386a06d86@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 4/13/24 11:19, Tomas Vondra wrote:
> On 4/13/24 10:36, Andres Freund wrote:
>> Hi,
>>
>> While preparing a differential code coverage report between 16 and HEAD, one
>> thing that stands out is the parallel brin build code. Neither on
>> coverage.postgresql.org nor locally is that code reached during our tests.
>>
>
> Thanks for pointing this out, it's definitely something that I need to
> improve (admittedly, should have been part of the patch). I'll also look
> into eliminating the difference between BTREE and BRIN parallel builds,
> mentioned in my last message in this thread.
>

Here's a couple patches adding a test for the parallel CREATE INDEX with
BRIN. The actual test is 0003/0004 - I added the test to pageinspect,
because that allows cross-checking the index to one built without
parallelism, which I think is better than just doing CREATE INDEX
without properly testing it produces correct results.

It's not entirely trivial because for some opclasses (e.g. minmax-multi)
the results depend on the order in which values are added, and order in
which summaries from different workers are merged.

Funnily enough, while adding the test, I ran into two pre-existing bugs.
One is that brin_bloom_union forgot to update the number of bits set in
the bitmap, another one is that 6bcda4a721 changes PG_DETOAST_DATUM to
the _PACKED version, which however does the wrong thing. Both of which
are mostly harmless - it only affects the output function, which is
unused outside pageinspect. No impact on query correctness etc.

The test needs a bit more work to make sure it works on 32-bit machines
etc. which I think may affect available space on a page, which in turn
might affect the minmax-multi summaries. But I'll take care this early
next week.

Funnily

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
v20240413-0001-Update-nbits_set-in-brin_bloom_union.patch text/x-patch 1.2 KB
v20240413-0002-Use-correct-PG_DETOAST_DATUM-macro-in-BRIN.patch text/x-patch 1.9 KB
v20240413-0003-Add-regression-tests-for-BRIN-parallel-bui.patch text/x-patch 11.1 KB
v20240413-0004-add-minmax-multi-to-the-regression-test.patch text/x-patch 8.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dima Rybakov (Tlt) 2024-04-13 22:58:21 Why should I use fileno(stderr) instead of STDERR_FILENO
Previous Message Nathan Bossart 2024-04-13 19:44:50 Re: allow changing autovacuum_max_workers without restarting