Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: ensure pausable portals compatible with prepared_statements_cache_size #99959

Closed
ZhouXing19 opened this issue Mar 29, 2023 · 2 comments
Closed
Labels
A-pausable-portals Issues related to multiple active portals C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@ZhouXing19
Copy link
Collaborator

ZhouXing19 commented Mar 29, 2023

In #98917 we introduced prepared_statements_cache_size, which automatically deallocate LRU prepared statements. In #99663 we (will) introduce pausable portals, which enable executing prepared stmts in an interleaving manner. We need to sort out the expected interaction of both features.

There're several options:

  1. Allowing both, meaning when deallocating LRU prepared stmts, we close the associated portals.
  2. Disallow setting prepared_statements_cache_size if there're any active portals, or if the session var multiple_active_portals_enabled has been set to true. And vice versa.

Jira issue: CRDB-26158

Epic CRDB-25183

@ZhouXing19 ZhouXing19 added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) A-pausable-portals Issues related to multiple active portals labels Mar 29, 2023
@rafiss
Copy link
Collaborator

rafiss commented Mar 29, 2023

Is a third option feasible: what if we update the LRU logic so it does not evict a prepared statement if there is still an open portal for it? And maybe instead it could try to find the next least recently used prepared statement that does not have an open portal.

If that's not feasible, my preference is option 1.

@ZhouXing19
Copy link
Collaborator Author

I ran the following test in PG and turns out that deallocating a prepared stmt won't affect associated portals' execution. This is what we're doing already, so I'm closing this issue.

send
Query {"String": "BEGIN"}
Parse {"Name": "q1", "Query": "SELECT * FROM generate_series(1,20)"}
Parse {"Name": "q2", "Query": "SELECT * FROM generate_series(1,20)"}
Bind {"DestinationPortal": "p1", "PreparedStatement": "q1"}
Bind {"DestinationPortal": "p2", "PreparedStatement": "q2"}
Execute {"Portal": "p1", "MaxRows": 1}
Execute {"Portal": "p2", "MaxRows": 1}
Query {"String": "DEALLOCATE q1"}
Execute {"Portal": "p1", "MaxRows": 1}
Execute {"Portal": "p2", "MaxRows": 1}
Sync
----

until
ReadyForQuery
ReadyForQuery
ReadyForQuery
----
{"Type":"CommandComplete","CommandTag":"BEGIN"}
{"Type":"ReadyForQuery","TxStatus":"T"}
{"Type":"ParseComplete"}
{"Type":"ParseComplete"}
{"Type":"BindComplete"}
{"Type":"BindComplete"}
{"Type":"DataRow","Values":[{"text":"1"}]}
{"Type":"PortalSuspended"}
{"Type":"DataRow","Values":[{"text":"1"}]}
{"Type":"PortalSuspended"}
{"Type":"CommandComplete","CommandTag":"DEALLOCATE"}
{"Type":"ReadyForQuery","TxStatus":"T"}
{"Type":"DataRow","Values":[{"text":"2"}]}
{"Type":"PortalSuspended"}
{"Type":"DataRow","Values":[{"text":"2"}]}
{"Type":"PortalSuspended"}
{"Type":"ReadyForQuery","TxStatus":"T"}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-pausable-portals Issues related to multiple active portals C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

No branches or pull requests

2 participants