Rename PqMsg_Progress to PqMsg_ParallelWorkerProgress
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253732psql -h localhost -U postgresBuilt from patchset v7 (message #7), September 10, 2026 at 05:21 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253732_7 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253732_7 && git checkout t253732_7Patchset v7 (message #7) is on t253732_7
While looking at this, I realized that "PqMsg_Progress" is the name of the
'P' message that was added for parallel index progress reporting.
I missed [1]/messages/by-id/ZpboPlg1v38vJhsD@nathan at the time, but if there is still a chance to rename it to
"PqMsg_ParallelWorkerProgress", that seems better,
"PqMsg_Progress" does not really describe what it is, whereas
"PqMsg_ParallelWorkerProgress" makes it explicit that this is a
parallel-worker-to-leader message. It also matches other places in the code
that deal with parallel workers, such as "IsParallelWorker", so it is easier
to find when looking through parallel-worker code.
[1]: /messages/by-id/ZpboPlg1v38vJhsD@nathan
--
Sami Imseih
Amazon Web Services (AWS)
Sami Imseih <samimseih.pg@gmail.com> writes:
While looking at this, I realized that "PqMsg_Progress" is the name of the
'P' message that was added for parallel index progress reporting.
I missed [1] at the time, but if there is still a chance to rename it to
"PqMsg_ParallelWorkerProgress", that seems better,
"PqMsg_Progress" does not really describe what it is, whereas
"PqMsg_ParallelWorkerProgress" makes it explicit that this is a
parallel-worker-to-leader message. It also matches other places in the code
that deal with parallel workers, such as "IsParallelWorker", so it is easier
to find when looking through parallel-worker code.
Yeah, "PqMsg_Progress" seems unduly generic. I don't think we should
rename it in the back branches, but doing so in master looks like
it'd be a small-footprint change.
regards, tom lane
On Thu, 10 Sept 2026 at 02:57, Sami Imseih <samimseih.pg@gmail.com> wrote:
While looking at this, I realized that "PqMsg_Progress" is the name of the
'P' message that was added for parallel index progress reporting.I missed [1] at the time, but if there is still a chance to rename it to
"PqMsg_ParallelWorkerProgress", that seems better,
"PqMsg_Progress" does not really describe what it is, whereas
"PqMsg_ParallelWorkerProgress" makes it explicit that this is a
parallel-worker-to-leader message. It also matches other places in the code
that deal with parallel workers, such as "IsParallelWorker", so it is easier
to find when looking through parallel-worker code.[1] /messages/by-id/ZpboPlg1v38vJhsD@nathan
--
Sami Imseih
Amazon Web Services (AWS)
Well, that's kind of bikeshedding for me to start with. But if we do,
maybe a better option will be to describe what this exactly does, not
who sends it. Like PgMsg_StatProgressReport or something. Does it
sound?
--
Best regards,
Kirill Reshke
Kirill Reshke <reshkekirill@gmail.com> wrote:
On Thu, 10 Sept 2026 at 02:57, Sami Imseih <samimseih.pg@gmail.com> wrote:
While looking at this, I realized that "PqMsg_Progress" is the name of the
'P' message that was added for parallel index progress reporting.I missed [1] at the time, but if there is still a chance to rename it to
"PqMsg_ParallelWorkerProgress", that seems better,
"PqMsg_Progress" does not really describe what it is, whereas
"PqMsg_ParallelWorkerProgress" makes it explicit that this is a
parallel-worker-to-leader message. It also matches other places in the code
that deal with parallel workers, such as "IsParallelWorker", so it is easier
to find when looking through parallel-worker code.[1] /messages/by-id/ZpboPlg1v38vJhsD@nathan
--
Sami Imseih
Amazon Web Services (AWS)Well, that's kind of bikeshedding for me to start with. But if we do,
maybe a better option will be to describe what this exactly does, not
who sends it. Like PgMsg_StatProgressReport or something. Does it
sound?
My objection to PqMsg_ParallelWorkerProgress is that other kinds of workers
might use it too, so PqMsg_WorkerProgress would make more sense. Anyway, your
suggestion sounds good to me as well because the existing message types do not
(except for PqMsg_BackendKeyData) use the name to indicate the sender.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
On Wed, Sep 09, 2026 at 06:35:55PM -0400, Tom Lane wrote:
Sami Imseih <samimseih.pg@gmail.com> writes:
While looking at this, I realized that "PqMsg_Progress" is the name of the
'P' message that was added for parallel index progress reporting.I missed [1] at the time, but if there is still a chance to rename it to
"PqMsg_ParallelWorkerProgress", that seems better,
"PqMsg_Progress" does not really describe what it is, whereas
"PqMsg_ParallelWorkerProgress" makes it explicit that this is a
parallel-worker-to-leader message. It also matches other places in the code
that deal with parallel workers, such as "IsParallelWorker", so it is easier
to find when looking through parallel-worker code.Yeah, "PqMsg_Progress" seems unduly generic. I don't think we should
rename it in the back branches, but doing so in master looks like
it'd be a small-footprint change.
All of the options I've seen listed in this thread so far involve changing
the part after the PqMsg_ prefix, but given we now have PqReplMsg_ and
PqBackupMsg_, I'm thinking it'd be better to give parallel worker messages
their own prefix. That seems to act as a sort of class or namespace for
the message. Maybe something like PqParallelMsg_Progress would work here.
--
nathan
On Thu, Sep 10, 2026 at 03:20:12PM +0200, Antonin Houska wrote:
Kirill Reshke <reshkekirill@gmail.com> wrote:
Well, that's kind of bikeshedding for me to start with. But if we do,
maybe a better option will be to describe what this exactly does, not
who sends it. Like PgMsg_StatProgressReport or something. Does it
sound?My objection to PqMsg_ParallelWorkerProgress is that other kinds of workers
might use it too, so PqMsg_WorkerProgress would make more sense. Anyway, your
suggestion sounds good to me as well because the existing message types do not
(except for PqMsg_BackendKeyData) use the name to indicate the sender.
I don't think we need to worry too much about other kinds of workers in
this case. The message is already listed under a comment that says it's
for parallel workers. Presumably we'd add new macros for things that other
types of workers send.
--
nathan
Hi,
Thanks for the feedback!
On Thu, Sep 10, 2026 at 9:30 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
On Wed, Sep 09, 2026 at 06:35:55PM -0400, Tom Lane wrote:
Sami Imseih <samimseih.pg@gmail.com> writes:
While looking at this, I realized that "PqMsg_Progress" is the name of the
'P' message that was added for parallel index progress reporting.I missed [1] at the time, but if there is still a chance to rename it to
"PqMsg_ParallelWorkerProgress", that seems better,
"PqMsg_Progress" does not really describe what it is, whereas
"PqMsg_ParallelWorkerProgress" makes it explicit that this is a
parallel-worker-to-leader message. It also matches other places in the code
that deal with parallel workers, such as "IsParallelWorker", so it is easier
to find when looking through parallel-worker code.Yeah, "PqMsg_Progress" seems unduly generic. I don't think we should
rename it in the back branches, but doing so in master looks like
it'd be a small-footprint change.All of the options I've seen listed in this thread so far involve changing
the part after the PqMsg_ prefix, but given we now have PqReplMsg_ and
PqBackupMsg_, I'm thinking it'd be better to give parallel worker messages
their own prefix. That seems to act as a sort of class or namespace for
the message. Maybe something like PqParallelMsg_Progress would work here.
I think PqParallelMsg_Progress makes sense. The attached does this.
--
Sami Imseih
Amazon Web Services (AWS)
Sami Imseih <samimseih.pg@gmail.com> writes:
On Thu, Sep 10, 2026 at 9:30 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
All of the options I've seen listed in this thread so far involve changing
the part after the PqMsg_ prefix, but given we now have PqReplMsg_ and
PqBackupMsg_, I'm thinking it'd be better to give parallel worker messages
their own prefix. That seems to act as a sort of class or namespace for
the message. Maybe something like PqParallelMsg_Progress would work here.
I think PqParallelMsg_Progress makes sense. The attached does this.
+1
regards, tom lane
On Thu, Sep 10, 2026 at 01:14:55PM -0400, Tom Lane wrote:
Sami Imseih <samimseih.pg@gmail.com> writes:
On Thu, Sep 10, 2026 at 9:30 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
All of the options I've seen listed in this thread so far involve changing
the part after the PqMsg_ prefix, but given we now have PqReplMsg_ and
PqBackupMsg_, I'm thinking it'd be better to give parallel worker messages
their own prefix. That seems to act as a sort of class or namespace for
the message. Maybe something like PqParallelMsg_Progress would work here.I think PqParallelMsg_Progress makes sense. The attached does this.
+1
As I'm writing the commit message for this, I'm remembering why I
originally went with "PqMsg" for this one. Parallel workers reuse a bunch
of "regular" message types for roughly the same purpose as non-parallel
uses, so (with this commit applied) a function like
ProcessParallelMessage() would handle a bunch of "PqMsg" message types and
one "PqParallelMsg" type. I think the obvious way to resolve that
inconsistency is to duplicate those macros and give them the parallel
prefix, but I'm not sure that's a worthwhile endeavor. Those characters
have roughly the same meaning for parallel workers/leaders as they do
elsewhere, so there's no real gain in code readability. I'm content to
commit Sami's patch and let it be, but if folks would rather I straighten
things out further, I can do so.
--
nathan
I think the obvious way to resolve that
inconsistency is to duplicate those macros and give them the parallel
prefix, but I'm not sure that's a worthwhile endeavor. Those characters
have roughly the same meaning for parallel workers/leaders as they do
elsewhere, so there's no real gain in code readability.
IMO, this is not necessary. I can't think of a good reason.
--
Sami
Sami Imseih <samimseih.pg@gmail.com> writes:
I think the obvious way to resolve that
inconsistency is to duplicate those macros and give them the parallel
prefix, but I'm not sure that's a worthwhile endeavor. Those characters
have roughly the same meaning for parallel workers/leaders as they do
elsewhere, so there's no real gain in code readability.
IMO, this is not necessary. I can't think of a good reason.
Agreed. I think the main problem with the way things stand is that
it looks like PqMsg_Progress might apply for regular clients, which
it doesn't.
Possibly it'd be appropriate to adjust the comments in protocol.h
to make things clearer. I think this might be enough:
-/* These are the codes sent by parallel workers to leader processes. */
+/* Additional codes sent by parallel workers to leader processes. */regards, tom lane
On Thu, Sep 10, 2026 at 03:50:20PM -0400, Tom Lane wrote:
Sami Imseih <samimseih.pg@gmail.com> writes:
I think the obvious way to resolve that
inconsistency is to duplicate those macros and give them the parallel
prefix, but I'm not sure that's a worthwhile endeavor. Those characters
have roughly the same meaning for parallel workers/leaders as they do
elsewhere, so there's no real gain in code readability.IMO, this is not necessary. I can't think of a good reason.
Agreed. I think the main problem with the way things stand is that
it looks like PqMsg_Progress might apply for regular clients, which
it doesn't.Possibly it'd be appropriate to adjust the comments in protocol.h
to make things clearer. I think this might be enough:-/* These are the codes sent by parallel workers to leader processes. */ +/* Additional codes sent by parallel workers to leader processes. */
Committed with that comment adjustment, thanks.
--
nathan