Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions

On Thu, Nov 7, 2019 at 3:50 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Thu, Nov 7, 2019 at 3:19 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> > What do you think?
> I have reviewed your changes and looks fine to me.
>

Okay, thanks. I am also happy with the two patches I have posted in
my last email [1].

Tomas, would you like to take a look at those patches and commit them
if you are happy or would you like me to do the same?

Some notes before commit:
--------------------------------------
1.
Commit message need to be changed for the first patch
-------------------------------------------------------------------------
A.
> The memory limit is defined by a new logical_decoding_work_mem GUC, so for example we can do this

SET logical_decoding_work_mem = '128kB'

> to trigger very aggressive streaming. The minimum value is 64kB.

I think this patch doesn't contain streaming, so we either need to
reword it or remove it.

B.
> The logical_decoding_work_mem may be set either in postgresql.conf, in which case it serves as the default for all publishers on that instance, or when creating the
> subscription, using a work_mem paramemter in the WITH clause (specifies number of kilobytes).

We need to reword this as we have decided to remove the setting from
the subscription side as of now.

2. I think we can change the message level in UpdateSpillStats() to DEBUG2.

3. I think we need catversion bump for the second patch.

4. I think we can combine both patches and commit as one patch, but it
is okay to commit them separately as well.

[1] - https://www.postgresql.org/message-id/CAA4eK1Kdmi6VVguKEHV6Ho2isCPVFdQtt0WLsK10fiuE59_0Yw%40mail.gmail.com

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com