jiayuasu commented on code in PR #1187:
URL: https://github.com/apache/sedona-db/pull/1187#discussion_r3911547614
##########
rust/sedona-datasource/src/format.rs:
##########
@@ -216,6 +226,10 @@ impl FileFormat for ExternalFileFormat {
#[derive(Debug, Clone)]
struct ExternalFileSource {
spec: Arc<dyn ExternalFormatSpec>,
+ /// Shared by all openers cloned from this physical scan. Keeping the lock
+ /// here scopes serialization to one scan instead of coupling independent
+ /// user streams through a process-wide lock.
+ reader_lock: Option<Arc<Mutex<()>>>,
Review Comment:
Thanks, you were right that the previous coexistence test was not enough.
The committed Python test uses two threads, two connections, and two distinct
datasets. It synchronizes immediately before real pyogrio backed
read_next_batch calls, requires overlapping native call intervals across
multiple batches, and validates every row for FlatGeoBuf and GeoPackage. It
runs in a spawned child process with a hard timeout and exit code reporting.
I also added a progress test. Reader A consumes one batch and remains alive.
Reader B completes, then reader A resumes. A temporary process wide lifetime
lock mutation makes this test time out, so holding one global mutex for the
full lifetime of a lazy reader would deadlock valid usage. That is why this PR
keeps lifecycle serialization scoped to one scan.
I also stress tested the original FlatGeoBuf path locally with open versus
read and close versus read overlap. I left that timing harness out of the PR
because it added substantial test complexity and its evidence is specific to
the tested build.
This evidence is limited to the tested drivers and builds. It is not a claim
that every GDAL driver is universally safe. If the wheel matrix reproduces a
crash under the forced independent overlap, the next design should serialize
individual native operations across the process rather than hold a lock for the
reader lifetime.
--
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]
