On Thu, Dec 22, 2022 3:17 PM Masahiko Sawada wrote:
> > 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.
>
> +1. I think t
On Mon, 26 Dec 2022 at 14:04, Amit Kapila wrote:
>
> On Sat, Dec 24, 2022 at 3:28 PM Dilip Kumar wrote:
> >
> > On Sat, Dec 24, 2022 at 8:56 AM Amit Kapila wrote:
> > >
> > > > >
> > > > > OK, I removed the modification in 022_twophase_cascade.pl and combine
> > > > > the two patches.
> > > >
>
On Wed, Dec 28, 2022 at 1:42 AM Andres Freund wrote:
>
> On 2022-12-26 14:04:28 +0530, Amit Kapila wrote:
> > Pushed.
>
> I did not follow this thread but saw the commit. Could you explain why a GUC
> is the right mechanism here? The commit message didn't explain why a GUC was
> chosen.
>
> To me
Hi,
On 2022-12-26 14:04:28 +0530, Amit Kapila wrote:
> Pushed.
I did not follow this thread but saw the commit. Could you explain why a GUC
is the right mechanism here? The commit message didn't explain why a GUC was
chosen.
To me an option like this should be passed in when decoding rather than
On Sat, Dec 24, 2022 at 3:28 PM Dilip Kumar wrote:
>
> On Sat, Dec 24, 2022 at 8:56 AM Amit Kapila wrote:
> >
> > > >
> > > > OK, I removed the modification in 022_twophase_cascade.pl and combine
> > > > the two patches.
> > >
> > > Thank you for updating the patch. The v6 patch looks good to me
On Sat, Dec 24, 2022 at 8:56 AM Amit Kapila wrote:
>
> > >
> > > OK, I removed the modification in 022_twophase_cascade.pl and combine the
> > > two patches.
> >
> > Thank you for updating the patch. The v6 patch looks good to me.
> >
>
> LGTM as well. I'll push this on Monday.
>
The patch looks
On Fri, Dec 23, 2022 at 10:31 PM Masahiko Sawada wrote:
>
> On Fri, Dec 23, 2022 at 5:32 PM shiy.f...@fujitsu.com
> wrote:
> >
> > On Fri, Dec 23, 2022 1:50 PM Amit Kapila
> > >
> > > On Thu, Dec 22, 2022 at 6:18 PM shiy.f...@fujitsu.com
> > > wrote:
> > > >
> > > >
> > > > Besides, I tried to
On Fri, Dec 23, 2022 at 5:32 PM shiy.f...@fujitsu.com
wrote:
>
> On Fri, Dec 23, 2022 1:50 PM Amit Kapila
> >
> > On Thu, Dec 22, 2022 at 6:18 PM shiy.f...@fujitsu.com
> > wrote:
> > >
> > >
> > > Besides, I tried to reduce data size in streaming subscription tap tests
> > > by this
> > > new G
On Fri, Dec 23, 2022 at 2:03 PM shiy.f...@fujitsu.com
wrote:
>
> On Fri, Dec 23, 2022 1:50 PM Amit Kapila
> >
> > On Thu, Dec 22, 2022 at 6:18 PM shiy.f...@fujitsu.com
> > wrote:
> > >
> > >
> > > Besides, I tried to reduce data size in streaming subscription tap
tests by this
> > > new GUC (see
Dear Shi,
> I also fixed Kuroda-san's comments[1].
Thanks for updating the patch! I confirmed that my comments were fixed and
your patch could pass subscription and test_decoding tests. I think we can
change the status to "Ready for Committer".
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
On Fri, Dec 23, 2022 1:50 PM Amit Kapila
>
> On Thu, Dec 22, 2022 at 6:18 PM shiy.f...@fujitsu.com
> wrote:
> >
> >
> > Besides, I tried to reduce data size in streaming subscription tap tests by
> > this
> > new GUC (see 0002 patch). But I didn't covert all streaming tap tests
> because I
> >
On Fri, Dec 23, 2022 at 1:12 PM Hayato Kuroda (Fujitsu)
wrote:
>
> Dear hackers,
>
> > I will check and report the test coverage if I can.
>
> I ran make coverage. PSA the screen shot that shows results.
> According to the result the coverage seemed to be not changed
> even if the elapsed time was
On Thu, Dec 22, 2022 at 6:18 PM shiy.f...@fujitsu.com
wrote:
>
>
> Besides, I tried to reduce data size in streaming subscription tap tests by
> this
> new GUC (see 0002 patch). But I didn't covert all streaming tap tests because
> I
> think we also need to cover the case that there are lots of
On Fri, Dec 23, 2022 at 10:48 AM Hayato Kuroda (Fujitsu)
wrote:
>
> ```
> +* If logical_decoding_mode is immediate, loop until there's no
> change.
> +* Otherwise, loop until we reach under the memory limit. One might
> think
> +* that just by evicting the largest (sub)tr
On Fri, Dec 23, 2022 at 10:32 AM Masahiko Sawada wrote:
>
> >
> > Besides, I tried to reduce data size in streaming subscription tap tests by
> > this
> > new GUC (see 0002 patch). But I didn't covert all streaming tap tests
> > because I
> > think we also need to cover the case that there are l
Dear Shi,
Thanks for updating the patch! Followings are comments.
ReorderBufferCheckMemoryLimit()
```
+ /*
+* Bail out if logical_decoding_mode is disabled and we haven't exceeded
+* the memory limit.
+*/
```
I think 'disabled' should be '"buffered"'.
```
+
On Thu, Dec 22, 2022 at 9:48 PM shiy.f...@fujitsu.com
wrote:
>
> On Thu, Dec 22, 2022 5:24 PM Amit Kapila wrote:
> >
> > On Thu, Dec 22, 2022 at 1:15 PM Kyotaro Horiguchi
> > wrote:
> > >
> > > At Thu, 22 Dec 2022 12:35:46 +0530, Amit Kapila
> > wrote in
> > > > I have addressed these comments
On Thu, Dec 22, 2022 at 6:19 PM Amit Kapila wrote:
>
> On Thu, Dec 22, 2022 at 12:47 PM Masahiko Sawada
> wrote:
> >
> > On Thu, Dec 22, 2022 at 4:05 PM Amit Kapila wrote:
> > >
> >
> > > I think
> > > instead of adding new tests with this patch, it may be better to
> > > change some of the ex
At Thu, 22 Dec 2022 14:53:34 +0530, Amit Kapila wrote
in
> For this, I like your proposal for "buffered" as an explicit default value.
Good to hear.
> Okay, how about modifying it to: "When set to
> immediate, stream each change if
> streaming option (see optional parameters set by
> CREATE SU
On Thu, Dec 22, 2022 5:24 PM Amit Kapila wrote:
>
> On Thu, Dec 22, 2022 at 1:15 PM Kyotaro Horiguchi
> wrote:
> >
> > At Thu, 22 Dec 2022 12:35:46 +0530, Amit Kapila
> wrote in
> > > I have addressed these comments in the attached. Additionally, I have
> > > modified the docs and commit messag
On Thu, Dec 22, 2022 at 1:15 PM Kyotaro Horiguchi
wrote:
>
> At Thu, 22 Dec 2022 12:35:46 +0530, Amit Kapila
> wrote in
> > 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
On Thu, Dec 22, 2022 at 12:47 PM Masahiko Sawada wrote:
>
> On Thu, Dec 22, 2022 at 4:05 PM Amit Kapila wrote:
> >
>
> > 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
On Thu, Dec 22, 2022 at 1:55 PM Kyotaro Horiguchi
wrote:
>
> At Thu, 22 Dec 2022 16:59:30 +0900, Masahiko Sawada
> wrote in
> > On Thu, Dec 22, 2022 at 4:18 PM Hayato Kuroda (Fujitsu)
> > wrote:
> > >
> > > Dear Amit,
> > >
> > > Thank you for updating the patch. I have also checked the patch
>
At Thu, 22 Dec 2022 16:59:30 +0900, Masahiko Sawada
wrote in
> On Thu, Dec 22, 2022 at 4:18 PM Hayato Kuroda (Fujitsu)
> wrote:
> >
> > Dear Amit,
> >
> > Thank you for updating the patch. I have also checked the patch
> > and basically it has worked well. Almost all things I found were modifie
On Thu, Dec 22, 2022 at 4:18 PM Hayato Kuroda (Fujitsu)
wrote:
>
> Dear Amit,
>
> Thank you for updating the patch. I have also checked the patch
> and basically it has worked well. Almost all things I found were modified
> by v4.
>
> One comment: while setting logical_decoding_mode to wrong value
At Thu, 22 Dec 2022 12:35:46 +0530, Amit Kapila wrote
in
> 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 t
Dear Amit,
Thank you for updating the patch. I have also checked the patch
and basically it has worked well. Almost all things I found were modified
by v4.
One comment: while setting logical_decoding_mode to wrong value,
I got unfriendly ERROR message.
```
postgres=# SET logical_decoding_mode =
On Thu, Dec 22, 2022 at 4:05 PM Amit Kapila wrote:
>
> On Wed, Dec 21, 2022 at 7:25 PM Masahiko Sawada wrote:
> >
> > On Wed, Dec 21, 2022 at 10:14 PM shiy.f...@fujitsu.com
> > wrote:
> >
> > The patch looks good to me. Some minor comments are:
> >
> > - * limit, but we might also adapt a more e
On Wed, Dec 21, 2022 at 7:25 PM Masahiko Sawada wrote:
>
> On Wed, Dec 21, 2022 at 10:14 PM shiy.f...@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 trans
On Wed, Dec 21, 2022 at 10:14 PM shiy.f...@fujitsu.com
wrote:
>
> On Wed, Dec 21, 2022 4:54 PM Amit Kapila wrote:
> >
> > On Wed, Dec 21, 2022 at 1:55 PM Peter Smith
> > wrote:
> > >
> > > On Wed, Dec 21, 2022 at 6:22 PM Masahiko Sawada
> > wrote:
> > > >
> > > > On Tue, Dec 20, 2022 at 7:49 PM
On Wed, Dec 21, 2022 4:05 PM Peter Smith wrote:
>
> Here are some review comments for patch v2.
>
> Since the GUC is still under design maybe these comments can be
> ignored for now, but I guess similar comments will apply in future
> anyhow (just with some name changes).
>
Thanks for your com
On Wed, Dec 21, 2022 4:54 PM Amit Kapila wrote:
>
> On Wed, Dec 21, 2022 at 1:55 PM Peter Smith
> wrote:
> >
> > On Wed, Dec 21, 2022 at 6:22 PM Masahiko Sawada
> wrote:
> > >
> > > On Tue, Dec 20, 2022 at 7:49 PM Amit Kapila
> wrote:
> > > >
> > > > On Tue, Dec 20, 2022 at 2:46 PM Hayato Kuro
Dear Shveta, other hackers
> Going with ' logical_decoding_work_mem' seems a reasonable solution, but
> since we are mixing
> the functionality of developer and production GUC, there is a slight risk
> that customer/DBAs may end
> up setting it to 0 and forget about it and thus hampering system'
On Wed, Dec 21, 2022 at 1:55 PM Peter Smith wrote:
>
> On Wed, Dec 21, 2022 at 6:22 PM Masahiko Sawada wrote:
> >
> > On Tue, Dec 20, 2022 at 7:49 PM Amit Kapila wrote:
> > >
> > > On Tue, Dec 20, 2022 at 2:46 PM Hayato Kuroda (Fujitsu)
> > > wrote:
> > > >
> > > > Dear hackers,
> > > >
> > > >
On Wed, Dec 21, 2022 at 6:22 PM Masahiko Sawada wrote:
>
> On Tue, Dec 20, 2022 at 7:49 PM Amit Kapila wrote:
> >
> > On Tue, Dec 20, 2022 at 2:46 PM Hayato Kuroda (Fujitsu)
> > wrote:
> > >
> > > Dear hackers,
> > >
> > > > We have discussed three different ways to provide GUC for these
> > > >
Here are some review comments for patch v2.
Since the GUC is still under design maybe these comments can be
ignored for now, but I guess similar comments will apply in future
anyhow (just with some name changes).
==
1. Commit message
Add a new GUC force_stream_mode, when it is set on, send
On Tue, Dec 20, 2022 at 7:49 PM Amit Kapila wrote:
>
> On Tue, Dec 20, 2022 at 2:46 PM Hayato Kuroda (Fujitsu)
> wrote:
> >
> > Dear hackers,
> >
> > > We have discussed three different ways to provide GUC for these
> > > features. (1) Have separate GUCs like force_server_stream_mode,
> > > force
Going with ' logical_decoding_work_mem' seems a reasonable solution, but
since we are mixing
the functionality of developer and production GUC, there is a slight risk
that customer/DBAs may end
up setting it to 0 and forget about it and thus hampering system's
performance.
Have seen many such cases
Dear Amit,
> The other possibility to achieve what you are saying is that we allow
> a minimum value of logical_decoding_work_mem as 0 which would mean
> stream or serialize each change depending on whether the streaming
> option is enabled.
I understood that logical_decoding_work_mem may double
On Tue, Dec 20, 2022 at 2:46 PM Hayato Kuroda (Fujitsu)
wrote:
>
> Dear hackers,
>
> > We have discussed three different ways to provide GUC for these
> > features. (1) Have separate GUCs like force_server_stream_mode,
> > force_server_serialize_mode, force_client_serialize_mode (we can use
> > di
Dear hackers,
> We have discussed three different ways to provide GUC for these
> features. (1) Have separate GUCs like force_server_stream_mode,
> force_server_serialize_mode, force_client_serialize_mode (we can use
> different names for these) for each of these; (2) Have two sets of
> GUCs for s
On Tue, Dec 20, 2022 at 10:52 AM Dilip Kumar wrote:
>
> On Wed, Dec 14, 2022 at 5:29 PM Amit Kapila wrote:
> >
> > On Wed, Dec 14, 2022 at 2:15 PM shiy.f...@fujitsu.com
> > wrote:
> > >
> > > Please see the attached patch. I also fix Peter's comments[1]. The GUC
> > > name and
> > > design are
On Wed, Dec 14, 2022 at 5:29 PM Amit Kapila wrote:
>
> On Wed, Dec 14, 2022 at 2:15 PM shiy.f...@fujitsu.com
> wrote:
> >
> > Please see the attached patch. I also fix Peter's comments[1]. The GUC name
> > and
> > design are still under discussion, so I didn't modify them.
> >
>
> Let me summari
On Wed, Dec 14, 2022 at 2:15 PM shiy.f...@fujitsu.com
wrote:
> > -while (rb->size >= logical_decoding_work_mem * 1024L)
> > +while ((!force_stream && rb->size >= logical_decoding_work_mem *
> > 1024L) ||
> > + (force_stream && rb->size > 0))
> > {
> >
> > It seems like if f
On Wed, Dec 14, 2022 at 2:15 PM shiy.f...@fujitsu.com
wrote:
>
> Please see the attached patch. I also fix Peter's comments[1]. The GUC name
> and
> design are still under discussion, so I didn't modify them.
>
Let me summarize the discussion on name and design till now. As per my
understanding,
On Sat, Dec 10, 2022 2:03 PM Dilip Kumar wrote:
>
> On Tue, Dec 6, 2022 at 11:53 AM shiy.f...@fujitsu.com
> wrote:
> >
> > Hi hackers,
> >
> > In logical decoding, when logical_decoding_work_mem is exceeded, the
> changes are
> > sent to output plugin in streaming mode. But there is a restrictio
On Tue, Dec 13, 2022 at 2:33 PM Peter Smith wrote:
>
> On Tue, Dec 6, 2022 at 5:23 PM shiy.f...@fujitsu.com
> wrote:
> >
> > Hi hackers,
> >
> > In logical decoding, when logical_decoding_work_mem is exceeded, the
> > changes are
> > sent to output plugin in streaming mode. But there is a restri
On Tue, Dec 6, 2022 at 5:23 PM shiy.f...@fujitsu.com
wrote:
>
> Hi hackers,
>
> In logical decoding, when logical_decoding_work_mem is exceeded, the changes
> are
> sent to output plugin in streaming mode. But there is a restriction that the
> minimum value of logical_decoding_work_mem is 64kB. I
On Sat, Dec 10, 2022 at 11:18 AM Dilip Kumar wrote:
>
> On Wed, Dec 7, 2022 at 5:16 AM Peter Smith wrote:
>
> +1 for the idea
>
> >
> > There is potential for lots of developer GUCs for testing/debugging in
> > the area of logical replication but IMO it might be better to keep
> > them all separa
On Sat, Dec 10, 2022 at 5:03 PM Dilip Kumar wrote:
>
> On Tue, Dec 6, 2022 at 11:53 AM shiy.f...@fujitsu.com
> wrote:
> >
> > Hi hackers,
> >
> > In logical decoding, when logical_decoding_work_mem is exceeded, the
> > changes are
> > sent to output plugin in streaming mode. But there is a restr
On Tue, Dec 6, 2022 at 11:53 AM shiy.f...@fujitsu.com
wrote:
>
> Hi hackers,
>
> In logical decoding, when logical_decoding_work_mem is exceeded, the changes
> are
> sent to output plugin in streaming mode. But there is a restriction that the
> minimum value of logical_decoding_work_mem is 64kB.
On Wed, Dec 7, 2022 at 5:16 AM Peter Smith wrote:
+1 for the idea
>
> There is potential for lots of developer GUCs for testing/debugging in
> the area of logical replication but IMO it might be better to keep
> them all separated. Putting everything into a single
> 'logical_replication_mode' mi
On Wed, Dec 7, 2022 at 10:55 AM Masahiko Sawada wrote:
>
> On Wed, Dec 7, 2022 at 12:55 PM Amit Kapila wrote:
> >
>
> > On one side having separate GUCs for publisher and subscriber seems to
> > give better flexibility but having more GUCs also sometimes makes them
> > less usable. Here, my thoug
On Wed, Dec 7, 2022 at 12:55 PM Amit Kapila wrote:
>
> On Wed, Dec 7, 2022 at 7:31 AM Masahiko Sawada wrote:
> >
> > On Wed, Dec 7, 2022 at 8:46 AM Peter Smith wrote:
> > >
> > > >
> > > > Yeah, I think this can also help in reducing the time for various
> > > > tests in test_decoding/stream and
On Wed, Dec 7, 2022 at 7:31 AM Masahiko Sawada wrote:
>
> On Wed, Dec 7, 2022 at 8:46 AM Peter Smith wrote:
> >
> > >
> > > Yeah, I think this can also help in reducing the time for various
> > > tests in test_decoding/stream and
> > > src/test/subscription/t/*_stream_*.pl file by reducing the nu
On Tue, Dec 6, 2022 at 7:18 PM Masahiko Sawada wrote:
>
> On Tue, Dec 6, 2022 at 7:29 PM Amit Kapila wrote:
> >
> > On Tue, Dec 6, 2022 at 11:53 AM shiy.f...@fujitsu.com
> > wrote:
> > >
> > > Hi hackers,
> > >
> > > In logical decoding, when logical_decoding_work_mem is exceeded, the
> > > cha
On Wed, Dec 7, 2022 at 8:46 AM Peter Smith wrote:
>
> On Tue, Dec 6, 2022 at 9:29 PM Amit Kapila wrote:
> >
> > On Tue, Dec 6, 2022 at 11:53 AM shiy.f...@fujitsu.com
> > wrote:
> > >
> > > Hi hackers,
> > >
> > > In logical decoding, when logical_decoding_work_mem is exceeded, the
> > > changes
On Tue, Dec 6, 2022 at 5:23 PM shiy.f...@fujitsu.com
wrote:
>
> Hi hackers,
>
> In logical decoding, when logical_decoding_work_mem is exceeded, the changes
> are
> sent to output plugin in streaming mode. But there is a restriction that the
> minimum value of logical_decoding_work_mem is 64kB. I
On Tue, Dec 6, 2022 at 9:29 PM Amit Kapila wrote:
>
> On Tue, Dec 6, 2022 at 11:53 AM shiy.f...@fujitsu.com
> wrote:
> >
> > Hi hackers,
> >
> > In logical decoding, when logical_decoding_work_mem is exceeded, the
> > changes are
> > sent to output plugin in streaming mode. But there is a restri
On Tue, Dec 6, 2022 at 7:29 PM Amit Kapila wrote:
>
> On Tue, Dec 6, 2022 at 11:53 AM shiy.f...@fujitsu.com
> wrote:
> >
> > Hi hackers,
> >
> > In logical decoding, when logical_decoding_work_mem is exceeded, the
> > changes are
> > sent to output plugin in streaming mode. But there is a restri
On Tue, Dec 6, 2022 at 11:53 AM shiy.f...@fujitsu.com
wrote:
>
> Hi hackers,
>
> In logical decoding, when logical_decoding_work_mem is exceeded, the changes
> are
> sent to output plugin in streaming mode. But there is a restriction that the
> minimum value of logical_decoding_work_mem is 64kB.
61 matches
Mail list logo
