From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
Date: Tue, 1 Oct 2019 15:51:51 +0200
Message-ID: <20191001135151.swlspdnkxxnhszhn@development> (raw)
In-Reply-To: <CAA4eK1+-F_ELz+n3S9bBN8kOgZ6ww67tOoy99f8Ug39RWXH20w@mail.gmail.com>
References: <688b0b7f-2f6c-d827-c27b-216a8e3ea700@2ndquadrant.com>
<3530384f-f73a-186a-a728-8eeeb5533a10@2ndquadrant.com>
<262cc9df-0d19-3fca-f899-0890bd2cd4be@2ndquadrant.com>
<47e505c1-9a21-9edf-9a97-e13acce5aa21@2ndquadrant.com>
<CAA4eK1Jz_8Sa8rHmUfJZgNpUUWqu9uqiyCmwAzqmGbRxyObgSg@mail.gmail.com>
<20190927112520.3kco6tqbhcvlk6cd@development>
<CAA4eK1J7NajKPE58Vnf5pBtwR3upxATUCRFoR04BPKbKBes5hw@mail.gmail.com>
<20190928190917.hrpknmq76v3ts3lj@development>
<CAA4eK1LCk4aAeTv8SofwgmKLaFakj+Sn4Y7MLEJORX0BjY2hGw@mail.gmail.com>
<CAA4eK1+-F_ELz+n3S9bBN8kOgZ6ww67tOoy99f8Ug39RWXH20w@mail.gmail.com>
On Tue, Oct 01, 2019 at 06:55:52PM +0530, Amit Kapila wrote:
>On Sun, Sep 29, 2019 at 11:24 AM Amit Kapila <amit.kapila16@gmail.com>
>wrote:
>> On Sun, Sep 29, 2019 at 12:39 AM Tomas Vondra
>> <tomas.vondra@2ndquadrant.com> wrote:
>> >
>>
>> 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.
>>
>
>On further testing, I found that the patch seems to have problems with
>toast. Consider below scenario:
>Session-1
>Create table large_text(t1 text);
>INSERT INTO large_text
>SELECT (SELECT string_agg('x', ',')
>FROM generate_series(1, 1000000)) FROM generate_series(1, 1000);
>
>Session-2
>SELECT * FROM pg_create_logical_replication_slot('regression_slot',
>'test_decoding');
>SELECT * FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL);
>*--kaboom*
>
>The second statement in Session-2 leads to a crash.
>
OK, thanks for the report - will investigate.
>Other than that, I am not sure if the changes related to spill to disk
>after logical_decoding_work_mem works for toast table as I couldn't hit
>that code for toast table case, but I might be missing something. As
>mentioned previously, I feel there should be some way to test whether this
>patch works for the cases it claims to work. As of now, I have to check
>via debugging. Let me know if there is any way, I can test this.
>
That's one of the reasons why I proposed to move the statistics (which
say how many transactions / bytes were spilled to disk) from a later
patch in the series. I don't think there's a better way.
>I am reluctant to say, but I think this patch still needs some more work
>(review, test, rework) before we can commit it.
>
I agreee.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, amit.kapila16@gmail.com, peter.eisentraut@2ndquadrant.com
Subject: Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
In-Reply-To: <20191001135151.swlspdnkxxnhszhn@development>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox