Re: Bypassing cursors in postgres_fdw to enable parallel plans

2025-02-17 Thread KENAN YILMAZ
Hi Rafia,

Based on our previous private discussion, thanks for the update and for
clarifying the current state of the patch.
I understand that more substantial changes are on the way, so I’ll focus on
relevant test scenarios rather than performance testing at this stage.

I will proceed with expanding the scenarios, including:

Multiple postgres_fdw servers are active at the same time
Multiple connections from the same postgres_fdw are active concurrently
Multiple transactions run simultaneously on a single connection
Multiple sessions operate from a single active connection

I will submit the results of these tests to this mail thread so that they
can benefit the broader community as well. Additionally, once you publish
the updated version of the patch, I will rerun the tests with the latest
changes and share the updated results.

Best Regards,

Rafia Sabih , 17 Şub 2025 Pzt, 16:46 tarihinde
şunu yazdı:

> On Tue, 14 Jan 2025 at 18:33, Robert Haas  wrote:
>
>> On Mon, Jan 6, 2025 at 3:52 AM Rafia Sabih 
>> wrote:
>> > Now, to overcome this limitation, I have worked on this idea (suggested
>> by my colleague Bernd Helmle) of bypassing the cursors. The way it works is
>> as follows,
>> > there is a new GUC introduced postgres_fdw.use_cursor, which when unset
>> uses the mode without the cursor. Now, it uses PQsetChunkedRowsMode in
>> create_cursor when non-cursor mode is used. The size of the chunk is the
>> same as the fetch_size. Now in fetch_more_data, when non-cursor mode is
>> used, pgfdw_get_next_result is used to get the chunk in PGresult and
>> processed in the same manner as before.
>> >
>> > Now, the issue comes when there are simultaneous queries, which is the
>> case with the join queries where all the tables involved in the join are at
>> the local server. Because in that case we have multiple cursors opened at
>> the same time and without a cursor mechanism we do not have any information
>> or any other structure to know what to fetch from which query. To handle
>> that case, we have a flag only_query, which is unset as soon as we have
>> assigned the cursor_number >= 2, in postgresBeginForeignScan. Now, in
>> fetch_more data, when we find out that only_query is unset, then we fetch
>> all the data for the query and store it in a Tuplestore. These tuples are
>> then transferred to the fsstate->tuples and then processed as usual.
>> >
>> > So yes there is a performance drawback in the case of simultaneous
>> queries, however, the ability to use parallel plans is really an added
>> advantage for the users. Plus, we can keep things as before by this new GUC
>> -- use_cursor, in case we are losing more for some workloads.  So, in short
>> I feel hopeful that this could be a good idea and a good time to improve
>> postgres_fdw.
>>
>> Hi,
>>
>> I think it might have been nice to credit me in this post, since I
>> made some relevant suggestions here off-list, in particular the idea
>> of using a Tuplestore when there are multiple queries running. But I
>> don't think this patch quite implements what I suggested. Here, you
>> have a flag only_query which gets set to true at some point in time
>> and thereafter remains true for the lifetime of a session. That means,
>> I think, that all future queries will use the tuplestore even though
>> there might not be multiple queries running any more. which doesn't
>> seem like what we want. And, actually, this looks like it will be set
>> as soon as you reach the second query in the same transaction, even if
>> the two queries don't overlap. I think what you want to do is test
>> whether, at the point where we would need to issue a new query,
>> whether an existing query is already running. If not, move that
>> query's remaining results into a Tuplestore so you can issue the new
>> query.
>>
>> I'm not sure what the best way to implement that is, exactly. Perhaps
>> fsstate->conn_state needs to store some more details about the
>> connection, but that's just a guess. I don't think a global variable
>> is what you want. Not only is that session-lifetime, but it applies
>> globally to every connection to every server. You want to test
>> something that is specific to one connection to one server, so it
>> needs to be part of a data structure that is scoped that way.
>>
>> I think you'll want to figure out a good way to test this patch. I
>> don't know if we need or can reasonably have automated test cases for
>> this new functionality, but you at least want to have a good way to do
>> manual testing, so that you can show that the tuplestore is used in
>> cases where it's necessary and not otherwise. I'm not yet sure whether
>> this patch needs automated test cases or whether they can reasonably
>> be written, but you at least want to have a good procedure for manual
>> validation so that you can verify that the Tuplestore is used in all
>> the cases where it needs to be and, hopefully, no others.
>>
>> --
>> Robert Haas
>> EDB: http://www.enterprisedb.com
>>
>

Re: Bypassing cursors in postgres_fdw to enable parallel plans

2025-03-04 Thread KENAN YILMAZ
al time=0.255..343.378
rows=1666333 loops=3)
Filter: (aid > 1000)
Rows Removed by Filter: 333
bg=client backend, db=testdb, usr=postgres, client=192.168.1.68 , qryId=0 ,
txId=0, app=postgres_fdw, line=25 LOG:  statement: START TRANSACTION
ISOLATION LEVEL REPEATABLE READ
..
STATEMENT:  SELECT aid FROM public.pgbench_accounts WHERE ((aid > 1000))
LIMIT 1::bigint
bg=client backend, db=testdb, usr=postgres, client=192.168.1.68 ,
qryId=5994602644362067232 , txId=0, app=postgres_fdw, line=29 LOG:  execute
: SELECT aid FROM public.pgbench_accounts WHERE ((aid > 1000))
LIMIT 1::bigint
bg=client backend, db=testdb, usr=postgres, client=192.168.1.68 , qryId=0 ,
txId=0, app=postgres_fdw, line=30 LOG:  statement: COMMIT TRANSACTION
bg=client backend, db=testdb, usr=postgres, client=192.168.1.68 ,
qryId=-2835399305386018931 , txId=0, app=postgres_fdw, line=31 LOG:
 duration: 7.983 ms  plan:
Query Text: SELECT aid FROM public.pgbench_accounts WHERE ((aid >
1000)) LIMIT 1::bigint
Limit  (cost=0.00..0.02 rows=1 width=4) (actual time=0.836..7.974
rows=1 loops=1)
  ->  Gather  (cost=0.00..108009.67 rows=4999007 width=4) (actual
time=0.834..7.972 rows=1 loops=1)
Workers Planned: 2
Workers Launched: 2
->  Parallel Seq Scan on pgbench_accounts
 (cost=0.00..108009.67 rows=2082920 width=4) (actual time=0.270..0.271
rows=1 loops=3)
      Filter: (aid > 1000)
  Rows Removed by Filter: 333



if you would like me to conduct more complex tests, feel free to let me
know.
Best regards,
Kenan YILMAZ



KENAN YILMAZ , 17 Şub 2025 Pzt, 17:09
tarihinde şunu yazdı:

> Hi Rafia,
>
> Based on our previous private discussion, thanks for the update and for
> clarifying the current state of the patch.
> I understand that more substantial changes are on the way, so I’ll focus
> on relevant test scenarios rather than performance testing at this stage.
>
> I will proceed with expanding the scenarios, including:
>
> Multiple postgres_fdw servers are active at the same time
> Multiple connections from the same postgres_fdw are active concurrently
> Multiple transactions run simultaneously on a single connection
> Multiple sessions operate from a single active connection
>
> I will submit the results of these tests to this mail thread so that they
> can benefit the broader community as well. Additionally, once you publish
> the updated version of the patch, I will rerun the tests with the latest
> changes and share the updated results.
>
> Best Regards,
>
> Rafia Sabih , 17 Şub 2025 Pzt, 16:46 tarihinde
> şunu yazdı:
>
>> On Tue, 14 Jan 2025 at 18:33, Robert Haas  wrote:
>>
>>> On Mon, Jan 6, 2025 at 3:52 AM Rafia Sabih 
>>> wrote:
>>> > Now, to overcome this limitation, I have worked on this idea
>>> (suggested by my colleague Bernd Helmle) of bypassing the cursors. The way
>>> it works is as follows,
>>> > there is a new GUC introduced postgres_fdw.use_cursor, which when
>>> unset uses the mode without the cursor. Now, it uses PQsetChunkedRowsMode
>>> in create_cursor when non-cursor mode is used. The size of the chunk is the
>>> same as the fetch_size. Now in fetch_more_data, when non-cursor mode is
>>> used, pgfdw_get_next_result is used to get the chunk in PGresult and
>>> processed in the same manner as before.
>>> >
>>> > Now, the issue comes when there are simultaneous queries, which is the
>>> case with the join queries where all the tables involved in the join are at
>>> the local server. Because in that case we have multiple cursors opened at
>>> the same time and without a cursor mechanism we do not have any information
>>> or any other structure to know what to fetch from which query. To handle
>>> that case, we have a flag only_query, which is unset as soon as we have
>>> assigned the cursor_number >= 2, in postgresBeginForeignScan. Now, in
>>> fetch_more data, when we find out that only_query is unset, then we fetch
>>> all the data for the query and store it in a Tuplestore. These tuples are
>>> then transferred to the fsstate->tuples and then processed as usual.
>>> >
>>> > So yes there is a performance drawback in the case of simultaneous
>>> queries, however, the ability to use parallel plans is really an added
>>> advantage for the users. Plus, we can keep things as before by this new GUC
>>> -- use_cursor, in case we are losing more for some workloads.  So, in short
>>> I feel hopeful that this could be a good idea and a good time to improve
>>> postgres_fdw.
>>>
>>> Hi