On Fri, Feb 7, 2020 at 4:18 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > On Wed, Feb 5, 2020 at 4:05 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Wed, Feb 5, 2020 at 9:46 AM Dilip Kumar <dilipbal...@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
