Re: [I] [Parquet][Python][Dataset] dataset(...).to_table() sometimes returns a truncated table instead of raising when one listed Parquet fragment cannot be opened (freshly written files) [arrow]

["kita-renji (via GitHub)" <gi...@apache.org>]

kita-renji commented on issue #51495:
URL: https://github.com/apache/arrow/issues/51495#issuecomment-5843131416

   I can reproduce this on Linux with pyarrow 23.0.1 and 25.0.1, and I think I've found the cause. It's a race in `MergedGenerator`, not the filesystem.
   
   On a plain `LocalFileSystem` I couldn't hit it in ~15k iterations. Routing the same files through a `pyarrow.fs.PyFileSystem` wrapper (which adds thread handoffs) makes it show up at roughly 1-3% with 8 files and one 25-byte bad file. That's with both `use_threads=False` and `True`, and even with `fragment_readahead=1`.
   
   The root cause is in `cpp/src/arrow/util/async_generator.h`. When an inner subscription (here: the fragment whose `GetReaderAsync` fails) delivers an error while no caller is waiting, `InnerCallback` sets `state->broken = true` under the mutex, releases it, and only then stores the error in `MarkFinalError()` (`final_error = err`). If the consumer calls `operator()` in that window, it sees `broken` with `final_error` still OK and gets a plain end-of-stream. The source node finishes normally, the plan completes OK, and `ToTable()` returns whatever batches were already delivered. `OuterCallback` has the same pattern.
   
   This also fits the "shape" observation. The unlocked path is only taken when nobody is waiting, i.e. while the consumer is still processing the previous batch. A 25-byte file fails its footer check almost immediately after that batch is handed off, so the two collide. A larger half-written file fails later, when the consumer is usually already waiting, and that path delivers the error safely.
   
   A standalone C++ loop hits it naturally about 1 in 200k runs. With a 200µs sleep injected before the `final_error` store it goes to 1990/2000. For pyarrow, I rebuilt `libarrow_dataset.so` from apache-arrow-25.0.1 and swapped it into the 25.0.1 wheel: 57/4500 truncated with the unmodified source, 0/11.6k with the fix.
   
   The fix is small: store `final_error` while still holding the lock (and only defer the `all_finished` callback for the case where a caller is already waiting). I'm happy to open a PR with it and a regression test if that sounds right.
   
   Until then, the workaround from the issue (opening each footer with `pq.ParquetFile(path).metadata` before the scan) is a reasonable guard.
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org