sunchao commented on PR #5365:
URL: 
https://github.com/apache/datafusion-comet/pull/5365#issuecomment-5373688862

   Reposting the two remaining P2 findings here for visibility. Both remain 
present at `95125623`; these are the existing findings, not additional issues.
   
   ### [P2] Check selected-file schemes before claiming a shallow clone
   
   The filesystem gate checks only the table's `rootPaths`. A valid Delta 
shallow clone can have a supported `file:` root while its selected data files 
still reference `viewfs:`. With the default libhdfs configuration (`hdfs` 
only), both authority checks accept those files and the contrib claims the 
scan. Native store preparation then fails with `Unable to recognise URL 
"viewfs://..."` instead of falling back to Spark.
   
   This was verified with a real Spark 4.0.2 / Delta 4.0.0 shallow clone that 
Spark successfully reads, plus the exact native store-preparation helper. 
Please apply the supported-scheme check to the selected data-file URIs before 
claiming the scan.
   
   
[Code](https://github.com/apache/datafusion-comet/blob/951256235efa99c35002d10b64368a2a1520482a/contrib/delta-spark/src/main/scala/org/apache/comet/contrib/delta/DeltaScanSupport.scala#L272-L278)
 · [Existing discussion and reproduction 
details](https://github.com/apache/datafusion-comet/pull/5365#discussion_r3831646182)
   
   ### [P2] Account for the DV reader's combined-selection allocation
   
   Construction admission and the initial reader clone are now covered. 
However, DataFusion 54.1 subsequently calls `into_overall_row_selection`, which 
allocates another selector buffer while the attached original and the consumed 
clone's backing vector remain live. The reservation has already been reduced to 
twice the retained selector bytes.
   
   With the default-permitted 1,000,000 alternating deletions across 2,000,000 
rows, the current attachment reserves **64,000,000 bytes**, but the attached 
selectors plus reader-normalization allocations peak at **97,554,457 bytes** 
and retain **65,554,432 bytes** afterward. Please account for normalization and 
vector capacity, or avoid the additional allocation through ownership transfer. 
Simply changing the factor to 3 would still fall below this measured peak.
   
   This was reproduced using the unchanged attachment code and the real locked 
dependency conversion. These are allocator-requested bytes, not RSS or a 
reproduced executor OOM. Both findings were checked with focused probes and 
source tracing, not a full Comet/JNI integration run.
   
   
[Code](https://github.com/apache/datafusion-comet/blob/951256235efa99c35002d10b64368a2a1520482a/native/core/src/execution/delta_dv.rs#L504-L507)
 · [Existing discussion and reproduction 
details](https://github.com/apache/datafusion-comet/pull/5365#discussion_r3831648406)


-- 
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