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

fix: StreamWriter hang when we reach the inflight limit control and is doing a retry #799

Merged
merged 6 commits into from Jan 17, 2021

Conversation

yirutang
Copy link
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> ☕️

@yirutang yirutang requested review from a team and steffnay January 14, 2021 23:44
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jan 14, 2021
@product-auto-label product-auto-label bot added the api: bigquerystorage Issues related to the googleapis/java-bigquerystorage API. label Jan 14, 2021
@codecov
Copy link

codecov bot commented Jan 14, 2021

Codecov Report

Merging #799 (b154fc4) into master (031655b) will increase coverage by 0.49%.
The diff coverage is 80.64%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #799      +/-   ##
============================================
+ Coverage     80.64%   81.13%   +0.48%     
- Complexity      992      996       +4     
============================================
  Files            74       74              
  Lines          5333     5339       +6     
  Branches        398      413      +15     
============================================
+ Hits           4301     4332      +31     
+ Misses          848      837      -11     
+ Partials        184      170      -14     
Impacted Files Coverage Δ Complexity Δ
...e/cloud/bigquery/storage/v1beta2/StreamWriter.java 84.98% <80.64%> (+0.40%) 38.00 <0.00> (ø)
...ery/storage/v1beta1/BaseBigQueryStorageClient.java 72.88% <0.00%> (+1.69%) 22.00% <0.00%> (ø%)
...ud/bigquery/storage/v1/BaseBigQueryReadClient.java 60.97% <0.00%> (+2.43%) 12.00% <0.00%> (ø%)
...gquery/storage/v1beta2/BaseBigQueryReadClient.java 60.97% <0.00%> (+2.43%) 12.00% <0.00%> (ø%)
.../bigquery/storage/v1beta2/BigQueryWriteClient.java 75.38% <0.00%> (+4.61%) 33.00% <0.00%> (ø%)
...bigquery/storage/v1alpha2/BigQueryWriteClient.java 80.51% <0.00%> (+6.49%) 38.00% <0.00%> (ø%)
.../google/cloud/bigquery/storage/v1beta2/Waiter.java 72.94% <0.00%> (+15.29%) 18.00% <0.00%> (+4.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 031655b...b154fc4. Read the comment docs.

Comment on lines 948 to 951
} catch (IOException | InterruptedException e) {
LOG.info("Got exception while retrying: " + e.toString());
inflightBatch.onFailure(e);
abortInflightRequests(e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add test coverage for catching these lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. I tried this morning but couldn't figure out a good way to hit the code. I will play around a little more, but could we proceed with this first. Datalfow is waiting for a fix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SG

@stephaniewang526 stephaniewang526 removed the request for review from steffnay January 15, 2021 16:35
@stephaniewang526 stephaniewang526 merged commit f8f9770 into googleapis:master Jan 17, 2021
gcf-merge-on-green bot pushed a commit that referenced this pull request Jan 19, 2021
🤖 I have created a release \*beep\* \*boop\* 
---
### [1.8.5](https://www.github.com/googleapis/java-bigquerystorage/compare/v1.8.4...v1.8.5) (2021-01-17)


### Bug Fixes

* StreamWriter hang when we reach the inflight limit control and is doing a retry ([#799](https://www.github.com/googleapis/java-bigquerystorage/issues/799)) ([f8f9770](https://www.github.com/googleapis/java-bigquerystorage/commit/f8f97701e5ca698a170a1d3b6ecb3886e186f9d5))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@yirutang yirutang deleted the reconnection branch February 18, 2021 00:10
shubhwip pushed a commit to shubhwip/java-bigquerystorage that referenced this pull request Oct 7, 2023
[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [actions/github-script](https://togithub.com/actions/github-script) | action | major | `v3` -> `v5` |

---

### Release Notes

<details>
<summary>actions/github-script</summary>

### [`v5`](https://togithub.com/actions/github-script/compare/v4...v5)

[Compare Source](https://togithub.com/actions/github-script/compare/v4...v5)

### [`v4`](https://togithub.com/actions/github-script/compare/v3...v4)

[Compare Source](https://togithub.com/actions/github-script/compare/v3...v4)

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-storage-nio).
shubhwip pushed a commit to shubhwip/java-bigquerystorage that referenced this pull request Oct 7, 2023
🤖 I have created a release *beep* *boop*
---


### [0.123.19](googleapis/java-storage-nio@v0.123.18...v0.123.19) (2022-02-03)


### Dependencies

* **java:** update actions/github-script action to v5 ([googleapis#1339](googleapis/java-storage-nio#1339)) ([googleapis#800](googleapis/java-storage-nio#800)) ([4c82c37](googleapis/java-storage-nio@4c82c37))
* update actions/github-script action to v5 ([googleapis#799](googleapis/java-storage-nio#799)) ([40febb2](googleapis/java-storage-nio@40febb2))
* update dependency com.google.cloud:google-cloud-shared-dependencies to v2.7.0 ([googleapis#802](googleapis/java-storage-nio#802)) ([2beefb6](googleapis/java-storage-nio@2beefb6))
* update dependency com.google.cloud:google-cloud-storage to v2.2.3 ([googleapis#786](googleapis/java-storage-nio#786)) ([b82657c](googleapis/java-storage-nio@b82657c))
* update dependency com.google.cloud:google-cloud-storage to v2.3.0 ([googleapis#796](googleapis/java-storage-nio#796)) ([e822be5](googleapis/java-storage-nio@e822be5))

---
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
api: bigquerystorage Issues related to the googleapis/java-bigquerystorage API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants