[C++] CSVBufferIterator potentially used incorrectly
- Dominant language
- C++
- Stars
- 17.1k
- Forks
- 4.3k
- Avg merge
- 3d 18h
- Merged PRs (30d)
- 91
Description
### Describe the bug, including details regarding any error messages, version, and platform.
The CSV streaming reader has this snippet:
https://github.com/apache/arrow/blob/6f4a539323da0d6aef528ccce2404e36f0a08585/cpp/src/arrow/csv/reader.cc#L860-L862
and there's a similar one in the one-shot reader implementation.
This will potentially execute `CSVBufferIterator::operator()` concurrently from multiple threads, but it is not thread-safe as it mutates internal state. Its execution should be serialized.
Fortunately, `CSVBufferIterator` should be very cheap CPU-wise, so instead of transferring it to the CPU executor it can probably be serialized instead from the `IOStream` iterator.
However, there seems to be a similar problem with `SerialBlockReader` which is potentially heavier, especially if `ParseOptions::newlines_in_values` has been enabled.
So perhaps we instead need a more general `SerializingGenerator` building block that delays execution of a generator until its previous execution terminated.
### Component(s)
C++
Contributor guide
Research direction
Start with cpp/src/arrow/csv/reader.cc at the linked streaming-reader lines, then inspect the similar one-shot reader implementation and the CSVBufferIterator and SerialBlockReader paths. Determine how concurrent execution can occur and compare the serialization options described in the issue. Done means both reader paths safely serialize the stateful work without losing the intended streaming behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- data-engineering
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100