Re: Parallel Apply - Mailing list pgsql-hackers

On Wed, Apr 22, 2026 at 7:23 PM shveta malik <shveta.malik@gmail.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