HyukjinKwon commented on code in PR #108:
URL: 
https://github.com/apache/spark-connect-rust/pull/108#discussion_r4415458574


##########
dev/design/arrow-handoff-rss.md:
##########
@@ -0,0 +1,71 @@
+# Arrow handoff repeated-call RSS control
+
+This is a local diagnostic for PR #108, not a performance benchmark or proof of
+leak freedom. Both the mock gRPC server and the native client run in one Python
+process, so the measurement includes their allocations and allocator retention.
+
+On 2026-10-29, the same 28-call probe was run in fresh processes on master
+`d884e34` (IPC handoff) and the review revision of #108 based on `fe79d54`
+(C Stream handoff; production changes since that head are comments only).
+Both extensions were rebuilt with the same release Cargo configuration and Rust
+1.99.1, then loaded using the same Python 3.12 / PyArrow 26 environment on 
macOS
+arm64. Master ran first. These are single runs on a shared desktop.
+
+Each call receives 128 one-MiB numeric batches from the same mock service,
+validates the 16,777,216-row table's endpoints, deletes the table, forces 
Python
+GC, and samples process RSS. The first four calls are warmup. All calls passed
+validation.
+
+| Handoff | First measured RSS (MiB) | Last measured RSS (MiB) | Measured 
minimum–maximum (MiB) |
+| --- | ---: | ---: | ---: |
+| Master IPC | 1129.23 | 3976.06 | 1129.23–3976.06 |
+| PR C Stream | 418.81 | 569.47 | 418.81–569.47 |
+
+Growth is not unique to the C Stream change; the master control grew much more
+in this run. This does not identify the source of retention, isolate client
+allocations, or establish a general memory advantage. The earlier C Stream run
+reported 416.27 → 481.66 MiB; variation between runs remains unexplained. A 
claim
+of flat post-GC RSS is not supported.
+
+To reproduce, run the following from the PR checkout with its tests available,
+setting `PYTHONPATH` to the Python package containing the extension under test.

Review Comment:
   **Nit (P3):** Followed literally, these two instructions don't load the 
extension under test. The harness does `from pyspark.sql import SparkSession`, 
so `PYTHONPATH` has to be the directory that contains the `pyspark` package, 
i.e. `<checkout>/python`. That's what the new parity step uses (`PYTHONPATH: 
${{ github.workspace }}/python`). "The Python package containing the extension" 
reads as `python/pyspark`, and "point `PYTHONPATH` at that checkout" (next 
line) is the checkout root, which has no top-level `pyspark`. In both cases the 
import fails or silently falls back to another installed pyspark, and the 
master-versus-PR RSS control can measure the wrong build. Could both lines say 
`<PR checkout>/python` and `<master checkout>/python` explicitly? The 
working-directory note below can stay as is.
   
   **Verification:**
   - **Inspection:** The reproduction instructions name the checkout's python 
directory, the one containing the pyspark package, as PYTHONPATH for both the 
PR build and the master control, and no longer refer to the package directory 
itself or the checkout root.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to