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

2021-05-06 Thread Dilip Kumar
On Thu, May 6, 2021 at 9:01 AM Amit Kapila wrote: > > On Fri, Apr 30, 2021 at 7:43 PM Dilip Kumar wrote: > > > > So, I > > > used the other approach which led to the attached. > > > > The patch looks fine to me. > > > > Thanks, pushed! Thanks! -- Regards, Dilip Kumar EnterpriseDB:

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

2021-05-05 Thread Amit Kapila
On Fri, Apr 30, 2021 at 7:43 PM Dilip Kumar wrote: > > So, I > > used the other approach which led to the attached. > > The patch looks fine to me. > Thanks, pushed! -- With Regards, Amit Kapila.

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

2021-04-30 Thread Dilip Kumar
On Fri, Apr 30, 2021 at 3:01 PM Amit Kapila wrote: > > On Wed, Apr 28, 2021 at 11:03 AM Dilip Kumar wrote: > > > > On Wed, Apr 28, 2021 at 11:00 AM Amit Kapila > > wrote: > > > > > > > The idea I have is to additionally check that we are decoding > > > streaming or prepared transaction (the

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

2021-04-30 Thread Amit Kapila
On Wed, Apr 28, 2021 at 11:03 AM Dilip Kumar wrote: > > On Wed, Apr 28, 2021 at 11:00 AM Amit Kapila wrote: > > > > The idea I have is to additionally check that we are decoding > > streaming or prepared transaction (the same check as we have for > > setting curtxn) or we can check if

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

2021-04-27 Thread Dilip Kumar
On Wed, Apr 28, 2021 at 11:00 AM Amit Kapila wrote: > > Tom Lane has raised a complaint on pgsql-commiters [1] about one of > the commits related to this work [2]. The new member wrasse is showing > Warning: > >

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

2021-04-27 Thread Amit Kapila
Tom Lane has raised a complaint on pgsql-commiters [1] about one of the commits related to this work [2]. The new member wrasse is showing Warning: "/export/home/nm/farm/studio64v12_6/HEAD/pgsql.build/../pgsql/src/backend/replication/logical/reorderbuffer.c", line 2510: Warning: Likely null

Re: HASH_BLOBS hazards (was Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions)

2020-12-15 Thread Tom Lane
Noah Misch writes: > On Mon, Dec 14, 2020 at 01:59:03PM -0500, Tom Lane wrote: >> Here's a rolled-up patch that does some further documentation work >> and gets rid of the unnecessary memset's as well. > +1 on removing the memset() calls. That said, it's not a big deal if more > creep in over

Re: HASH_BLOBS hazards (was Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions)

2020-12-15 Thread Noah Misch
On Mon, Dec 14, 2020 at 01:59:03PM -0500, Tom Lane wrote: > * Should we just have a blanket insistence that all callers supply > HASH_ELEM? The default sizes that dynahash.c uses without that are > undocumented and basically useless. +1 > we should just rip out all those memsets as pointless,

Re: HASH_BLOBS hazards (was Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions)

2020-12-14 Thread Tom Lane
Here's a rolled-up patch that does some further documentation work and gets rid of the unnecessary memset's as well. regards, tom lane diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index 2dc9e44ae6..651227f510 100644 --- a/contrib/dblink/dblink.c +++

Re: HASH_BLOBS hazards (was Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions)

2020-12-14 Thread Tom Lane
I wrote: > There are a couple of other API oddities that maybe we should think > about while we're here: > * Should we just have a blanket insistence that all callers supply > HASH_ELEM? The default sizes that dynahash.c uses without that are > undocumented and basically useless. We're already

Re: HASH_BLOBS hazards (was Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions)

2020-12-14 Thread Tom Lane
Noah Misch writes: > On Sun, Dec 13, 2020 at 11:49:31AM -0500, Tom Lane wrote: >> A quick count of grep hits suggest that the large majority of >> existing hash_create() calls use HASH_BLOBS, and there might be >> only order-of-ten calls that would need to be touched if we >> required an explicit

Re: HASH_BLOBS hazards (was Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions)

2020-12-14 Thread Amit Kapila
On Mon, Dec 14, 2020 at 1:36 AM Noah Misch wrote: > > On Sun, Dec 13, 2020 at 11:49:31AM -0500, Tom Lane wrote: > > But what jumps out at me here is that this sort of error seems way > > too easy to make, and evidently way too hard to detect. What can we > > do to make it more obvious if one has

Re: HASH_BLOBS hazards (was Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions)

2020-12-14 Thread Peter Eisentraut
On 2020-12-13 17:49, Tom Lane wrote: 2. Don't allow a default: invent a new HASH_STRING flag, and require that hash_create() calls specify exactly one of HASH_BLOBS, HASH_STRING, or HASH_FUNCTION. This doesn't completely fix the hazard of mindless-copy-and-paste, but I think it might make it a

Re: HASH_BLOBS hazards (was Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions)

2020-12-13 Thread Noah Misch
On Sun, Dec 13, 2020 at 11:49:31AM -0500, Tom Lane wrote: > But what jumps out at me here is that this sort of error seems way > too easy to make, and evidently way too hard to detect. What can we > do to make it more obvious if one has incorrectly used or omitted > HASH_BLOBS? Both directions

HASH_BLOBS hazards (was Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions)

2020-12-13 Thread Tom Lane
Amit Kapila writes: > On Wed, Dec 9, 2020 at 2:56 PM Noah Misch wrote: >> The problem is xidhash using strcmp() to compare keys; it needs memcmp(). > Your analysis is correct. Sorry for not having noticed this thread before. Noah's fix is clearly correct, and I have no objection to the added

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

2020-12-09 Thread Amit Kapila
On Wed, Dec 9, 2020 at 2:56 PM Noah Misch wrote: > > Further testing showed it was a file location problem, not a deletion problem. > The worker tried to open > base/pgsql_tmp/pgsql_tmp9896408.1.sharedfileset/16393-510.changes.0, but these > were the files actually existing: > > [nm@power-aix 0:2

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

2020-12-09 Thread Noah Misch
On Wed, Dec 02, 2020 at 01:50:25PM +0530, Amit Kapila wrote: > On Wed, Dec 2, 2020 at 1:20 PM Dilip Kumar wrote: > > > On Mon, Nov 30, 2020 at 6:49 PM Amit Kapila > > > wrote: > > > > On Mon, Nov 30, 2020 at 3:14 AM Noah Misch wrote: > > > > > On Mon, Sep 07, 2020 at 12:00:41PM +0530, Amit

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

2020-12-02 Thread Amit Kapila
On Wed, Dec 2, 2020 at 1:20 PM Dilip Kumar wrote: > > On Tue, Dec 1, 2020 at 11:38 AM Dilip Kumar wrote: > > > > On Mon, Nov 30, 2020 at 6:49 PM Amit Kapila wrote: > > > > > > On Mon, Nov 30, 2020 at 3:14 AM Noah Misch wrote: > > > > > > > > On Mon, Sep 07, 2020 at 12:00:41PM +0530, Amit

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

2020-12-01 Thread Dilip Kumar
On Tue, Dec 1, 2020 at 11:38 AM Dilip Kumar wrote: > > On Mon, Nov 30, 2020 at 6:49 PM Amit Kapila wrote: > > > > On Mon, Nov 30, 2020 at 3:14 AM Noah Misch wrote: > > > > > > On Mon, Sep 07, 2020 at 12:00:41PM +0530, Amit Kapila wrote: > > > > Thanks, I have pushed the last patch. Let's wait

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

2020-12-01 Thread Amit Kapila
On Tue, Dec 1, 2020 at 11:38 AM Dilip Kumar wrote: > > On Mon, Nov 30, 2020 at 6:49 PM Amit Kapila wrote: > > > > > > What is going on here is that the expected streaming file is missing. > > Normally, the first time we send a stream of changes (some percentage > > of transaction changes) we

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

2020-11-30 Thread Dilip Kumar
On Mon, Nov 30, 2020 at 6:49 PM Amit Kapila wrote: > > On Mon, Nov 30, 2020 at 3:14 AM Noah Misch wrote: > > > > On Mon, Sep 07, 2020 at 12:00:41PM +0530, Amit Kapila wrote: > > > Thanks, I have pushed the last patch. Let's wait for a day or so to > > > see the buildfarm reports > > > >

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

2020-11-30 Thread Amit Kapila
On Mon, Nov 30, 2020 at 3:14 AM Noah Misch wrote: > > On Mon, Sep 07, 2020 at 12:00:41PM +0530, Amit Kapila wrote: > > Thanks, I have pushed the last patch. Let's wait for a day or so to > > see the buildfarm reports > >

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

2020-11-29 Thread Amit Kapila
On Mon, Nov 30, 2020 at 3:14 AM Noah Misch wrote: > > On Mon, Sep 07, 2020 at 12:00:41PM +0530, Amit Kapila wrote: > > Thanks, I have pushed the last patch. Let's wait for a day or so to > > see the buildfarm reports > >

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

2020-11-29 Thread Noah Misch
On Mon, Sep 07, 2020 at 12:00:41PM +0530, Amit Kapila wrote: > Thanks, I have pushed the last patch. Let's wait for a day or so to > see the buildfarm reports https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sungazer=2020-09-08%2006%3A24%3A14 failed the new 015_stream.pl test with the

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

2020-09-16 Thread Amit Kapila
On Tue, Sep 15, 2020 at 10:17 AM Amit Kapila wrote: > > On Tue, Sep 15, 2020 at 8:38 AM Tom Lane wrote: > > > As far as I can see they are useless in this case but I think they > > > might be required in case the user provides its own allocator function > > > (using HASH_ALLOC). So, we can

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

2020-09-14 Thread Amit Kapila
On Tue, Sep 15, 2020 at 8:38 AM Tom Lane wrote: > > Amit Kapila writes: > > On Mon, Sep 14, 2020 at 9:42 PM Tom Lane wrote: > >> BTW, unless someone has changed the behavior of dynahash when I > >> wasn't looking, those MemoryContextSwitchTos shown above are useless. > > > As far as I can see

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

2020-09-14 Thread Tom Lane
Amit Kapila writes: > On Mon, Sep 14, 2020 at 9:42 PM Tom Lane wrote: >> BTW, unless someone has changed the behavior of dynahash when I >> wasn't looking, those MemoryContextSwitchTos shown above are useless. > As far as I can see they are useless in this case but I think they > might be

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

2020-09-14 Thread Amit Kapila
On Mon, Sep 14, 2020 at 9:42 PM Tom Lane wrote: > > Amit Kapila writes: > > The attached patch will fix the issue. What do you think? > > I think it'd be cleaner to separate the initialization of a new entry from > validation altogether, along the lines of > > /* Find cached function info,

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

2020-09-14 Thread Tom Lane
Amit Kapila writes: > The attached patch will fix the issue. What do you think? I think it'd be cleaner to separate the initialization of a new entry from validation altogether, along the lines of /* Find cached function info, creating if not found */ oldctx =

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

2020-09-14 Thread Dilip Kumar
On Mon, Sep 14, 2020 at 4:50 PM Amit Kapila wrote: > > On Mon, Sep 14, 2020 at 1:23 PM Dilip Kumar wrote: > > > > On Mon, Sep 14, 2020 at 8:48 AM Amit Kapila wrote: > > > > > > > > > Yeah, this is right, and here is some initial analysis. It seems to be > > > failing in below code: > > >

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

2020-09-14 Thread Amit Kapila
On Mon, Sep 14, 2020 at 1:23 PM Dilip Kumar wrote: > > On Mon, Sep 14, 2020 at 8:48 AM Amit Kapila wrote: > > > > > > Yeah, this is right, and here is some initial analysis. It seems to be > > failing in below code: > > rel_sync_cache_relation_cb(){ ...list_free(entry->streamed_txns);..} > > > >

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

2020-09-14 Thread Dilip Kumar
On Mon, Sep 14, 2020 at 8:48 AM Amit Kapila wrote: > > On Mon, Sep 14, 2020 at 3:08 AM Tom Lane wrote: > > > > Amit Kapila writes: > > > Pushed. > > > > Observe the following reports: > > > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=idiacanthus=2020-09-13%2016%3A54%3A03 > >

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

2020-09-13 Thread Amit Kapila
On Mon, Sep 14, 2020 at 3:08 AM Tom Lane wrote: > > Amit Kapila writes: > > Pushed. > > Observe the following reports: > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=idiacanthus=2020-09-13%2016%3A54%3A03 >

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

2020-09-13 Thread Tom Lane
I wrote: > * Starting over, it appears that 001_rep_changes.pl almost immediately > gets into an infinite loop. It does not complete the third test step, > rather infinitely waiting for progress to be made. Ah, looking closer, the problem is that wal_receiver_timeout = 60s is too short when the

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

2020-09-13 Thread Amit Kapila
On Mon, Sep 14, 2020 at 3:08 AM Tom Lane wrote: > > Amit Kapila writes: > > Pushed. > > Observe the following reports: > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=idiacanthus=2020-09-13%2016%3A54%3A03 >

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

2020-09-13 Thread Tom Lane
I wrote: > Probably this requires a relcache inval at the wrong time; > although we have recent passes from CLOBBER_CACHE_ALWAYS animals, > so that can't be the whole triggering condition. I wonder whether > it is relevant that all of the complaining animals are JIT-enabled. Hmmm ... I take that

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

2020-09-13 Thread Tom Lane
Amit Kapila writes: > Pushed. Observe the following reports: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=idiacanthus=2020-09-13%2016%3A54%3A03 https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=desmoxytes=2020-09-10%2009%3A08%3A03

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

2020-09-12 Thread Amit Kapila
On Wed, Sep 9, 2020 at 2:26 PM Amit Kapila wrote: > > On Wed, Sep 9, 2020 at 2:13 PM Tomas Vondra > wrote: > > > > Hi, > > > > while looking at the streaming code I noticed two minor issues: > > > > 1) logicalrep_read_stream_stop is never defined/called, so the prototype > > in logicalproto.h is

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

2020-09-09 Thread Dilip Kumar
On Wed, Sep 9, 2020 at 2:13 PM Tomas Vondra wrote: > Hi, > > while looking at the streaming code I noticed two minor issues: > > 1) logicalrep_read_stream_stop is never defined/called, so the prototype > in logicalproto.h is unnecessary > > Yeah, right. > 2) minor typo in one of the comments >

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

2020-09-09 Thread Amit Kapila
On Wed, Sep 9, 2020 at 2:13 PM Tomas Vondra wrote: > > Hi, > > while looking at the streaming code I noticed two minor issues: > > 1) logicalrep_read_stream_stop is never defined/called, so the prototype > in logicalproto.h is unnecessary > > 2) minor typo in one of the comments > > Patch

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

2020-09-09 Thread Tomas Vondra
Hi, while looking at the streaming code I noticed two minor issues: 1) logicalrep_read_stream_stop is never defined/called, so the prototype in logicalproto.h is unnecessary 2) minor typo in one of the comments Patch attached. regards -- Tomas Vondra

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

2020-09-08 Thread Amit Kapila
On Mon, Sep 7, 2020 at 12:57 PM Dilip Kumar wrote: > > On Mon, Sep 7, 2020 at 12:00 PM Amit Kapila wrote: >> >> On Sat, Sep 5, 2020 at 8:55 PM Dilip Kumar wrote: >> > >> > >> > I have reviewed the changes and looks fine to me. >> > >> >> Thanks, I have pushed the last patch. Let's wait for a

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

2020-09-07 Thread Dilip Kumar
On Mon, Sep 7, 2020 at 12:00 PM Amit Kapila wrote: > On Sat, Sep 5, 2020 at 8:55 PM Dilip Kumar wrote: > > > > > > I have reviewed the changes and looks fine to me. > > > > Thanks, I have pushed the last patch. Let's wait for a day or so to > see the buildfarm reports and then we can probably

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

2020-09-07 Thread Amit Kapila
On Sat, Sep 5, 2020 at 8:55 PM Dilip Kumar wrote: > > > I have reviewed the changes and looks fine to me. > Thanks, I have pushed the last patch. Let's wait for a day or so to see the buildfarm reports and then we can probably close this CF entry. I am aware that we have one patch related to

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

2020-09-05 Thread Dilip Kumar
On Sat, 5 Sep 2020 at 4:02 PM, Amit Kapila wrote: > On Tue, Sep 1, 2020 at 8:33 PM Amit Kapila > wrote: > > > > > > On Tue, Sep 1, 2020 at 9:28 AM Amit Kapila > wrote: > > > > > > I have fixed all the comments except the below comments. > > > 1. verify the size of various tests to ensure that

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

2020-09-05 Thread Amit Kapila
On Tue, Sep 1, 2020 at 8:33 PM Amit Kapila wrote: > > On Tue, Sep 1, 2020 at 9:28 AM Amit Kapila wrote: > > I have fixed all the comments except the below comments. > 1. verify the size of various tests to ensure that it is above > logical_decoding_work_mem. > 2. I have checked that in one of

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

2020-09-03 Thread Amit Kapila
On Fri, Sep 4, 2020 at 3:10 AM Bossart, Nathan wrote: > > I noticed a small compiler warning for this. > > diff --git a/src/backend/replication/logical/worker.c > b/src/backend/replication/logical/worker.c > index 812aca8011..88d3444c39 100644 > --- a/src/backend/replication/logical/worker.c >

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

2020-09-03 Thread Bossart, Nathan
I noticed a small compiler warning for this. diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 812aca8011..88d3444c39 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -199,7 +199,7 @@

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

2020-09-02 Thread Dilip Kumar
On Wed, Sep 2, 2020 at 7:19 PM Amit Kapila wrote: > On Wed, Sep 2, 2020 at 3:41 PM Dilip Kumar wrote: > > > > On Wed, Sep 2, 2020 at 10:55 AM Amit Kapila > wrote: > > > > > > > > > > > > > We can combine the tests in 015_stream_simple.pl and > > > 020_stream_binary.pl as I can't see a good

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

2020-09-02 Thread Amit Kapila
On Wed, Sep 2, 2020 at 3:41 PM Dilip Kumar wrote: > > On Wed, Sep 2, 2020 at 10:55 AM Amit Kapila wrote: > > > > > > > > > We can combine the tests in 015_stream_simple.pl and > > 020_stream_binary.pl as I can't see a good reason to keep them > > separate. Then, I think we can keep only this

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

2020-09-02 Thread Dilip Kumar
On Tue, Sep 1, 2020 at 8:33 PM Amit Kapila wrote: > > On Tue, Sep 1, 2020 at 9:28 AM Amit Kapila wrote: > > > > On Mon, Aug 31, 2020 at 7:28 PM Dilip Kumar wrote: > > > > > > On Mon, Aug 31, 2020 at 1:24 PM Amit Kapila > > > wrote: > > > > In functions cleanup_rel_sync_cache and > >

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

