Re: PATCH: logical_work_mem and logical streaming of largein-progress transactions

Поиск
Список
Период
Сортировка
От Amit Kapila
Тема Re: PATCH: logical_work_mem and logical streaming of largein-progress transactions
Дата
Msg-id CAA4eK1KB_N-gUNgUN9KysJyBnnVWd7_sE9x53EYQyv3XjVGojw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: PATCH: logical_work_mem and logical streaming of largein-progress transactions  (Tomas Vondra <tomas.vondra@2ndquadrant.com>)
Ответы Re: PATCH: logical_work_mem and logical streaming of largein-progress transactions  (Tomas Vondra <tomas.vondra@2ndquadrant.com>)
Список pgsql-hackers
On Tue, Oct 1, 2019 at 7:21 PM Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
On Tue, Oct 01, 2019 at 06:55:52PM +0530, Amit Kapila wrote:
>
>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.

It was an assertion failure in ReorderBufferCleanupTXN at below line:
+ /* Check we're not mixing changes from different transactions. */
+ Assert(change->txn == txn);

 
>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 like that idea, but I think you need to split that patch to only get the stats related to the spill.  It would be easier to review if you can prepare that atop of 0001-Add-logical_decoding_work_mem-to-limit-ReorderBuffer.

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

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: Re: Modest proposal for making bpchar less inconsistent
Следующее
От: "Smith, Peter"
Дата:
Сообщение: RE: Proposal: Make use of C99 designated initialisers fornulls/values arrays