Thread: Justin Pryzby
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
Alexander Pyhalov писал 2022-02-09 15:18: > 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. Sorry, messed the topic. -- Best regards, Alexander Pyhalov, Postgres Professional
Hi Alexander, On Wed, Feb 09, 2022 at 03:18:12PM +0300, Alexander Pyhalov wrote: > 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. If you plan to review a patch set, could you reply directly on its thread without changing the subject please? This way of replying breaks the flow of a thread, making it harder to track. Thanks! -- Michael