Skip to content

Add first-class coroutine PyCoreAsyncCursor to mssql-py-core - #208

Closed
Saurabh Singh (saurabh500) wants to merge 1 commit into
dev/saurabh/rewire-py-core-sync-async-cursor-l6from
dev/saurabh/l7-async-cursor
Closed

Add first-class coroutine PyCoreAsyncCursor to mssql-py-core#208
Saurabh Singh (saurabh500) wants to merge 1 commit into
dev/saurabh/rewire-py-core-sync-async-cursor-l6from
dev/saurabh/l7-async-cursor

Conversation

@saurabh500

@saurabh500 Saurabh Singh (saurabh500) commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds PyCoreAsyncCursor, a genuine asyncio coroutine cursor for mssql-py-core, built on pyo3-async-runtimes future_into_py. This is a correctness/architecture change (a real non-blocking coroutine cursor), not a performance win.

execute / fetchone / fetchmany / fetchall / close each return a Python awaitable. The TDS I/O is driven on the connection's own tokio runtime and the coroutine .awaits the resulting join handle — there is no block_on on the coroutine path. The !Send client is checked out of the shared cell and the guard is dropped before .await, so the Python event loop stays free while a fetch is in flight.

Strictly additive

  • The default PyCoreCursor (conn.cursor()) and the opt-in sync PyCoreSyncCursor (conn.sync_cursor()) are byte-identical to their prior form — cursor.rs and sync_cursor.rs are unchanged.
  • New: conn.async_cursor()PyCoreAsyncCursor.
  • The async path uses a self-contained with_async_client checkout helper in pyclient.rs; the shared client cell stays std::sync::Mutex (no retype needed).
  • Doc-label scrub: fixes the stale "async cursor" comments now that a real coroutine cursor exists.

Changes (py-core only)

  • src/async_cursor.rs (new) — PyCoreAsyncCursor coroutine cursor.
  • src/pyclient.rswith_async_client async checkout helper + honest module doc.
  • src/connection.rsConnection.async_cursor().
  • src/lib.rs — register PyCoreAsyncCursor.
  • Cargo.toml — add pyo3-async-runtimes = { version = "0.29", features = ["tokio-runtime"] }.
  • Tests: mock-backed coroutine tests + a CI WAITFOR-based non-blocking proof.

Non-blocking proof

  • Static: zero block_on on the async_cursor.rs coroutine path (I/O is spawned on the connection runtime; the coroutine awaits the join handle).
  • Local (mock): test_async_cursor_yields_to_event_loop — a concurrent ticker advances while a fetch is awaited (mock answers in µs, so this asserts the weaker "yields at least once").
  • CI (live server): test_async_cursor_integration.py — a concurrent 10ms ticker racks up ticks during a server-side WAITFOR DELAY '00:00:02' inside the coroutine's await.

Notes

  • Perf-sanity (flag, not block): over the mock (µs-scale I/O) the coroutine path is ~3.7x the default block_on path — this is pure coroutine-plumbing overhead on a zero-latency backend, not representative of real network I/O where the overhead is negligible and concurrency dominates.
  • mssql-py-core Cargo.lock is gitignored; the new dep resolves under the --frozen py-core clippy/test leg locally.

Part of the sans-I/O native stack - see cover PR #189 for the stack-level summary and tracked debt.

@saurabh500
Saurabh Singh (saurabh500) marked this pull request as ready for review August 10, 2026 06:38
@saurabh500
Saurabh Singh (saurabh500) requested a review from a team as a code owner August 10, 2026 06:38
Copilot AI balanced review requested due to automatic review settings August 10, 2026 06:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a coroutine-based Python cursor that performs TDS operations without blocking the asyncio event loop.

Changes:

  • Adds and registers PyCoreAsyncCursor and Connection.async_cursor().
  • Adds asynchronous client checkout and the required runtime dependency.
  • Adds mock and live-server coroutine tests and updates cursor terminology.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
mssql-py-core/Cargo.toml Adds the PyO3 async runtime dependency.
mssql-py-core/src/async_cursor.rs Implements the coroutine cursor.
mssql-py-core/src/connection.rs Exposes async_cursor().
mssql-py-core/src/lib.rs Registers the new Python class.
mssql-py-core/src/pyclient.rs Adds asynchronous client checkout.
mssql-py-core/tests/rs-only-tests/test_async_cursor_mock.py Tests coroutine cursor behavior.
mssql-py-core/tests/rs-only-tests/test_sync_async_cursor_mock.py Corrects default cursor terminology.
mssql-py-core/tests/test_async_cursor_integration.py Verifies live-server non-blocking behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

&self,
py: Python<'py>,
query: String,
params: Option<Vec<Py<PyAny>>>,
Comment on lines +268 to +270
let taken = {
let mut guard = cell.lock().map_err(|_| poisoned())?;
std::mem::replace(&mut *guard, PyClient::Transitioning)
@saurabh500
Saurabh Singh (saurabh500) marked this pull request as draft August 10, 2026 06:49
@saurabh500
Saurabh Singh (saurabh500) marked this pull request as ready for review August 10, 2026 07:40
@saurabh500
Saurabh Singh (saurabh500) marked this pull request as draft August 10, 2026 13:28
Add PyCoreAsyncCursor, a genuine asyncio coroutine cursor built on
pyo3-async-runtimes future_into_py. execute/fetchone/fetchmany/fetchall/
close return awaitables that drive the TDS I/O on the connection's tokio
runtime with no block_on on the coroutine path. The !Send client is checked
out of the shared cell and the guard dropped before .await, so the Python
event loop stays free during fetches.

Strictly additive: the default PyCoreCursor and opt-in PyCoreSyncCursor are
byte-identical to their prior form. Also scrubs stale 'async cursor' doc
labels now that a real coroutine cursor exists.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e2c378f8-3ba1-4b48-9ebe-5a4ea2bd2761
@saurabh500

Copy link
Copy Markdown
Contributor Author

Closing, consistent with the rest of the sans-I/O stack (#189#203, closed 2026-08-13).

This PR is an L5–L7 consumer of the reactor-free sync core, so it has no base once that core is not landing. The stack was closed because the row-decode performance work that motivated it was re-scoped around #247, where benchmarked spikes showed the dominant win came from a far smaller change than inverting the core to a sync step() driver.

That argument has since been settled by measurement rather than projection. PR #264 has now merged (7a9c0b93), converting TdsPacketReader to RPITIT and removing per-read boxing — −61.6% decode time, independently corroborated at −61.4% on a separate harness built for another purpose. The win this stack was reaching for is realised, on ~12 draft PRs and ~8,400 lines less surface area.

Two further results from that work are worth recording here, because both cut against restructuring this path speculatively:

This is cleanup, not a rejection of the analysis. The branch is deliberately not deleted, so the work remains recoverable if the direction is revisited.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants