On Wed, Apr 22, 2026 at 7:23 PM shveta malik <[email protected]> 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
v15-design.pdf
Description: Adobe PDF document
