From: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
---|---|
To: | Jingtang Zhang <mrdrivingduck(at)gmail(dot)com> |
Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Make reorder buffer max_changes_in_memory adjustable? |
Date: | 2024-07-22 09:46:20 |
Message-ID: | e7789ba4-842d-4a1e-8893-cb607d12b397@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 7/22/24 05:28, Jingtang Zhang wrote:
> Thanks, Tomas.
>
>> Theoretically, yes, we could make max_changes_in_memory a GUC, but it's
>> not clear to me how would that help 12/13, because there's ~0% chance
>> we'd backpatch that ...
>
> What I mean is not about back-patch work. Things should happen on publisher
> side?
>
> Consider when the publisher is a PostgreSQL v14+~master (with streaming
> support) and subscriber is a 12/13 where streaming is not supported, the
> publisher
> would still have the risk of OOM. The same thing should happen when we use a
> v14+~master as publisher and a whatever open source CDC as subscriber.
>
Yes, you're right - if we're talking about mixed setups with just the
subscriber running the old version, then it would benefit from this
improvement even without backpatching.
>> Wouldn't it be better to have adjusts the value automatically, somehow?
>> For example, before restoring the changes, we could count the number of
>> transactions, and set it to 4096/ntransactions or something like that.
>> Or do something smarter by estimating tuple size, to count it in the
>> logical__decoding_work_mem budget.
>
> Yes, I think this issue should have been solved when
> logical_decoding_work_mem
> was initially been introduced, but it didn't. There could be some reasons
> like
> sub-transaction stuff which has been commented in the header of
> reorderbuffer.c.
>
True, but few patches are perfect/complete from V1. There's often stuff
that's unlikely to happen, left for future improvements. And this is one
of those cases, I believe. The fact that the comment even mentions this
is a sign the developers considered this, and chose to ignore it.
That being said, I think it'd be nice to improve this, and I'm willing
to take a look if someone prepares a patch. But I don't think making
this a GUC is the right approach - it's the simplest patch, but every
new GUC just makes the database harder to manage.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Banck | 2024-07-22 09:53:58 | Re: why there is not VACUUM FULL CONCURRENTLY? |
Previous Message | Tomas Vondra | 2024-07-22 09:26:28 | Re: WIP: parallel GiST index builds |