On Sun, Sep 29, 2019 at 11:24 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > On Sun, Sep 29, 2019 at 12:39 AM Tomas Vondra > <tomas.von...@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. 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. I am reluctant to say, but I think this patch still needs some more work (review, test, rework) before we can commit it. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com