2020-09-01 Thread Amit Kapila
On Tue, Sep 1, 2020 at 8:33 PM Amit Kapila wrote: > > I have fixed all the comments except .. > 3. +# Change the local values of the extra columns on the subscriber, > +# update publisher, and check that subscriber retains the expected > +# values > +$node_subscriber->safe_psql('postgres',

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

2020-08-31 Thread Amit Kapila
On Mon, Aug 31, 2020 at 10:27 PM Neha Sharma wrote: > > Hi Amit/Dilip, > > I have tested a few scenarios on top of the v56 patches, where the > replication worker still had few subtransactions in uncommitted state and we > restart the publisher server. > No crash or data discrepancies were

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

2020-08-31 Thread Amit Kapila
On Mon, Aug 31, 2020 at 7:28 PM Dilip Kumar wrote: > > On Mon, Aug 31, 2020 at 1:24 PM Amit Kapila wrote: > > > > On Mon, Aug 31, 2020 at 10:49 AM Amit Kapila > > wrote: > > > > > > On Sun, Aug 30, 2020 at 2:43 PM Dilip Kumar wrote: > > > > > > > > > > Another comment: > > > > > >

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

2020-08-31 Thread Neha Sharma
Hi Amit/Dilip, I have tested a few scenarios on top of the v56 patches, where the replication worker still had few subtransactions in uncommitted state and we restart the publisher server. No crash or data discrepancies were observed, attached are the test scenarios verified. *Data Setup:*

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

2020-08-31 Thread Amit Kapila
On Mon, Aug 31, 2020 at 1:24 PM Amit Kapila wrote: > > > 2. > 009_stream_simple.pl > +# Insert, update and delete enough rows to exceed the 64kB limit. > +$node_publisher->safe_psql('postgres', q{ > +BEGIN; > +INSERT INTO test_tab SELECT i, md5(i::text) FROM generate_series(3, 5000) > s(i); >

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

2020-08-30 Thread Amit Kapila
On Sun, Aug 30, 2020 at 2:43 PM Dilip Kumar wrote: > > On Sat, Aug 29, 2020 at 5:18 PM Amit Kapila wrote: > > > > > One more comment for which I haven't done anything yet. > > +static void > > +set_schema_sent_in_streamed_txn(RelationSyncEntry *entry, TransactionId > > xid) > > +{ > > +

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

2020-08-29 Thread Amit Kapila
On Fri, Aug 28, 2020 at 2:18 PM Dilip Kumar wrote: > > As discussed, I have added a another test case for covering the out of > order subtransaction rollback scenario. > +# large (streamed) transaction with out of order subtransaction ROLLBACKs +$node_publisher->safe_psql('postgres', q{ How

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

2020-08-28 Thread Neha Sharma
Hi, I have done code coverage analysis on the latest patches(v53) and below is the report for the same. Highlighted are the files where the coverage modifications were observed. OS: Ubuntu 18.04 Patch applied on commit : 77c7267c37f7fa8e5e48abda4798afdbecb2b95a File Name Coverage Without

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

2020-08-28 Thread Dilip Kumar
On Thu, Aug 27, 2020 at 11:16 AM Amit Kapila wrote: > > On Wed, Aug 26, 2020 at 11:22 PM Jeff Janes wrote: > > > > > > On Tue, Aug 25, 2020 at 8:58 AM Amit Kapila wrote: > > > >> > >> I am planning > >> to push the first patch (v53-0001-Extend-the-BufFile-interface) in > >> this series

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

2020-08-26 Thread Amit Kapila
On Wed, Aug 26, 2020 at 11:22 PM Jeff Janes wrote: > > > On Tue, Aug 25, 2020 at 8:58 AM Amit Kapila wrote: > >> >> I am planning >> to push the first patch (v53-0001-Extend-the-BufFile-interface) in >> this series tomorrow unless you have any comments on the same. > > > > I'm getting compiler

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

2020-08-26 Thread Jeff Janes
On Tue, Aug 25, 2020 at 8:58 AM Amit Kapila wrote: > I am planning > to push the first patch (v53-0001-Extend-the-BufFile-interface) in > this series tomorrow unless you have any comments on the same. > I'm getting compiler warnings now, src/backend/storage/file/sharedfileset.c line 288

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

2020-08-25 Thread Dilip Kumar
On Tue, Aug 25, 2020 at 6:27 PM Amit Kapila wrote: > > On Tue, Aug 25, 2020 at 10:41 AM Dilip Kumar wrote: > > > > On Tue, Aug 25, 2020 at 9:31 AM Amit Kapila wrote: > > > > > > > > > I think the existing design is superior as it allows the flexibility > > > to create transaction files in

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

2020-08-24 Thread Dilip Kumar
On Tue, Aug 25, 2020 at 9:31 AM Amit Kapila wrote: > > On Mon, Aug 24, 2020 at 9:41 PM Dilip Kumar wrote: > > > > On Sat, Aug 22, 2020 at 8:38 AM Amit Kapila wrote: > > > > > > On Fri, Aug 21, 2020 at 5:10 PM Dilip Kumar wrote: > > > > > > > > I have reviewed and tested the patch and the

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

2020-08-24 Thread Amit Kapila
On Mon, Aug 24, 2020 at 9:41 PM Dilip Kumar wrote: > > On Sat, Aug 22, 2020 at 8:38 AM Amit Kapila wrote: > > > > On Fri, Aug 21, 2020 at 5:10 PM Dilip Kumar wrote: > > > > > > I have reviewed and tested the patch and the changes look fine to me. > > > > > > > Thanks, I will push the next patch

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

2020-08-24 Thread Dilip Kumar
On Sat, Aug 22, 2020 at 8:38 AM Amit Kapila wrote: > > On Fri, Aug 21, 2020 at 5:10 PM Dilip Kumar wrote: > > > > I have reviewed and tested the patch and the changes look fine to me. > > > > Thanks, I will push the next patch early next week (by Tuesday) unless > you or someone else has any

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

2020-08-21 Thread Amit Kapila
On Fri, Aug 21, 2020 at 5:10 PM Dilip Kumar wrote: > > I have reviewed and tested the patch and the changes look fine to me. > Thanks, I will push the next patch early next week (by Tuesday) unless you or someone else has any more comments on it. The summary of the patch

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

2020-08-21 Thread Dilip Kumar
On Fri, Aug 21, 2020 at 3:13 PM Amit Kapila wrote: > > On Fri, Aug 21, 2020 at 10:33 AM Dilip Kumar wrote: > > > > On Fri, Aug 21, 2020 at 9:14 AM Amit Kapila wrote: > > > > > > 2. > > > + /* > > > + * If the new location is smaller then the current location in file then > > > + * we need to

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

2020-08-20 Thread Dilip Kumar
On Fri, Aug 21, 2020 at 10:20 AM Amit Kapila wrote: > > On Fri, Aug 21, 2020 at 9:14 AM Amit Kapila wrote: > > > > On Thu, Aug 20, 2020 at 5:42 PM Dilip Kumar wrote: > > > > > > On Thu, Aug 20, 2020 at 2:30 PM Amit Kapila > > > wrote: > > > > > > > > > > > > Right, I think this can happen if

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

2020-08-20 Thread Dilip Kumar
On Fri, Aug 21, 2020 at 9:14 AM Amit Kapila wrote: > > On Thu, Aug 20, 2020 at 5:42 PM Dilip Kumar wrote: > > > > On Thu, Aug 20, 2020 at 2:30 PM Amit Kapila wrote: > > > > > > > > > Right, I think this can happen if one has changed those by BufFileSeek > > > before doing truncate. We should

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

2020-08-20 Thread Amit Kapila
On Fri, Aug 21, 2020 at 9:14 AM Amit Kapila wrote: > > On Thu, Aug 20, 2020 at 5:42 PM Dilip Kumar wrote: > > > > On Thu, Aug 20, 2020 at 2:30 PM Amit Kapila wrote: > > > > > > > > > Right, I think this can happen if one has changed those by BufFileSeek > > > before doing truncate. We should

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

2020-08-20 Thread Amit Kapila
On Fri, Aug 21, 2020 at 9:14 AM Amit Kapila wrote: > > On Thu, Aug 20, 2020 at 5:42 PM Dilip Kumar wrote: > > > > On Thu, Aug 20, 2020 at 2:30 PM Amit Kapila wrote: > > > > > > > > > Right, I think this can happen if one has changed those by BufFileSeek > > > before doing truncate. We should

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

2020-08-20 Thread Amit Kapila
On Thu, Aug 20, 2020 at 5:42 PM Dilip Kumar wrote: > > On Thu, Aug 20, 2020 at 2:30 PM Amit Kapila wrote: > > > > > > Right, I think this can happen if one has changed those by BufFileSeek > > before doing truncate. We should fix that case as well. > > Right. > > > > I will work on those along

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

2020-08-20 Thread Amit Kapila
On Thu, Aug 20, 2020 at 1:41 PM Dilip Kumar wrote: > > On Wed, Aug 19, 2020 at 1:35 PM Dilip Kumar wrote: > > > > On Wed, Aug 19, 2020 at 12:20 PM Amit Kapila > > wrote: > > > > > > On Wed, Aug 19, 2020 at 10:10 AM Amit Kapila > > > wrote: > > > > > > > > On Mon, Aug 17, 2020 at 6:29 PM

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

2020-08-20 Thread Dilip Kumar
On Wed, Aug 19, 2020 at 1:35 PM Dilip Kumar wrote: > > On Wed, Aug 19, 2020 at 12:20 PM Amit Kapila wrote: > > > > On Wed, Aug 19, 2020 at 10:10 AM Amit Kapila > > wrote: > > > > > > On Mon, Aug 17, 2020 at 6:29 PM Dilip Kumar wrote: > > > > > > > > > > > > In last patch v49-0001, there is

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

2020-08-19 Thread Dilip Kumar
On Wed, Aug 19, 2020 at 12:20 PM Amit Kapila wrote: > > On Wed, Aug 19, 2020 at 10:10 AM Amit Kapila wrote: > > > > On Mon, Aug 17, 2020 at 6:29 PM Dilip Kumar wrote: > > > > > > > > > In last patch v49-0001, there is one issue, Basically, I have called > > > BufFileFlush in all the cases.

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

2020-08-19 Thread Dilip Kumar
On Wed, Aug 19, 2020 at 10:11 AM Amit Kapila wrote: > > On Mon, Aug 17, 2020 at 6:29 PM Dilip Kumar wrote: > > > > > > In last patch v49-0001, there is one issue, Basically, I have called > > BufFileFlush in all the cases. But, ideally, we can not call this if > > the underlying files are

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

2020-08-19 Thread Amit Kapila
On Wed, Aug 19, 2020 at 10:10 AM Amit Kapila wrote: > > On Mon, Aug 17, 2020 at 6:29 PM Dilip Kumar wrote: > > > > > > In last patch v49-0001, there is one issue, Basically, I have called > > BufFileFlush in all the cases. But, ideally, we can not call this if > > the underlying files are

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

2020-08-18 Thread Amit Kapila
On Mon, Aug 17, 2020 at 6:29 PM Dilip Kumar wrote: > > > In last patch v49-0001, there is one issue, Basically, I have called > BufFileFlush in all the cases. But, ideally, we can not call this if > the underlying files are deleted/truncated because those files/blocks > might not exist now. So

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

2020-08-15 Thread Dilip Kumar
On Thu, Aug 13, 2020 at 6:47 PM Amit Kapila wrote: > > On Thu, Aug 13, 2020 at 12:08 PM Amit Kapila wrote: > > > > On Fri, Aug 7, 2020 at 2:04 PM Dilip Kumar wrote: > > > > > > On Thu, Aug 6, 2020 at 2:43 PM Amit Kapila > > > wrote: > > > > > > .. > > > > This patch's functionality can be

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

2020-08-14 Thread Amit Kapila
On Sat, Aug 15, 2020 at 4:14 AM Thomas Munro wrote: > > On Fri, Aug 14, 2020 at 6:14 PM Amit Kapila wrote: > > Yeah, that makes sense. I will take care of that later today or > > tomorrow. We have not noticed that because currently none of the > > extensions is using those functions. BTW, I

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

2020-08-14 Thread Thomas Munro
On Fri, Aug 14, 2020 at 6:14 PM Amit Kapila wrote: > Yeah, that makes sense. I will take care of that later today or > tomorrow. We have not noticed that because currently none of the > extensions is using those functions. BTW, I noticed that after > failure, the next run is green, why so? Is the

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

2020-08-14 Thread Amit Kapila
On Fri, Aug 14, 2020 at 10:11 AM Thomas Munro wrote: > > On Thu, Aug 13, 2020 at 6:38 PM Amit Kapila wrote: > > I have pushed that patch last week and attached are the remaining > > patches. I have made a few changes in the next patch > > 0001-Extend-the-BufFile-interface.patch and have some

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

2020-08-13 Thread Thomas Munro
On Thu, Aug 13, 2020 at 6:38 PM Amit Kapila wrote: > I have pushed that patch last week and attached are the remaining > patches. I have made a few changes in the next patch > 0001-Extend-the-BufFile-interface.patch and have some comments on it > which are as below: Hi Amit, I noticed that

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

2020-08-13 Thread Amit Kapila
On Thu, Aug 13, 2020 at 12:08 PM Amit Kapila wrote: > > On Fri, Aug 7, 2020 at 2:04 PM Dilip Kumar wrote: > > > > On Thu, Aug 6, 2020 at 2:43 PM Amit Kapila wrote: > > > > .. > > > This patch's functionality can be independently verified by SQL APIs > > > > Your changes look fine to me. > > > >

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

2020-08-07 Thread Dilip Kumar
On Thu, Aug 6, 2020 at 2:43 PM Amit Kapila wrote: > > On Wed, Aug 5, 2020 at 7:37 PM Dilip Kumar wrote: > > > > On Wed, Aug 5, 2020 at 6:25 PM Amit Kapila wrote: > > > > > > > > Can we add a test for incomplete changes (probably with toast > > > insertion but we can do it for spec_insert case

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

2020-08-05 Thread Amit Kapila
On Tue, Aug 4, 2020 at 12:42 PM Dilip Kumar wrote: > > On Tue, Aug 4, 2020 at 10:12 AM Amit Kapila wrote: > > > > > 4. I think we can explain the problems (like we can see the wrong > > tuple or see two versions of the same tuple or whatever else wrong can > > happen, if possible with some

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

2020-08-03 Thread Amit Kapila
On Wed, Jul 29, 2020 at 10:46 AM Dilip Kumar wrote: > > Thanks, please find the rebased patch set. > Few comments on v44-0001-Implement-streaming-mode-in-ReorderBuffer: 1. +-- streaming with subxact, nothing in main +BEGIN; +savepoint

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

2020-07-31 Thread Ajin Cherian
> Attaching an updated patch for the stats for streaming based on v2 of > Sawada's san replication slot stats framework and v44 of this patch series > . This is one patch that has both the stats framework from Sawada-san (1) > as well as my update for streaming, so it can be applied easily on top

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

2020-07-30 Thread Amit Kapila
On Thu, Jul 30, 2020 at 12:28 PM Ajin Cherian wrote: > > I was running some tests on this patch. I was generally trying to see how the > patch affects logical replication when doing bulk inserts. This issue has > been raised in the past, for eg: this [1]. > My test setup is: > 1. Two postgres

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

2020-07-30 Thread Ajin Cherian
On Wed, Jul 29, 2020 at 3:16 PM Dilip Kumar wrote: > > > Thanks, please find the rebased patch set. > > -- > Regards, > Dilip Kumar > EnterpriseDB: http://www.enterprisedb.com I was running some tests on this patch. I was generally trying to see how the patch affects logical replication when

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

2020-07-27 Thread Amit Kapila
On Sun, Jul 26, 2020 at 11:04 AM Dilip Kumar wrote: > > > Today, I have again looked at the first patch > > (v42-0001-Extend-the-logical-decoding-output-plugin-API-wi) and didn't > > find any more problems with it so planning to commit the same unless > > you or someone else want to add more to

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

2020-07-25 Thread Dilip Kumar
On Sat, Jul 25, 2020 at 5:08 PM Amit Kapila wrote: > > On Fri, Jul 24, 2020 at 7:17 PM Dilip Kumar wrote: > > > > Your changes look fine to me. Additionally, I have changed a test > > case of getting the streaming changes in 0002. Instead of just > > showing the count, I am showing that the

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

2020-07-25 Thread Amit Kapila
On Fri, Jul 24, 2020 at 7:17 PM Dilip Kumar wrote: > > Your changes look fine to me. Additionally, I have changed a test > case of getting the streaming changes in 0002. Instead of just > showing the count, I am showing that the transaction is actually > streaming. > If you want to show the

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

2020-07-23 Thread Amit Kapila
On Wed, Jul 22, 2020 at 4:55 PM Dilip Kumar wrote: > > You are right. I have changed it. > Thanks, I have pushed the second patch in this series which is 0001-WAL-Log-invalidations-at-command-end-with-wal_le in your latest patch. I will continue working on remaining patches. -- With Regards,

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

2020-07-22 Thread Amit Kapila
On Wed, Jul 22, 2020 at 10:20 AM Dilip Kumar wrote: > > On Wed, Jul 22, 2020 at 9:18 AM Amit Kapila wrote: > > > > On Mon, Jul 20, 2020 at 6:46 PM Dilip Kumar wrote: > > > > > > There was one warning in release mode in the last version in 0004 so > > > attaching a new version. > > > > > > >

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

2020-07-21 Thread Amit Kapila
On Mon, Jul 20, 2020 at 6:46 PM Dilip Kumar wrote: > > There was one warning in release mode in the last version in 0004 so > attaching a new version. > Today, I was reviewing patch v38-0001-WAL-Log-invalidations-at-command-end-with-wal_le and found a small problem with it. + /* + * Execute the

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

2020-07-20 Thread Ajin Cherian
On Mon, Jul 20, 2020 at 11:16 PM Dilip Kumar wrote: > > > There was one warning in release mode in the last version in 0004 so > attaching a new version. > > -- > Regards, > Dilip Kumar > EnterpriseDB: http://www.enterprisedb.com Hello, I have tried to rework the patch which did the stats

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

2020-07-20 Thread Amit Kapila
On Mon, Jul 20, 2020 at 12:01 PM Dilip Kumar wrote: > > On Thu, Jul 16, 2020 at 4:25 PM Amit Kapila wrote: > > > > On Thu, Jul 16, 2020 at 12:23 PM Dilip Kumar wrote: > > > > > > On Wed, Jul 15, 2020 at 6:59 PM Amit Kapila > > > wrote: > > > > > > > > > > > > Let me know what you think of the

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

2020-07-16 Thread Dilip Kumar
On Wed, Jul 15, 2020 at 6:59 PM Amit Kapila wrote: > > On Wed, Jul 15, 2020 at 9:29 AM Dilip Kumar wrote: > > > > > > I have reviewed your changes and those look good to me, please find > > the latest version of the patch set. > > > > I have done an additional round of review and below are the

  1   2   3   4   5   >