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
