From: Antonin Houska <ah@cybertec.at>
To: Sami Imseih <samimseih@gmail.com>
cc: pgsql-hackers@postgresql.org
Subject: Re: Conflicting updates of command progress
In-reply-to: <CAA5RZ0vg3jAJSjQDZZdjz--sJCh0v1MVZ5Lpqh4F=Rn8vnVjOw@mail.gmail.com>
References: <117147.1744380740@localhost> <CAA5RZ0s_Hta6PWDwY9Lf-c+14F+C13BK2ax=B6R7bmDNhN6xdQ@mail.gmail.com> <17632.1745403972@localhost> <CAA5RZ0v5tYwsdSxQaFUh0xkHPnY62m55qZ=a7k6EewbP-zCMvA@mail.gmail.com> <4746.1745487225@localhost> <CAA5RZ0vg3jAJSjQDZZdjz--sJCh0v1MVZ5Lpqh4F=Rn8vnVjOw@mail.gmail.com>
Comments: In-reply-to Sami Imseih <samimseih@gmail.com>
   message dated "Thu, 24 Apr 2025 13:15:06 -0500."
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <25939.1745602488.1@localhost>
Content-Transfer-Encoding: quoted-printable
Date: Fri, 25 Apr 2025 19:34:48 +0200
Message-ID: <25940.1745602488@localhost>
Archived-At: <https://www.postgresql.org/message-id/25940.1745602488%40localhost>
Precedence: bulk

Sami Imseih <samimseih@gmail.com> wrote:

> > pgstat_progress_start_command() is called twice: First with
> > cmdtype=3DPROGRESS_COMMAND_CLUSTER, second with
> > PROGRESS_COMMAND_CREATE_INDEX. The first happens in cluster_rel() the =
second
> > in cluster_rel() -> rebuild_relation() -> finish_heap_swap() ->
> > reindex_relation() -> reindex_index().
> >
> > It does not matter though, the current design only expects one command=
.
> =

> When I set a breakpoint on pgstat_progress_start_command and ran a
> CLUSTER on a table with a few indexes, I only saw start_command being
> called once for PROGRESS_COMMAND_CLUSTER. Then I went back in the
> code and realized that reindex_index when called via the CLUSTER command
> has progress set to false. So as it stands now, only PROGRESS_COMMAND_CL=
USTER
> is effectively started when CLUSTER is called.
> =

> ```
> if (progress)
> {
> const int progress_cols[] =3D {
> PROGRESS_CREATEIDX_COMMAND,
> PROGRESS_CREATEIDX_INDEX_OID
> };
> const int64 progress_vals[] =3D {
> PROGRESS_CREATEIDX_COMMAND_REINDEX,
> indexId
> };
> =

> pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
> heapId);
> pgstat_progress_update_multi_param(2, progress_cols, progress_vals);
> }

Ah, got it. So for now the REPACK CONCURRENTLY patch only needs to turn of=
f
the progress reporting for PROGRESS_COMMAND_CREATE_INDEX properly. Thanks =
for
checking!

-- =

Antonin Houska
Web: https://www.cybertec-postgresql.com