Horton

Thread

Commits

  1. logical decoding: Remove TODO about unnecessary optimization.

  2. Stamp 12.2.

  1. Fastpath while arranging the changes in LSN order in logical decoding

    Dilip Kumar <dilipbalaut@gmail.com> — 2019-11-25T03:52:49Z

    In logical decoding, while sending the changes to the output plugin we
need to arrange them in the LSN order.  But, if there is only one
transaction which is a very common case then we can avoid building the
binary heap.  A small patch is attached for the same.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

  2. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Dilip Kumar <dilipbalaut@gmail.com> — 2019-12-17T03:03:13Z

    On Mon, Nov 25, 2019 at 9:22 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> In logical decoding, while sending the changes to the output plugin we
> need to arrange them in the LSN order.  But, if there is only one
> transaction which is a very common case then we can avoid building the
> binary heap.  A small patch is attached for the same.

I have registered it in the next commitfest.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

  3. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Heikki Linnakangas <hlinnaka@iki.fi> — 2020-01-08T11:58:23Z

    On 25/11/2019 05:52, Dilip Kumar wrote:
> In logical decoding, while sending the changes to the output plugin we
> need to arrange them in the LSN order.  But, if there is only one
> transaction which is a very common case then we can avoid building the
> binary heap.  A small patch is attached for the same.

Does this make any measurable performance difference? Building a 
one-element binary heap seems pretty cheap.

- Heikki

  4. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Dilip Kumar <dilipbalaut@gmail.com> — 2020-01-08T12:36:52Z

    On Wed, 8 Jan 2020 at 5:28 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

> On 25/11/2019 05:52, Dilip Kumar wrote:
> > In logical decoding, while sending the changes to the output plugin we
> > need to arrange them in the LSN order.  But, if there is only one
> > transaction which is a very common case then we can avoid building the
> > binary heap.  A small patch is attached for the same.
>
> Does this make any measurable performance difference? Building a
> one-element binary heap seems pretty cheap.


I haven’t really measured the performance for this.  I will try to do that
next week.  Thanks for looking into this.

>
> --
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

  5. Re: Fastpath while arranging the changes in LSN order in logical decoding

    David Zhang <david.zhang@highgo.ca> — 2020-02-19T00:16:09Z

    1. Tried to apply the patch to PG 12.2 commit 45b88269a353ad93744772791feb6d01bc7e1e42 (HEAD -> REL_12_2, tag: REL_12_2), it doesn't work. Then tried to check the patch, and found the errors showing below.
$ git apply --check 0001-Fastpath-for-sending-changes-to-output-plugin-in-log.patch
error: patch failed: contrib/test_decoding/logical.conf:1
error: contrib/test_decoding/logical.conf: patch does not apply
error: patch failed: src/backend/replication/logical/reorderbuffer.c:1133
error: src/backend/replication/logical/reorderbuffer.c: patch does not apply

2. Ran a further check for file "logical.conf", and found there is only one commit since 2014, which doesn't have the parameter, "logical_decoding_work_mem = 64kB"

3. Manually apply the patch including src/backend/replication/logical/reorderbuffer.c, and then ran a simple logical replication test. A connection issue is found like below,
"table public.pgbench_accounts: INSERT: aid[integer]:4071 bid[integer]:1 abalance[integer]:0 filler[character]:'                                                                                    '
pg_recvlogical: error: could not receive data from WAL stream: server closed the connection unexpectedly
	This probably means the server terminated abnormally
	before or while processing the request.
pg_recvlogical: disconnected; waiting 5 seconds to try again"

4. This connection issue can be reproduced on PG 12.2 commit mentioned above, the basic steps,
4.1 Change "wal_level = logical" in "postgresql.conf"
4.2 create a logical slot and listen on it,
$ pg_recvlogical -d postgres --slot test --create-slot
$ pg_recvlogical -d postgres --slot test --start -f -

4.3 from another terminal, run the command below,
$ pgbench -i -p 5432 -d postgres

Let me know if I did something wrong, and if a new patch is available, I can re-run the test on the same environment.

-- 
David
Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca

  6. Re: Fastpath while arranging the changes in LSN order in logical decoding

    David Zhang <david.zhang@highgo.ca> — 2020-02-19T00:30:35Z

    After manually applied the patch, a diff regenerated is attached.

On 2020-02-18 4:16 p.m., David Zhang wrote:
> 1. Tried to apply the patch to PG 12.2 commit 45b88269a353ad93744772791feb6d01bc7e1e42 (HEAD -> REL_12_2, tag: REL_12_2), it doesn't work. Then tried to check the patch, and found the errors showing below.
> $ git apply --check 0001-Fastpath-for-sending-changes-to-output-plugin-in-log.patch
> error: patch failed: contrib/test_decoding/logical.conf:1
> error: contrib/test_decoding/logical.conf: patch does not apply
> error: patch failed: src/backend/replication/logical/reorderbuffer.c:1133
> error: src/backend/replication/logical/reorderbuffer.c: patch does not apply
>
> 2. Ran a further check for file "logical.conf", and found there is only one commit since 2014, which doesn't have the parameter, "logical_decoding_work_mem = 64kB"
>
> 3. Manually apply the patch including src/backend/replication/logical/reorderbuffer.c, and then ran a simple logical replication test. A connection issue is found like below,
> "table public.pgbench_accounts: INSERT: aid[integer]:4071 bid[integer]:1 abalance[integer]:0 filler[character]:'                                                                                    '
> pg_recvlogical: error: could not receive data from WAL stream: server closed the connection unexpectedly
> 	This probably means the server terminated abnormally
> 	before or while processing the request.
> pg_recvlogical: disconnected; waiting 5 seconds to try again"
>
> 4. This connection issue can be reproduced on PG 12.2 commit mentioned above, the basic steps,
> 4.1 Change "wal_level = logical" in "postgresql.conf"
> 4.2 create a logical slot and listen on it,
> $ pg_recvlogical -d postgres --slot test --create-slot
> $ pg_recvlogical -d postgres --slot test --start -f -
>
> 4.3 from another terminal, run the command below,
> $ pgbench -i -p 5432 -d postgres
>
> Let me know if I did something wrong, and if a new patch is available, I can re-run the test on the same environment.
>
-- 
David

Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca

  7. Re: Fastpath while arranging the changes in LSN order in logical decoding

    David Steele <david@pgmasters.net> — 2020-03-02T13:57:23Z

    Hi Dilip,

On 2/18/20 7:30 PM, David Zhang wrote:
> After manually applied the patch, a diff regenerated is attached.

David's updated patch applies but all logical decoding regression tests 
are failing on cfbot.

Do you know when you will be able to supply an updated patch?

Regards,
-- 
-David
david@pgmasters.net

  8. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Dilip Kumar <dilipbalaut@gmail.com> — 2020-03-03T03:12:52Z

    On Mon, Mar 2, 2020 at 7:27 PM David Steele <david@pgmasters.net> wrote:
>
> Hi Dilip,
>
> On 2/18/20 7:30 PM, David Zhang wrote:
> > After manually applied the patch, a diff regenerated is attached.
>
> David's updated patch applies but all logical decoding regression tests
> are failing on cfbot.
>
> Do you know when you will be able to supply an updated patch?

I will try to send in a day or two.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

  9. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Dilip Kumar <dilipbalaut@gmail.com> — 2020-03-03T05:11:11Z

    On Tue, Mar 3, 2020 at 8:42 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Mar 2, 2020 at 7:27 PM David Steele <david@pgmasters.net> wrote:
> >
> > Hi Dilip,
> >
> > On 2/18/20 7:30 PM, David Zhang wrote:
> > > After manually applied the patch, a diff regenerated is attached.
> >
> > David's updated patch applies but all logical decoding regression tests
> > are failing on cfbot.
> >
> > Do you know when you will be able to supply an updated patch?
>
> I will try to send in a day or two.

I have rebased the patch.  check-world is passing.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

  10. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Dilip Kumar <dilipbalaut@gmail.com> — 2020-03-03T05:11:27Z

    On Wed, Feb 19, 2020 at 6:00 AM David Zhang <david.zhang@highgo.ca> wrote:
>
> After manually applied the patch, a diff regenerated is attached.
>
> On 2020-02-18 4:16 p.m., David Zhang wrote:
> > 1. Tried to apply the patch to PG 12.2 commit 45b88269a353ad93744772791feb6d01bc7e1e42 (HEAD -> REL_12_2, tag: REL_12_2), it doesn't work. Then tried to check the patch, and found the errors showing below.
> > $ git apply --check 0001-Fastpath-for-sending-changes-to-output-plugin-in-log.patch
> > error: patch failed: contrib/test_decoding/logical.conf:1
> > error: contrib/test_decoding/logical.conf: patch does not apply
> > error: patch failed: src/backend/replication/logical/reorderbuffer.c:1133
> > error: src/backend/replication/logical/reorderbuffer.c: patch does not apply
> >
> > 2. Ran a further check for file "logical.conf", and found there is only one commit since 2014, which doesn't have the parameter, "logical_decoding_work_mem = 64kB"
> >
> > 3. Manually apply the patch including src/backend/replication/logical/reorderbuffer.c, and then ran a simple logical replication test. A connection issue is found like below,
> > "table public.pgbench_accounts: INSERT: aid[integer]:4071 bid[integer]:1 abalance[integer]:0 filler[character]:'                                                                                    '
> > pg_recvlogical: error: could not receive data from WAL stream: server closed the connection unexpectedly
> >       This probably means the server terminated abnormally
> >       before or while processing the request.
> > pg_recvlogical: disconnected; waiting 5 seconds to try again"
> >
> > 4. This connection issue can be reproduced on PG 12.2 commit mentioned above, the basic steps,
> > 4.1 Change "wal_level = logical" in "postgresql.conf"
> > 4.2 create a logical slot and listen on it,
> > $ pg_recvlogical -d postgres --slot test --create-slot
> > $ pg_recvlogical -d postgres --slot test --start -f -
> >
> > 4.3 from another terminal, run the command below,
> > $ pgbench -i -p 5432 -d postgres
> >
> > Let me know if I did something wrong, and if a new patch is available, I can re-run the test on the same environment.

Thanks for testing and rebasing.  I think one of the hunks is missing
in your rebased version.  That could be the reason for failure.  Can
you test on my latest version?

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

  11. Re: Fastpath while arranging the changes in LSN order in logical decoding

    David Zhang <david.zhang@highgo.ca> — 2020-03-03T21:32:05Z

    Hi Dilip,

I repeated the same test cases again and can't reproduce the 
disconnection issue after applied your new patch.

Best regards,

David

On 2020-03-02 9:11 p.m., Dilip Kumar wrote:
> On Wed, Feb 19, 2020 at 6:00 AM David Zhang <david.zhang@highgo.ca> wrote:
>> After manually applied the patch, a diff regenerated is attached.
>>
>> On 2020-02-18 4:16 p.m., David Zhang wrote:
>>> 1. Tried to apply the patch to PG 12.2 commit 45b88269a353ad93744772791feb6d01bc7e1e42 (HEAD -> REL_12_2, tag: REL_12_2), it doesn't work. Then tried to check the patch, and found the errors showing below.
>>> $ git apply --check 0001-Fastpath-for-sending-changes-to-output-plugin-in-log.patch
>>> error: patch failed: contrib/test_decoding/logical.conf:1
>>> error: contrib/test_decoding/logical.conf: patch does not apply
>>> error: patch failed: src/backend/replication/logical/reorderbuffer.c:1133
>>> error: src/backend/replication/logical/reorderbuffer.c: patch does not apply
>>>
>>> 2. Ran a further check for file "logical.conf", and found there is only one commit since 2014, which doesn't have the parameter, "logical_decoding_work_mem = 64kB"
>>>
>>> 3. Manually apply the patch including src/backend/replication/logical/reorderbuffer.c, and then ran a simple logical replication test. A connection issue is found like below,
>>> "table public.pgbench_accounts: INSERT: aid[integer]:4071 bid[integer]:1 abalance[integer]:0 filler[character]:'                                                                                    '
>>> pg_recvlogical: error: could not receive data from WAL stream: server closed the connection unexpectedly
>>>        This probably means the server terminated abnormally
>>>        before or while processing the request.
>>> pg_recvlogical: disconnected; waiting 5 seconds to try again"
>>>
>>> 4. This connection issue can be reproduced on PG 12.2 commit mentioned above, the basic steps,
>>> 4.1 Change "wal_level = logical" in "postgresql.conf"
>>> 4.2 create a logical slot and listen on it,
>>> $ pg_recvlogical -d postgres --slot test --create-slot
>>> $ pg_recvlogical -d postgres --slot test --start -f -
>>>
>>> 4.3 from another terminal, run the command below,
>>> $ pgbench -i -p 5432 -d postgres
>>>
>>> Let me know if I did something wrong, and if a new patch is available, I can re-run the test on the same environment.
> Thanks for testing and rebasing.  I think one of the hunks is missing
> in your rebased version.  That could be the reason for failure.  Can
> you test on my latest version?
>
-- 
David

Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca

  12. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Dilip Kumar <dilipbalaut@gmail.com> — 2020-03-04T03:03:59Z

    On Wed, Mar 4, 2020 at 3:02 AM David Zhang <david.zhang@highgo.ca> wrote:
>
> Hi Dilip,
>
> I repeated the same test cases again and can't reproduce the
> disconnection issue after applied your new patch.

Thanks for the confirmation.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

  13. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Andres Freund <andres@anarazel.de> — 2020-03-06T18:13:25Z

    Hi,

On 2020-01-08 18:06:52 +0530, Dilip Kumar wrote:
> On Wed, 8 Jan 2020 at 5:28 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> 
> > On 25/11/2019 05:52, Dilip Kumar wrote:
> > > In logical decoding, while sending the changes to the output plugin we
> > > need to arrange them in the LSN order.  But, if there is only one
> > > transaction which is a very common case then we can avoid building the
> > > binary heap.  A small patch is attached for the same.
> >
> > Does this make any measurable performance difference? Building a
> > one-element binary heap seems pretty cheap.
> 
> 
> I haven’t really measured the performance for this.  I will try to do that
> next week.  Thanks for looking into this.

Did you do that?

Regards,

Andres

  14. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Dilip Kumar <dilipbalaut@gmail.com> — 2020-03-07T04:29:38Z

    On Sat, Mar 7, 2020 at 12:30 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2020-01-08 18:06:52 +0530, Dilip Kumar wrote:
> > On Wed, 8 Jan 2020 at 5:28 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> >
> > > On 25/11/2019 05:52, Dilip Kumar wrote:
> > > > In logical decoding, while sending the changes to the output plugin we
> > > > need to arrange them in the LSN order.  But, if there is only one
> > > > transaction which is a very common case then we can avoid building the
> > > > binary heap.  A small patch is attached for the same.
> > >
> > > Does this make any measurable performance difference? Building a
> > > one-element binary heap seems pretty cheap.
> >
> >
> > I haven’t really measured the performance for this.  I will try to do that
> > next week.  Thanks for looking into this.
>
> Did you do that?

I tried once in my local machine but could not produce consistent
results.  I will try this once again in the performance machine and
report back.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

  15. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Dilip Kumar <dilipbalaut@gmail.com> — 2020-03-07T05:45:27Z

    On Sat, Mar 7, 2020 at 9:59 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Sat, Mar 7, 2020 at 12:30 AM Andres Freund <andres@anarazel.de> wrote:
> >
> > Hi,
> >
> > On 2020-01-08 18:06:52 +0530, Dilip Kumar wrote:
> > > On Wed, 8 Jan 2020 at 5:28 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> > >
> > > > On 25/11/2019 05:52, Dilip Kumar wrote:
> > > > > In logical decoding, while sending the changes to the output plugin we
> > > > > need to arrange them in the LSN order.  But, if there is only one
> > > > > transaction which is a very common case then we can avoid building the
> > > > > binary heap.  A small patch is attached for the same.
> > > >
> > > > Does this make any measurable performance difference? Building a
> > > > one-element binary heap seems pretty cheap.
> > >
> > >
> > > I haven’t really measured the performance for this.  I will try to do that
> > > next week.  Thanks for looking into this.
> >
> > Did you do that?
>
> I tried once in my local machine but could not produce consistent
> results.  I will try this once again in the performance machine and
> report back.

I have tried to decode changes for the 100,000 small transactions and
measured the time with head vs patch.  I did not observe any
significant gain.

Head
-------
519ms
500ms
487ms
501ms

patch
------
501ms
492ms
486ms
489ms

IMHO, if we conclude that because there is no performance gain so we
don't want to add one extra path in the code then we might want to
remove that TODO from the code so that we don't spend time for
optimizing this in the future.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

  16. Re: Fastpath while arranging the changes in LSN order in logical decoding

    James Coleman <jtc331@gmail.com> — 2020-03-08T15:54:56Z

    On Saturday, March 7, 2020, Dilip Kumar <dilipbalaut@gmail.com> wrote:

> On Sat, Mar 7, 2020 at 9:59 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Sat, Mar 7, 2020 at 12:30 AM Andres Freund <andres@anarazel.de>
> wrote:
> > >
> > > Hi,
> > >
> > > On 2020-01-08 18:06:52 +0530, Dilip Kumar wrote:
> > > > On Wed, 8 Jan 2020 at 5:28 PM, Heikki Linnakangas <hlinnaka@iki.fi>
> wrote:
> > > >
> > > > > On 25/11/2019 05:52, Dilip Kumar wrote:
> > > > > > In logical decoding, while sending the changes to the output
> plugin we
> > > > > > need to arrange them in the LSN order.  But, if there is only one
> > > > > > transaction which is a very common case then we can avoid
> building the
> > > > > > binary heap.  A small patch is attached for the same.
> > > > >
> > > > > Does this make any measurable performance difference? Building a
> > > > > one-element binary heap seems pretty cheap.
> > > >
> > > >
> > > > I haven’t really measured the performance for this.  I will try to
> do that
> > > > next week.  Thanks for looking into this.
> > >
> > > Did you do that?
> >
> > I tried once in my local machine but could not produce consistent
> > results.  I will try this once again in the performance machine and
> > report back.
>
> I have tried to decode changes for the 100,000 small transactions and
> measured the time with head vs patch.  I did not observe any
> significant gain.
>
> Head
> -------
> 519ms
> 500ms
> 487ms
> 501ms
>
> patch
> ------
> 501ms
> 492ms
> 486ms
> 489ms
>
> IMHO, if we conclude that because there is no performance gain so we
> don't want to add one extra path in the code then we might want to
> remove that TODO from the code so that we don't spend time for
> optimizing this in the future.
>

Would you be able to share your test setup? It seems like it’d helpful to
get a larger sample size to better determine if it’s measurable or not.
Visually those 4 runs look to me like it’s possible, but objectively I’m
not sure we can yet conclude one way or the other.

James

  17. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Dilip Kumar <dilipbalaut@gmail.com> — 2020-03-09T03:50:44Z

    On Sun, Mar 8, 2020 at 9:24 PM James Coleman <jtc331@gmail.com> wrote:
>
> On Saturday, March 7, 2020, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>>
>> On Sat, Mar 7, 2020 at 9:59 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>> >
>> > On Sat, Mar 7, 2020 at 12:30 AM Andres Freund <andres@anarazel.de> wrote:
>> > >
>> > > Hi,
>> > >
>> > > On 2020-01-08 18:06:52 +0530, Dilip Kumar wrote:
>> > > > On Wed, 8 Jan 2020 at 5:28 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> > > >
>> > > > > On 25/11/2019 05:52, Dilip Kumar wrote:
>> > > > > > In logical decoding, while sending the changes to the output plugin we
>> > > > > > need to arrange them in the LSN order.  But, if there is only one
>> > > > > > transaction which is a very common case then we can avoid building the
>> > > > > > binary heap.  A small patch is attached for the same.
>> > > > >
>> > > > > Does this make any measurable performance difference? Building a
>> > > > > one-element binary heap seems pretty cheap.
>> > > >
>> > > >
>> > > > I haven’t really measured the performance for this.  I will try to do that
>> > > > next week.  Thanks for looking into this.
>> > >
>> > > Did you do that?
>> >
>> > I tried once in my local machine but could not produce consistent
>> > results.  I will try this once again in the performance machine and
>> > report back.
>>
>> I have tried to decode changes for the 100,000 small transactions and
>> measured the time with head vs patch.  I did not observe any
>> significant gain.
>>
>> Head
>> -------
>> 519ms
>> 500ms
>> 487ms
>> 501ms
>>
>> patch
>> ------
>> 501ms
>> 492ms
>> 486ms
>> 489ms
>>
>> IMHO, if we conclude that because there is no performance gain so we
>> don't want to add one extra path in the code then we might want to
>> remove that TODO from the code so that we don't spend time for
>> optimizing this in the future.
>
>
> Would you be able to share your test setup? It seems like it’d helpful to get a larger sample size to better determine if it’s measurable or not. Visually those 4 runs look to me like it’s possible, but objectively I’m not sure we can yet conclude one way or the other.

Yeah, my test is very simple

