Обсуждение: Limit memory usage by postgres_fdw batches

Hi.

We had some real cases when client set rather big batch_size on server 
level, but for some foreign table, containing large documents, it was 
inadequate and lead to OOM killer intervention. You can argue that 
batch_size can be set on foreign table level, but it can still be not 
flexible enough, when tuple size varies. I suppose this case is also 
takes place for fetch_size. Issue here is that we can't somehow limit 
size of data (versus number of rows) while fetching from cursor. But we 
can use tuple store to preserve fetched results, so that they spill out 
to the disk.

I'm attaching two patches which try to fix issues with possible huge 
memory usage by postgres_fdw batches.
With fetched tuples we still can't use only tuplestore, as ctids are not 
preserved, and so have to store them separately.

The reproducer for insert is simple.

create extension postgres_fdw ;
create server loopback foreign data wrapper postgres_fdw options (dbname 
'postgres', port '5432', batch_size '100', fetch_size '100');
create table base_table(i int, s bytea);
create foreign table foreign_table (i int, s bytea) server loopback 
options(table_name 'base_table');
create user mapping for public server loopback ;

insert into foreign_table select i, 
pg_read_binary_file('/some/big/file') from generate_series(1,1000) i;

will easily grow backend RSS to several gigabytes.
The first patch fixes this problem.

The second patch alleviates the second issue - SELECT * queries also can 
grow backend memory to several GBs. Still memory usage can peak (on my 
toy examples) up to 3-4 GB, but at least it seams 1-2 GB less than 
non-patched version.

-- 
Best regards,
Alexander Pyhalov,
Postgres Professional

Hi Alexander,

On 12/26/25 11:54, Alexander Pyhalov wrote:
> Hi.
> 
> We had some real cases when client set rather big batch_size on server
> level, but for some foreign table, containing large documents, it was
> inadequate and lead to OOM killer intervention. You can argue that
> batch_size can be set on foreign table level, but it can still be not
> flexible enough, when tuple size varies.

I agree this can be an issue with large tuples. It'd be good to consider
the size, not just the number of tuples.

I think you modified the right places, but I think there are two or
three issues we should improve:

1) It calls estimate_batch_length() for every ExecInsert() call, i.e.
for every tuples. And it walks all tuples up to that point, which makes
it O(N^2). I haven't measured how significant it is, but AFAICS we could
track the current size of the batch fairly easily, and use that.

2) work_mem is in kilobytes, while batch_len is in bytes, so the
comparison (batch_len > work_mem) is not quite right. I'll probably fire
every time, preventing any batching.

3) Isn't this consider the size of the new tuple in batch_len? Imagine
the tuples are 99% of the work_mem limit. We add the first one. When
adding the next one we check the current batch is below work_mem, and so
we proceed to add the second tuple. Now the batch is 1.98% of the limit.

I think it should work like this:

1) batch_len + tup_len < work_mem => add tuple to batch
2) tup_len < work_mem => flush batch, add tuple to batch
3) tup_len => work_mem => flush batch, insert tuple directly

What bothers me a little bit is that this is per relation. AFAICS when
inserting into a partitioned table with multiple foreign partitions,
each partition will have a separate limit. I wonder if we could do
better and have some sort of "global" limit for the whole insert.

But that's not the fault of this patch, of course.

> I suppose this case is also
> takes place for fetch_size. Issue here is that we can't somehow limit
> size of data (versus number of rows) while fetching from cursor. But we
> can use tuple store to preserve fetched results, so that they spill out
> to the disk.
> 

Perhaps. Seems like a separate issue. I haven't looked very closely, but
do we want to use the tuplestore always, or just when the tuples get too
large? It might even be on batch-by-batch, I guess. With fetch_size=1
it's hardly useful, right? Is the tuplestore management measurable?

> I'm attaching two patches which try to fix issues with possible huge
> memory usage by postgres_fdw batches.
> With fetched tuples we still can't use only tuplestore, as ctids are not
> preserved, and so have to store them separately.
> 
> The reproducer for insert is simple.
> 
> create extension postgres_fdw ;
> create server loopback foreign data wrapper postgres_fdw options (dbname
> 'postgres', port '5432', batch_size '100', fetch_size '100');
> create table base_table(i int, s bytea);
> create foreign table foreign_table (i int, s bytea) server loopback
> options(table_name 'base_table');
> create user mapping for public server loopback ;
> 
> insert into foreign_table select i, pg_read_binary_file('/some/big/
> file') from generate_series(1,1000) i;
> 
> will easily grow backend RSS to several gigabytes.
> The first patch fixes this problem.
> 
> The second patch alleviates the second issue - SELECT * queries also can
> grow backend memory to several GBs. Still memory usage can peak (on my
> toy examples) up to 3-4 GB, but at least it seams 1-2 GB less than non-
> patched version.
> 

How large are the tuples? How much higher was this RSS than the
theoretical minimum?


regards

-- 
Tomas Vondra

Tomas Vondra писал(а) 2025-12-27 16:18:
> Hi Alexander,
> 
> On 12/26/25 11:54, Alexander Pyhalov wrote:
>> Hi.
>> 
>> We had some real cases when client set rather big batch_size on server
>> level, but for some foreign table, containing large documents, it was
>> inadequate and lead to OOM killer intervention. You can argue that
>> batch_size can be set on foreign table level, but it can still be not
>> flexible enough, when tuple size varies.
> 
> I agree this can be an issue with large tuples. It'd be good to 
> consider
> the size, not just the number of tuples.
> 
> I think you modified the right places, but I think there are two or
> three issues we should improve:
> 
> 1) It calls estimate_batch_length() for every ExecInsert() call, i.e.
> for every tuples. And it walks all tuples up to that point, which makes
> it O(N^2). I haven't measured how significant it is, but AFAICS we 
> could
> track the current size of the batch fairly easily, and use that.
> 
> 2) work_mem is in kilobytes, while batch_len is in bytes, so the
> comparison (batch_len > work_mem) is not quite right. I'll probably 
> fire
> every time, preventing any batching.
> 
> 3) Isn't this consider the size of the new tuple in batch_len? Imagine
> the tuples are 99% of the work_mem limit. We add the first one. When
> adding the next one we check the current batch is below work_mem, and 
> so
> we proceed to add the second tuple. Now the batch is 1.98% of the 
> limit.
> 
> I think it should work like this:
> 
> 1) batch_len + tup_len < work_mem => add tuple to batch
> 2) tup_len < work_mem => flush batch, add tuple to batch
> 3) tup_len => work_mem => flush batch, insert tuple directly
> 
> What bothers me a little bit is that this is per relation. AFAICS when
> inserting into a partitioned table with multiple foreign partitions,
> each partition will have a separate limit. I wonder if we could do
> better and have some sort of "global" limit for the whole insert.
> 
> But that's not the fault of this patch, of course.

