| From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
|---|---|
| To: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Cc: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Andrei Lepikhov <lepihov(at)gmail(dot)com>, wenhui qiu <qiuwenhuifx(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Parallel Apply |
| Date: | 2026-04-23 02:00:31 |
| Message-ID: | CAHut+Pu79i_h3fcOxGoGhKs5u63mzqG-tssHSNyZpbVU1HeJ4Q@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Apr 22, 2026 at 7:23 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
...
> Regarding 0001, I did not understand the need of having 2 separate messages:
>
> +#define PARALLEL_APPLY_INTERNAL_MESSAGE 'i'
> + LOGICAL_REP_MSG_INTERNAL_MESSAGE = 'i',
>
> And the need of sending both together in 0003:
>
> +send_internal_dependencies(ParallelApplyWorkerInfo *winfo, List
> *depends_on_xids)
> +{
> + pq_sendbyte(&dependencies, PARALLEL_APPLY_INTERNAL_MESSAGE);
> + pq_sendbyte(&dependencies, LOGICAL_REP_MSG_INTERNAL_MESSAGE);
>
>
> Also, it is confusing that above 2 are 'i' and
> WORKER_INTERNAL_MSG_RELATION is also 'i'. Code has become very tricky
> to understand now.
>
> Reviewing everything, I feel having 'i' outside of LogicalRepMsgType
> was better. I think it will eb better to retain
> PARALLEL_APPLY_INTERNAL_MESSAGE and getting rid of
> LOGICAL_REP_MSG_INTERNAL_MESSAGE. And when any worker intercepts
> PARALLEL_APPLY_INTERNAL_MESSAGE, it need not dispatch
> (apply_dispatch), instead it can handle it using
> apply_handle_internal_message()
>
> Goign above way:
> --Messaged received from pub can be handled using apply_dispatch.
> --Messages generated from leader to be handled separately/internally
> using apply_handle_internal_message().
>
> That way we have clear-cut boundary between the two types and less confusion.
Hi Shveta,
IIUC these need to be separate because they are used in 2 completly
different ways:
1. In LogicalParallelApplyLoop the code need to identify as different
from PqReplMsg_WALData
2. In apply_dispach() the message is delegated elsewhere according to
the type LogicalRepMsgType
PSA a pictue I made for my understanding of the current v15-0001
design. It might help to visualize the message format more easily.
While your suggestion looks good for LogicalParallelApplyLoop, I think
the real problem is going to be in the apply_spooled_mesages() which
wants call the apply_dispatch() directly. That won't be possible if
LOGICAL_REP_MSG_INTERNAL_MESSAGE is removed. And, you cannot call
directly to apply_handle_internal_message() withint knowing it is a
PARALLEL_APPLY_INTERNAL_MESSAGE message, but that means first read it
pq_getmsgbyte(s). Then, you also need some hacky way to "unread" that
byte in case it was not the PARALLEL_APPLY_INTERNAL_MESSAGE byte, but
something different. AFAIK that was exactly what the previous
v14-0001 code was doing with the is_worker_internal_message()
function. I also think v15-0001 is a bit confusing, but v14-0001 was
even more so.
If there was some new function like `pq_peekmsgbyte(s)` which could
simply "peek" the message byte value without advancing the cursor.
Then, I apply_spooled_mesages() can just peek to find
PARALLEL_APPLY_INTERNAL_MESSAGE and your suggested simplification
could work. But it would *still* be complicated by the fact that you
would have to ensure that PARALLEL_APPLY_INTERNAL_MESSAGE could not
clash with any of the LogicalRepMsgType! In the end, just keeping the
LOGICAL_REP_MSG_INTERNAL_MESSAGE like v14 does may be the best way to
ensure that uniqueness...
>
> Also, can we use 'R' for WORKER_INTERNAL_MSG_RELATION similar to
> LOGICAL_REP_MSG_RELATION i.e. if 'i' is followed by 'R', then it means
> it is internal relation-msg instead of pub's one? 'R' seems a better
> choice over 'i' here.
+1
======
Kind Reagrds,
Peter Smith.
Fujitsu Australia
| Attachment | Content-Type | Size |
|---|---|---|
| v15-design.pdf | application/pdf | 80.4 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Chao Li | 2026-04-23 02:02:31 | Re: pg_test_timing: fix unit typo and widen diff type |
| Previous Message | Xuneng Zhou | 2026-04-23 01:04:07 | Re: RFC: pg_stat_logmsg |