| From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
|---|---|
| To: | Corey Huinker <corey(dot)huinker(at)gmail(dot)com> |
| Cc: | solai v <solai(dot)cdac(at)gmail(dot)com>, Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, nurlan(dot)tulemisov(at)gmail(dot)com |
| Subject: | Re: postgres_fdw: Emit message when batch_size is reduced |
| Date: | 2026-10-15 13:55:42 |
| Message-ID: | CA+TgmoZJihQ6RSf-JnLQ+HYDHA1i7Yxn7JxexOJbV7RkdCRkLw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Oct 14, 2026 at 12:22 PM Corey Huinker <corey(dot)huinker(at)gmail(dot)com> wrote:
> My interest in the DEBUG messages was giving a curious user an answer to the question "If I'm not getting the batch_size I asked for, what batch_size am I actually getting?", but that should infer-able from logs looking for "FETCH <number> FROM cursor_name", albeit with some additional effort to line up which FETCH commands correspond to which FDW queries, and cross referencing that against the fetch_size that they've set, so it may be that a curious user already has the instrumentation they need.
Unless I'm confused, that's fetch_size, and this patch is about
batch_size. So, you're doing a bulk-insert to a foreign table, and the
question is about how many separate INSERT statements we're going to
have to issue in order to handle all the rows.
If the concern is about the difficulty of correlating local and remote
log lines, we could give postgres_fdw an option that causes the
commands it ships to the remote side to be logged locally at a level
of the user's choosing, and that could also involve logging fetch
sizes. I am not sure whether that would be valuable enough to justify
its carrying costs, but I can see the argument for it. You could watch
it issue transaction control commands and stuff, too, which could be
helpful for debugging. But all of that is a separate project from
this.
> I agree that's the best course of action once a user knows that they have a batch_size problem, but I don't think the user affected by this situation knows that specific detail just yet. They probably know that their FDW queries have slow and/or stair-step stuttering performance (i.e. why does a result set of 1372 rows take so much longer than one with 1371? What's special about that number?), and they might also know that they made some configuration changes to cut down on network chatter for very large result sets, but the root cause still eludes them. When in the discovery phase of problem, it is reasonable to crank up the logging detail and see what's in there. But that same increase in logging detail will surface the FETCH x FROM y queries as well
Do you think this is a real effect? I'm guessing that you're supposing
that the fetch size is 1372 here, so that with 1371 rows we fetch all
the rows in one go and see that there are no more and with 1372 rows
we have to issue a second fetch. But it's probably not the second
fetch that is the dominant cost here. In my mind, when you're sad
about the performance in these kinds of scenarios, you're more likely
to be sad that the plan had to use a cursor at all, because that may
for example have precluded the use of parallel query. Whether you have
to do one fetch or two probably matters relatively little by
comparison. That's not to say that the extra fetch is free. It isn't,
and you might be able to see that in some scenarios, like when the
cost of generating each row is very low. But I suspect it's not the
biggest of deals, either.
In the INSERT scenario, which is what I think the patch is targeting,
the issue would be one INSERT with 1372 rows vs. two inserts, one with
1371 rows and a second with 1 row. I don't know how much slower the
second one is, but I bet it isn't much slower. The extra statement has
some overhead, but so does inserting 1372 rows. You could test a case
with a single-row, single-column table with no indexes, so that the
extra network round trip is the biggest chunk of the cost possible,
but that's a pretty artificial scenario.
Honestly, there's room in my mind to question whether very large batch
sizes are even a good idea. Up near 64k parameters, the generated
INSERT INTO ... VALUES (...), (...), (...) ... statement will be
enormous. It has not been my experience that PostgreSQL users
routinely experience regret that they're only sending half a megabyte
of SQL in one shot; indeed, most of the regrets I've seen in this area
have come from query strings that are longer than they really ought to
be.
One more new discovery: apparently, you can already get the post-clamp
batch size by just using EXPLAIN (ANALYZE, VERBOSE) -- or if that's
too tricky in an IRL situation, you could set up auto_explain to do
the same thing for you automatically and dump the results to the logs.
In some ways, that's actually better than the DEBUG message this patch
proposes to add, because it takes into account all the ways that
postgresGetForeignModifyBatchSize can decide to adjust the numbers,
instead of only the last case. Regrettably, it appears that without
ANALYZE, you see the unclamped value, because
postgresGetForeignModifyBatchSize doesn't have an fmstate and skips
clamping the batch size for lack of knowledge. Potentially, that could
be fixed by factoring the p_nums calculation in create_foreign_modify
into a helper function that postgresGetForeignModifyBatchSize uses, or
by a larger refactoring that creates a partial fmstate even in pure
EXPLAIN mode, and just skips connection to the remote side. None of
that seems essential, but it could be done. Also regrettably, EXPLAIN
can only help with INSERT, not COPY FROM, so theoretically a DEBUG
message to cover that case could still have value, but it's aiming at
a very narrow target: the batch size is gigantic, you actually want to
be sending huge SQL statements, you're using COPY FROM, and you don't
have the ability to easily inspect the remote logs.
> I see this as an argument for removing the DEBUG message but keeping the WARNING in option.c - as it immediately tells the well user "the value you specified cannot possibly achieve the thing you think it will do", potentially eliminating a fruitless or at least confusing testing/debug cycle, and possibly making them aware that there is an upper bound to fetch_size, thus initiating the voyage of discovery as outlined by Robert above, as well as providing a strong clue as to why the logs show a fetch_size of 652 despite the number 652 not showing up in the config files anywhere.
Maybe, but keep in mind that (1) this is still about batch_size, not
fetch_size and (2) the two warnings aren't the same. The
postgres_fdw_validator WARNING is only going to fire if you set the
batch size above 64k, whereas the DEBUG message fires if the batch
size is more than 64k divided by the number of columns per row. They
only have the same threshold for a 1-column table, which is probably
not what you've got. Said differently, it's much more likely that you
have a ten-column table and a batch size of 7000 than a one-column
table and a batch size of 70000.
--
Robert Haas
Databricks
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Corey Huinker | 2026-10-15 14:08:38 | Re: postgres_fdw: Emit message when batch_size is reduced |
| Previous Message | Heikki Linnakangas | 2026-10-15 13:52:30 | Re: Avoid repeated scans of the output buffer in oidvectortypes() |