On Fri, Feb 7, 2020 at 4:18 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Wed, Feb 5, 2020 at 4:05 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Feb 5, 2020 at 9:46 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > Fixed in the latest version sent upthread.
> > >
> >
> > Okay, thanks. I haven't looked at the latest version of patch series
> > as I was reviewing the previous version and I think all of these
> > comments are in the patch which is not modified. Here are my
> > comments:
> >
> > I think we don't need to maintain
> > v8-0007-Support-logical_decoding_work_mem-set-from-create as per
> > discussion in one of the above emails [1] as its usage is not clear.
> >
> > v8-0008-Add-support-for-streaming-to-built-in-replication
> > 1.
> > - information. The allowed options are <literal>slot_name</literal> and
> > - <literal>synchronous_commit</literal>
> > + information. The allowed options are <literal>slot_name</literal>,
> > + <literal>synchronous_commit</literal>, <literal>work_mem</literal>
> > + and <literal>streaming</literal>.
> >
> > As per the discussion above [1], I don't think we need work_mem here.
> > You might want to remove the other usage from the patch as well.
>
> After putting more thought on this it appears that there could be some
> use cases for setting the work_mem from the subscription, Assume a
> case where data are coming from two different origins and based on the
> origin ids different slots might collect different type of changes,
> So isn't it good to have different work_mem for different slots? I am
> not saying that the current way of implementing is the best one but
> that we can improve. First, we need to decide whether we have a use
> case for this or not.
>
That is the whole point. I don't see a very clear usage of this and
neither did anybody explained clearly how it will be useful. I am not
denying that what you are describing has no use, but as you said we
might need to invent an entirely new way even if we have such a use.
I think it is better to avoid the requirements which are not essential
for this patch.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com