On Wed, Dec 21, 2022 at 7:25 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Wed, Dec 21, 2022 at 10:14 PM shiy.fnst@fujitsu.com
> <shiy.fnst@fujitsu.com> wrote:
>
> The patch looks good to me. Some minor comments are:
>
> - * limit, but we might also adapt a more elaborate eviction strategy
> - for example
> - * evicting enough transactions to free certain fraction (e.g. 50%)
> of the memory
> - * limit.
> + * limit, but we might also adapt a more elaborate eviction strategy - for
> + * example evicting enough transactions to free certain fraction (e.g. 50%) of
> + * the memory limit.
>
> This change is not relevant with this feature.
>
> ---
> + if (logical_decoding_mode == LOGICAL_DECODING_MODE_DEFAULT
> + && rb->size < logical_decoding_work_mem * 1024L)
>
> Since we put '&&' before the new line in all other places in
> reorderbuffer.c, I think it's better to make it consistent. The same
> is true for the change for while loop in the patch.
>
I have addressed these comments in the attached. Additionally, I have
modified the docs and commit messages to make those clear. I think
instead of adding new tests with this patch, it may be better to
change some of the existing tests related to streaming to use this
parameter as that will clearly show one of the purposes of this patch.
--
With Regards,
Amit Kapila.