Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

simplify BlockingProcessStreamReader; fix tests #1457

Merged
merged 1 commit into from
Dec 8, 2016
Merged

simplify BlockingProcessStreamReader; fix tests #1457

merged 1 commit into from
Dec 8, 2016

Conversation

pongad
Copy link
Contributor

@pongad pongad commented Dec 8, 2016

Update #1429.

This commit simplifies the main run() method.
Previous implementation checks whether the underlying Reader is ready to read
then either reads a line or sleeps,
catching InterruptedException to watch for any thread interruption.

There are subtle difficulties with this approach:

  • Even if the underlying Reader is ready to read, it might not have
    enough bytes to form a line. It might still block.
  • It's not necessary to sleep. If the thread is interrupted while
    reading. It should throw InterruptedIOException.
    The method now reads in a loop, waiting for either exceptions or EOF.

The test class implements a mock Logger that logs to a data structure.
It then verifies that the data structure holds appropriate logs.
As implemented, this can cause a race, as two threads,
the writer and the verifier, run concurrently.
This commit fixes this by waiting for the writing thread to terminate
before verifying.

Update #1429.

This commit simplifies the main run() method.
Previous implementation checks whether the underlying Reader is ready to read
then either reads a line or sleeps,
catching InterruptedException to watch for any thread interruption.

There are subtle difficulties with this approach:
- Even if the underlying Reader is ready to read, it might not have
  enough bytes to form a line. It might still block.
- It's not necessary to sleep. If the thread is interrupted while
  reading. It should throw InterruptedIOException.
The method now reads in a loop, waiting for either exceptions or EOF.

The test class implements a mock Logger that logs to a data structure.
It then verifies that the data structure holds appropriate logs.
As implemented, this can cause a race, as two threads,
the writer and the verifier, run concurrently.
This commit fixes this by waiting for the writing thread to terminate
before verifying.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 8, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 83.577% when pulling fca0cfe on pongad:stream-reader into cacf65e on GoogleCloudPlatform:master.

@garrettjonesgoogle garrettjonesgoogle merged commit 614cbe4 into googleapis:master Dec 8, 2016
@pongad pongad deleted the stream-reader branch December 12, 2016 03:33
github-actions bot pushed a commit that referenced this pull request Sep 15, 2022
🤖 I have created a release *beep* *boop*
---


## [2.3.7](googleapis/java-bigquerydatatransfer@v2.3.6...v2.3.7) (2022-08-29)


### Dependencies

* update dependency com.google.cloud:google-cloud-bigquery to v2.15.0 ([#1456](googleapis/java-bigquerydatatransfer#1456)) ([a20b6e6](googleapis/java-bigquerydatatransfer@a20b6e6))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants