| From: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
|---|---|
| To: | Antonin Houska <ah(at)cybertec(dot)at> |
| Cc: | Mihail Nikalayeu <mihailnikalayeu(at)gmail(dot)com>, Srinath Reddy Sadipiralla <srinath2133(at)gmail(dot)com>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Robert Treat <rob(at)xzilla(dot)net> |
| Subject: | Re: Adding REPACK [concurrently] |
| Date: | 2026-03-27 17:01:09 |
| Message-ID: | 202603271635.owyhm7btgoic@alvherre.pgsql |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 2026-Mar-27, Antonin Houska wrote:
> I tried to write a patch that allows progress tracking of two commands at the
> same time (a "main command" and a "subcommand"), but regression tests revealed
> that contrib/file_fdw is broken in a way that I could not fix easily: during
> execution of a join, two COPY FROM commands are executed at the same time and
> they overwrite the status of each other. Unlike the concept of a sub-command,
> we cannot assume here that the command that started the reporting as the
> second will stop as the first. Thus in pgstat_progress_end_command() we cannot
> figure out which node is trying to stop the reporting.
Yeah, I think we've discussed this kind of thing in the context of the
index creation by CLUSTER before, but no ideas have emerged on what the
best implementation is.
> It needs more work, I can get back to it after PG 19 feature freeze.
Yeah, clearly it's not getting in pg19. I'm not terribly upset about
that though; it would be nice to have, but it's not a dealbreaker.
Anyway, I've been changing how all the new code for REPACK is
distributed; here's patch series v45, and the breakdown and my
assessment is:
0001 is looking good. I don't commit it just yet because there's no
point in doing so until 0003 is also committable.
0002 continues to baffle me. I would much rather do without it.
0003 is the addition of CONCURRENTLY and all it needs. Regarding
structure, I think it's good to have the code that runs in a separate
worker have its own file, which I named commands/repack_worker.c.
Luckily it's all well contained. It shares data structures and whatnot
with the other parts of REPACK via include/commands/repack_internal.h.
Patch "Use BulkInsertState when copying data to the new heap." is now
0004 and I put it right after 0003, and I will probably be squashing
them into a single one.
0005 is new -- I renamed cluster.c to repack.c, as previously mentioned.
Then 0006 is "Fix a few problems in index build progress reporting",
which I'm likely to also squash in 0003 for v46.
0007 is "Error out any process that would block at REPACK" which is the
complete patch to change the deadlock detector to raise an error at any
process that competes with REPACK. This one I think I would commit
separately from 0003 -- if nothing else, because it introduces novel
behavior that could be contested, so I want to be able to easily revert
it without endangering the rest of CONCURRENTLY.
Lastly, 0008 is "Teach snapshot builder to skip transactions running
REPACK (CONCURRENTLY)" which I see as the least mature of the pack. I
would really like to be able to squash it with 0003, but I'm not yet
trusting it enough.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"No nos atrevemos a muchas cosas porque son difíciles,
pero son difíciles porque no nos atrevemos a hacerlas" (Séneca)
| Attachment | Content-Type | Size |
|---|---|---|
| v45-0001-Make-index_concurrently_create_copy-more-general.patch | text/x-diff | 9.4 KB |
| v45-0002-Do-not-dereference-varattrib_4b-in-VARSIZE_4B.patch | text/x-diff | 1.9 KB |
| v45-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch | text/x-diff | 183.3 KB |
| v45-0004-Use-BulkInsertState-when-copying-data-to-the-new.patch | text/x-diff | 6.7 KB |
| v45-0005-rename-cluster.c-h-to-repack.c-h.patch | text/x-diff | 9.1 KB |
| v45-0006-Fix-a-few-problems-in-index-build-progress-repor.patch | text/x-diff | 7.4 KB |
| v45-0007-Error-out-any-process-that-would-block-at-REPACK.patch | text/x-diff | 12.6 KB |
| v45-0008-Teach-snapshot-builder-to-skip-transactions-runn.patch | text/x-diff | 18.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jeff Davis | 2026-03-27 17:01:48 | Re: Expanding HOT updates for expression and partial indexes |
| Previous Message | Lucas DRAESCHER | 2026-03-27 16:57:42 | Re: [Bug Report + Patch] File descriptor leak when io_method=io_uring |