Fix TextSource incorrect handling in channels that return short reads.#23376
Fix TextSource incorrect handling in channels that return short reads.#23376lukecwik merged 2 commits intoapache:masterfrom
Conversation
The issue is that readDefaultLine and readCustomLine was incorrectly calculating the appendLength when the buffer returned was 0 length. This was solved by ensuring that the internal read loop always read at least one byte allowing for the code to ensure that we were making progress. For readDefaultLine we kept track of whether we need to skip an LF in the next buffer if the current buffer ended with a CR and for readCustomLine we had to remember how much of the delimiter we have read so far in this buffer. The bug was introduced in apache@30a48f0 There was no noticeable change in the TextSourceBenchmark performance results. Fixes apache#23375
| reader.startReading( | ||
| new ReadableByteChannel() { |
There was a problem hiding this comment.
nit: could be nice to document this for readability (could happen later though, I know you want to get this in)
| reader.startReading( | |
| new ReadableByteChannel() { | |
| reader.startReading( | |
| // Placeholder channel that only yields 0- and 1-length buffers. | |
| // Data is read one byte at a time from line parameter | |
| new ReadableByteChannel() { |
|
It looks like the bug was caught with an internal Google test, please make sure that this patch fixes it before merging. Thanks! |
|
Assigning reviewers. If you would like to opt out of this review, comment R: @robertwb for label java. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Will do. |
|
Java_Examples_Dataflow_Java11 succeeded, github UI failed to update: https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Commit/12526/ |
|
Internal testing within Google also passed. |
…rt reads. (apache#23376)" This reverts commit 7dd1c6b.
2 participants
The issue is that readDefaultLine and readCustomLine was incorrectly calculating the appendLength when the buffer returned was 0 length. This was solved by ensuring that the internal read loop always read at least one byte allowing for the code to ensure that we were making progress. For readDefaultLine we kept track of whether we need to skip an LF in the next buffer if the current buffer ended with a CR and for readCustomLine we had to remember how much of the delimiter we have read so far in this buffer.
The bug was introduced in 30a48f0
There was no noticeable change in the TextSourceBenchmark performance results.
Fixes #23375
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.