Skip adding row-marks for non target tables when result relation is foreign table.
Hi PostgreSQL Community,
I would like to bring to your attention an observation regarding the
planner's behavior for foreign table update/delete operations. It appears
that the planner adds rowmarks (ROW_MARK_COPY) for non-target tables, which
I believe is unnecessary when using the postgres-fdw. This is because
postgres-fdw performs early locking on tuples belonging to the target
foreign table by utilizing the SELECT FOR UPDATE clause.
In an attempt to address this, I tried implementing late locking. However,
this approach still doesn't work as intended because the API assumes that
foreign table rows can be re-fetched using TID (ctid). This assumption is
invalid for partitioned tables on the foreign server. Additionally, the
commit afb9249d06f47d7a6d4a89fea0c3625fe43c5a5d, which introduced late
locking for foreign tables, mentions that the benefits of late locking
against a remote server are unclear, as the extra round trips required are
likely to outweigh any potential concurrency improvements.
To address this issue, I have taken the initiative to create a patch that
prevents the addition of rowmarks for non-target tables when the target
table is using early locking. I would greatly appreciate it if you could
review the patch and provide any feedback or insights I may be overlooking.
Example query plan with my change: (foreign scan doesn't fetch whole row
for bar).
postgres=# \d+ bar
Foreign table "public.bar"
Column | Type | Collation | Nullable | Default | FDW options |
Storage | Stats target | Description
--------+---------+-----------+----------+---------+--------------------+---------+--------------+-------------
b1 | integer | | | | (column_name 'b1') |
plain | |
b2 | integer | | | | (column_name 'b2') |
plain | |
Server: postgres_1
FDW options: (schema_name 'public', table_name 'bar')
router=# \d+ foo
Foreign table "public.foo"
Column | Type | Collation | Nullable | Default | FDW options |
Storage | Stats target | Description
--------+---------+-----------+----------+---------+--------------------+---------+--------------+-------------
f1 | integer | | | | (column_name 'f1') |
plain | |
f2 | integer | | | | (column_name 'f2') |
plain | |
Server: postgres_2
FDW options: (schema_name 'public', table_name 'foo')
postgres=# explain verbose update foo set f1 = b1 from bar where f2=b2;
QUERY PLAN
----------------------------------------------------------------------------------------
Update on public.foo (cost=200.00..48713.72 rows=0 width=0)
Remote SQL: UPDATE public.foo SET f1 = $2 WHERE ctid = $1
-> Nested Loop (cost=200.00..48713.72 rows=15885 width=42)
Output: bar.b1, foo.ctid, foo.*
Join Filter: (foo.f2 = bar.b2)
-> Foreign Scan on public.bar (cost=100.00..673.20 rows=2560
width=8)
Output: bar.b1, bar.b2
Remote SQL: SELECT b1, b2 FROM public.bar
-> Materialize (cost=100.00..389.23 rows=1241 width=42)
Output: foo.ctid, foo.*, foo.f2
-> Foreign Scan on public.foo (cost=100.00..383.02
rows=1241 width=42)
Output: foo.ctid, foo.*, foo.f2
Remote SQL: SELECT f1, f2, ctid FROM public.foo FOR
UPDATE
(13 rows)
Thank you for your time and consideration.
Regards
Saikiran Avula
SDE, Amazon Web Services.
Attachments:
v1-0001-Skip-adding-row-marks-for-not-target-relations-wh.patchapplication/octet-stream; name=v1-0001-Skip-adding-row-marks-for-not-target-relations-wh.patchDownload+86-42
On Mon, 2024-05-06 at 23:10 +0100, SAIKIRAN AVULA wrote:
I would like to bring to your attention an observation regarding the
planner's behavior for foreign table update/delete operations. It
appears that the planner adds rowmarks (ROW_MARK_COPY) for non-target
tables, which I believe is unnecessary when using the postgres-fdw.
This is because postgres-fdw performs early locking on tuples
belonging to the target foreign table by utilizing the SELECT FOR
UPDATE clause.
I agree with your reasoning here. If it reads the row with SELECT FOR
UPDATE, what's the purpose of row marks?
The cost of ROW_MARK_COPY is that it brings the whole tuple along
rather than a reference. I assume you are concerned about wide tables
involved in the join or is there another concern?
In an attempt to address this, I tried implementing late locking.
For others in the thread, see:
https://www.postgresql.org/docs/current/fdw-row-locking.html
However, this approach still doesn't work as intended because the API
assumes that foreign table rows can be re-fetched using TID (ctid).
This assumption is invalid for partitioned tables on the foreign
server.
It looks like it's a "Datum rowid", but is currently only allowed to be
a ctid, which can't identify the partition. I wonder how much work it
would be to fix this?
Additionally, the commit afb9249d06f47d7a6d4a89fea0c3625fe43c5a5d,
which introduced late locking for foreign tables, mentions that the
benefits of late locking against a remote server are unclear, as the
extra round trips required are likely to outweigh any potential
concurrency improvements.
The extra round trip only happens when EPQ finds a newer version of the
tuple, which should be the exceptional case. I'm not sure how this
balances out, but to me late locking still seems preferable. Early
locking is a huge performance hit in some cases (locking many more rows
than necessary).
Early locking is also a violation of the documentation here:
"When a locking clause appears at the top level of a SELECT query, the
rows that are locked are exactly those that are returned by the query;
in the case of a join query, the rows locked are those that contribute
to returned join rows."
https://www.postgresql.org/docs/current/sql-select.html#SQL-FOR-UPDATE-SHARE
To address this issue, I have taken the initiative to create a patch
that prevents the addition of rowmarks for non-target tables when the
target table is using early locking. I would greatly appreciate it if
you could review the patch and provide any feedback or insights I may
be overlooking.
A couple comments:
* You're using GetFdwRoutineForRelation() with makecopy=false, and then
closing the relation. If the rd_fdwroutine was already set previously,
then the returned pointer will point into the relcache, which may be
invalid after closing the relation. I'd probably pass makecopy=true and
then free it. (Weirdly if you pass makecopy=false, you may or may not
get a copy, so there's no way to know whether to free it or not.)
* Core postgres doesn't really choose early locking. If
RefetchForeignRow is not defined, then late locking is impossible, so
it assumes that early locking is happening. That assumption is true for
postgres_fdw, but might not be for other FDWs. What if an FDW doesn't
do early locking and also doesn't define RefetchForeignRow?
Regards,
Jeff Davis
On Wed, May 22, 2024 at 10:13 AM Jeff Davis <pgsql@j-davis.com> wrote:
On Mon, 2024-05-06 at 23:10 +0100, SAIKIRAN AVULA wrote:
Additionally, the commit afb9249d06f47d7a6d4a89fea0c3625fe43c5a5d,
which introduced late locking for foreign tables, mentions that the
benefits of late locking against a remote server are unclear, as the
extra round trips required are likely to outweigh any potential
concurrency improvements.The extra round trip only happens when EPQ finds a newer version of the
tuple, which should be the exceptional case. I'm not sure how this
balances out, but to me late locking still seems preferable. Early
locking is a huge performance hit in some cases (locking many more rows
than necessary).
I might be missing something, but I think the extra round trip happens
for each foreign row locked using the RefetchForeignRow() API in
ExecLockRows(), so I think the overhead is caused in the normal case.
Early locking is also a violation of the documentation here:
"When a locking clause appears at the top level of a SELECT query, the
rows that are locked are exactly those that are returned by the query;
in the case of a join query, the rows locked are those that contribute
to returned join rows."
Yeah, but I think this holds true for SELECT queries postgres_fdw
sends to the remote side. :)
Best regards,
Etsuro Fujita
On Fri, 2024-08-09 at 17:35 +0900, Etsuro Fujita wrote:
I might be missing something, but I think the extra round trip
happens
for each foreign row locked using the RefetchForeignRow() API in
ExecLockRows(), so I think the overhead is caused in the normal case.
Is there any sample code that implements late locking for a FDW? I'm
not quite clear on how it's supposed to work.
Regards,
Jeff Davis
On Thu, Aug 15, 2024 at 9:56 AM Jeff Davis <pgsql@j-davis.com> wrote:
Is there any sample code that implements late locking for a FDW? I'm
not quite clear on how it's supposed to work.
See the patch in [1]/messages/by-id/16016.1431455074@sss.pgh.pa.us. It would not apply to HEAD anymore, though.
Best regards,
Etsuro Fujita