Hi. Thank you for feedback.
I've fixed the first two issues in the first patch.
I've tried to implement switching from batch to usual foreign insert 
(it's in the second patch). However, I don't like it. It changes the 
resultRelInfo->ri_NumSlots interface. Now it can be 0 when BatchInsert 
was followed by foreign insert, and ExecPendingInserts() should handle 
this. Also we can't longer keep fact that BatchInsert() was performed 
private to ExecInsert(), as we have to determine if 
estate->es_insert_pending_result_relations and 
estate->es_insert_pending_modifytables lists were modified between 
ExecInsert() calls. In theory we could look at 
list_member_ptr(estate->es_insert_pending_result_relations, 
resultRelInfo), but I don't know if iterating over this list per each 
tuple is a good idea.

> 
>> I suppose this case is also
>> takes place for fetch_size. Issue here is that we can't somehow limit
>> size of data (versus number of rows) while fetching from cursor. But 
>> we
>> can use tuple store to preserve fetched results, so that they spill 
>> out
>> to the disk.
>> 
> 
> Perhaps. Seems like a separate issue. I haven't looked very closely, 
> but
> do we want to use the tuplestore always, or just when the tuples get 
> too
> large? It might even be on batch-by-batch, I guess. With fetch_size=1
> it's hardly useful, right? Is the tuplestore management measurable?

I haven't noticed significant impact on simple pgbench tests. I will 
check different cases with
fetch_size 1  - for now I don't understand if we could trigger OOM for 
example with async foreign
scan of partitioned table with many foreign partitions - now we would 
have to store each tuple
we get in the memory, perhaps, saving it to the store until it's needed 
by Append is not so bad idea.

> 
>> I'm attaching two patches which try to fix issues with possible huge
>> memory usage by postgres_fdw batches.
>> With fetched tuples we still can't use only tuplestore, as ctids are 
>> not
>> preserved, and so have to store them separately.
>> 
>> The reproducer for insert is simple.
>> 
>> create extension postgres_fdw ;
>> create server loopback foreign data wrapper postgres_fdw options 
>> (dbname
>> 'postgres', port '5432', batch_size '100', fetch_size '100');
>> create table base_table(i int, s bytea);
>> create foreign table foreign_table (i int, s bytea) server loopback
>> options(table_name 'base_table');
>> create user mapping for public server loopback ;
>> 
>> insert into foreign_table select i, pg_read_binary_file('/some/big/
>> file') from generate_series(1,1000) i;
>> 
>> will easily grow backend RSS to several gigabytes.
>> The first patch fixes this problem.
>> 
>> The second patch alleviates the second issue - SELECT * queries also 
>> can
>> grow backend memory to several GBs. Still memory usage can peak (on my
>> toy examples) up to 3-4 GB, but at least it seams 1-2 GB less than 
>> non-
>> patched version.
>> 
> 
> How large are the tuples? How much higher was this RSS than the
> theoretical minimum?
> 

Sorry, have lost the exact reproducer. Memory consumption of course 
depends on tuple size and fetch size.

Remade some simple tests.


select * from pg_foreign_server ;
   oid  | srvname  | srvowner | srvfdw | srvtype | srvversion | srvacl |  
                       srvoptions

-------+----------+----------+--------+---------+------------+--------+-----------------------------------------------------------
  16403 | loopback |       10 |  16392 |         |            |        | 
{dbname=postgres,port=5432,batch_size=100,fetch_size=100}
(1 row)

select count(*) from foreign_table ;
  count
-------
   1100
(1 row)

  \d+ base_table
                                         Table "public.base_table"
  Column |  Type   | Collation | Nullable | Default | Storage  | 
Compression | Stats target | Description
--------+---------+-----------+----------+---------+----------+-------------+--------------+-------------
  i      | integer |           |          |         | plain    |          
    |              |
  s      | bytea   |           |          |         | extended |          
    |              |
Access method: heap

select pg_size_pretty(pg_total_relation_size('base_table')) ;
  pg_size_pretty
----------------
  5495 MB
(1 row)

postgres=# select 
pg_size_pretty(pg_total_relation_size('base_table')/1100) ;
  pg_size_pretty
----------------
  5116 kB
(1 row)

Backend, executing

"select  i, length(s) from foreign_table "

query takes up to 1.4 GB RSS on master and up to 1 GB on patched 
version.

P.S. Going to be on vacation for the first half of January.
-- 
Best regards,
Alexander Pyhalov,
Postgres Professional