CREATE TABLE t1 (a int, b int);
SELECT * FROM pg_create_logical_replication_slot('regression_slot',
'test_decoding');

--run 100,000 small transactions with pgbench
./pgbench -f test.sql -c 1 -j 1 -t 100000 -P 1  postgres;

--measure time to decode the changes
time ./psql -d postgres -c "select count(*) from
pg_logical_slot_get_changes('regression_slot', NULL,NULL);

*test.sql is just one insert query like below
insert into t1 values(1,1);

I guess this should be the best case to test this patch because we are
decoding a lot of small transactions but it seems the time taken for
creating the binary heap is quite small.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

  18. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Andres Freund <andres@anarazel.de> — 2020-03-09T17:37:03Z

    On 2020-03-07 11:15:27 +0530, Dilip Kumar wrote:
> IMHO, if we conclude that because there is no performance gain so we
> don't want to add one extra path in the code then we might want to
> remove that TODO from the code so that we don't spend time for
> optimizing this in the future.

+1

  19. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Amit Kapila <amit.kapila16@gmail.com> — 2020-03-24T12:46:28Z

    On Mon, Mar 9, 2020 at 11:07 PM Andres Freund <andres@anarazel.de> wrote:
>
> On 2020-03-07 11:15:27 +0530, Dilip Kumar wrote:
> > IMHO, if we conclude that because there is no performance gain so we
> > don't want to add one extra path in the code then we might want to
> > remove that TODO from the code so that we don't spend time for
> > optimizing this in the future.
>
> +1
>

Dilip, are you planning to do more tests for this?  Anyone else wants
to do more tests? If not, based on current results, we can remove that
TODO and in future, if someone comes with a test case to show benefit
for adding fastpath, then we can consider the patch proposed by Dilip.

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

  20. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Dilip Kumar <dilipbalaut@gmail.com> — 2020-03-24T13:06:03Z

    On Tue, Mar 24, 2020 at 6:16 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Mar 9, 2020 at 11:07 PM Andres Freund <andres@anarazel.de> wrote:
> >
> > On 2020-03-07 11:15:27 +0530, Dilip Kumar wrote:
> > > IMHO, if we conclude that because there is no performance gain so we
> > > don't want to add one extra path in the code then we might want to
> > > remove that TODO from the code so that we don't spend time for
> > > optimizing this in the future.
> >
> > +1
> >
>
> Dilip, are you planning to do more tests for this?  Anyone else wants
> to do more tests? If not, based on current results, we can remove that
> TODO and in future, if someone comes with a test case to show benefit
> for adding fastpath, then we can consider the patch proposed by Dilip.

IMHO, I have tried the best case but did not see any performance gain
so I am not planning to test this further.  I have attached the patch
for removing the TODO.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

  21. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Andres Freund <andres@anarazel.de> — 2020-03-24T19:16:51Z

    On 2020-03-24 18:36:03 +0530, Dilip Kumar wrote:
> IMHO, I have tried the best case but did not see any performance gain
> so I am not planning to test this further.  I have attached the patch
> for removing the TODO.

Pushed. Thanks!

  22. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Amit Kapila <amit.kapila16@gmail.com> — 2020-03-25T03:53:27Z

    On Wed, Mar 25, 2020 at 12:46 AM Andres Freund <andres@anarazel.de> wrote:
>
> On 2020-03-24 18:36:03 +0530, Dilip Kumar wrote:
> > IMHO, I have tried the best case but did not see any performance gain
> > so I am not planning to test this further.  I have attached the patch
> > for removing the TODO.
>
> Pushed. Thanks!
>

I have updated the CF entry.  Thanks to all involved in this.

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

  23. Re: Fastpath while arranging the changes in LSN order in logical decoding

    Dilip Kumar <dilipbalaut@gmail.com> — 2020-03-25T04:09:17Z

    On Wed, Mar 25, 2020 at 9:23 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Mar 25, 2020 at 12:46 AM Andres Freund <andres@anarazel.de> wrote:
> >
> > On 2020-03-24 18:36:03 +0530, Dilip Kumar wrote:
> > > IMHO, I have tried the best case but did not see any performance gain
> > > so I am not planning to test this further.  I have attached the patch
> > > for removing the TODO.
> >
> > Pushed. Thanks!
> >
>
> I have updated the CF entry.  Thanks to all involved in this.

Thanks!

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com