Re: Make reorder buffer max_changes_in_memory adjustable?

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

In response to

Browse pgsql-hackers by date

  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