Re: Logical replication 'invalid memory alloc request size 1585837200' after upgrading to 17.5

On Mon, 2 Jun 2025 at 22:49, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> Thank you for updating the patch. Here are some review comments:
>
> +   req_mem_size = sizeof(SharedInvalidationMessage) *
> (txn->ninvalidations_distr + nmsgs);
> +
> +   /*
> +    * If the number of invalidation messages is larger than 8MB, it's more
> +    * efficient to invalidate the entire cache rather than processing each
> +    * message individually.
> +    */
> +   if (req_mem_size > (8 * 1024 * 1024) || rbtxn_inval_all_cache(txn))
>
> It's better to define the maximum number of distributed inval messages
> per transaction as a macro instead of calculating the memory size
> every time.

Modified

> ---
> +static void
> +ReorderBufferAddInvalidationsCommon(ReorderBuffer *rb, TransactionId xid,
> +                                   XLogRecPtr lsn, Size nmsgs,
> +                                   SharedInvalidationMessage *msgs,
> +                                   ReorderBufferTXN *txn,
> +                                   bool for_inval)
>
> This function is quite confusing to me. For instance,
> ReorderBufferAddDistributedInvalidations() needs to call this function
> with for_inval=false in spite of adding inval messages actually. Also,
> the following condition seems not intuisive but there is no comment:
>
>     if (!for_inval || (for_inval && !rbtxn_inval_all_cache(txn)))
>
> Instead of having ReorderBufferAddInvalidationsCommon(), I think we
> can have a function say ReorderBufferQueueInvalidations() where we
> enqueue the given inval messages as a
> REORDER_BUFFER_CHANGE_INVALIDATION change.
> ReorderBufferAddInvalidations() adds inval messages to
> txn->invalidations and calls that function, while
> ReorderBufferQueueInvalidations() adds inval messages to
> txn->distributed_ivnalidations and calls that function if the array is
> not full.

Modified

> BTW if we need to invalidate all accumulated caches at the end of
> transaction replay anyway, we don't need to add inval messages to
> txn->invalidations once txn->distributed_invalidations gets full?

yes, no need to add invalidation messages to txn->invalidation once
RBTXN_INVAL_ALL_CACHE is set. This is handled now.

The attached v9 version patch has the changes for the same.

Regards,
Vignesh