Justin Pryzby <pryzby@telsasoft.com>

From: Alexander Pyhalov <a(dot)pyhalov(at)postgrespro(dot)ru>
To: Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Justin Pryzby <pryzby@telsasoft.com>
Date: 2022-02-09 12:18:12
Message-ID: e47898da6a354262ec332ca073847c0a@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi.

I've looked at patches, introducing CREATE INDEX CONCURRENTLY for
partitioned tables -
https://www.postgresql.org/message-id/flat/20210226182019.GU20769%40telsasoft.com#da169a0a518bf8121604437d9ab053b3
.
The thread didn't have any activity for a year.

I've rebased patches and tried to fix issues I've seen. I've fixed
reference after table_close() in the first patch (can be seen while
building with CPPFLAGS='-DRELCACHE_FORCE_RELEASE'). Also merged old
0002-f-progress-reporting.patch and
0003-WIP-Add-SKIPVALID-flag-for-more-integration.patch. It seems the
first one didn't really fixed issue with progress report (as
ReindexRelationConcurrently() uses pgstat_progress_start_command(),
which seems to mess up the effect of this command in DefineIndex()).
Also third patch completely removes attempts to report create index
progress correctly (reindex reports about individual commands, not the
whole CREATE INDEX).

So I've added 0003-Try-to-fix-create-index-progress-report.patch, which
tries to fix the mess with create index progress report. It introduces
new flag REINDEXOPT_REPORT_PART to ReindexParams->options. Given this
flag, ReindexRelationConcurrently() will not report about individual
operations, but ReindexMultipleInternal() will report about reindexed
partitions. To make the issue worse, some partitions can be handled in
ReindexPartitions() and ReindexMultipleInternal() should know how many
to correctly update PROGRESS_CREATEIDX_PARTITIONS_DONE counter, so we
pass the number of handled partitions to it.

I also have question if in src/backend/commands/indexcmds.c:1239
1240 oldcontext = MemoryContextSwitchTo(ind_context);
1239 childidxs = RelationGetIndexList(childrel);
1241 attmap =
1242
build_attrmap_by_name(RelationGetDescr(childrel),
1243 parentDesc);
1244 MemoryContextSwitchTo(oldcontext);

should live in ind_context, given that we iterate over this list of oids
and immediately free it, but at least it shouldn't do much harm.
--
Best regards,
Alexander Pyhalov,
Postgres Professional

Attachment Content-Type Size
0001-Allow-CREATE-INDEX-CONCURRENTLY-on-partitioned-table.patch text/x-diff 14.3 KB
0002-Add-SKIPVALID-flag-for-more-integration.patch text/x-diff 3.9 KB
0003-Try-to-fix-create-index-progress-report.patch text/x-diff 10.4 KB
0004-ReindexPartitions-to-set-indisvalid.patch text/x-diff 2.4 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Pyhalov 2022-02-09 12:20:54 CREATE INDEX CONCURRENTLY on partitioned index
Previous Message Ashutosh Sharma 2022-02-09 12:01:02 Re: Make mesage at end-of-recovery less scary.