On Sun, Sep 29, 2019 at 12:39 AM Tomas Vondra <tomas.von...@2ndquadrant.com> wrote: > > On Sat, Sep 28, 2019 at 01:36:46PM +0530, Amit Kapila wrote: > >On Fri, Sep 27, 2019 at 4:55 PM Tomas Vondra > ><tomas.von...@2ndquadrant.com> wrote: > > I do think having a separate GUC is a must, irrespectedly of what other > GUC (if any) is used as a default. You're right the maintenance_work_mem > value might be too high (e.g. in cases with many subscriptions), but the > same issue applies to work_mem - there's no guarantee work_mem is lower > than maintenance_work_mem, and in analytics databases it may be set very > high. So work_mem does not really solve the issue > > IMHO we can't really do without a new GUC. It's not difficult to create > examples that would benefit from small/large memory limit, depending on > the number of subscriptions etc. > > I do however agree the GUC does not have to be tied to any existing one, > it was just an attempt to use a more sensible default value. I do think > m_w_m would be fine, but I can live with using an explicit value. > > So that's what I did in the attached patch - I've renamed the GUC to > logical_decoding_work_mem, detached it from m_w_m and set the default to > 64MB (i.e. the same default as m_w_m).
Fair enough, let's not argue more on this unless someone else wants to share his opinion. > It should also fix all the issues > from the recent reviews (at least I believe so). > Have you given any thought on creating a test case for this patch? I think you also told that you will test some worst-case scenarios and report the numbers so that we are convinced that the current eviction algorithm is good. > I've realized that one of the subsequent patches allows overriding the > limit for individual subscriptions (in the CREATE SUBSCRIPTION command). > I think it'd be good to move this bit forward, but I think it can be > done in a separate patch. > Yeah, it is better to deal it separately as I am also not entirely convinced at this stage about this parameter. I have mentioned the same in the previous email as well. While glancing through the changes, I noticed a small thing: +#logical_decoding_work_mem = 64MB # min 1MB, or -1 to use maintenance_work_mem I guess this need to be updated. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
