Skip to content

feat: Create Stream writer v2 - starting with core logics #867

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.

Sign up for GitHub

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

Jump to bottom
Merged
stephaniewang526 merged 4 commits into from
Feb 23, 2021
Merged

feat: Create Stream writer v2 - starting with core logics #867

stephaniewang526 merged 4 commits into from
Feb 23, 2021

Conversation

@yayi-google
Copy link
Contributor

@yayi-google yayi-google commented Feb 21, 2021 •
edited by stephaniewang526
Loading

Unblocks Dataflow sink test which unblocks customer update.

@yayi-google
Add a new StreamWriterV2.
b365503 
Compared to existing StreamWriter, its locking mechanism is much
simpler.
@yayi-google
Merge remote-tracking branch 'upstream/master' into stream-writer-v2
d45416d
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 21, 2021
@product-auto-label product-auto-label bot added the api: bigquerystorage Issues related to the googleapis/java-bigquerystorage API. label Feb 21, 2021
@yayi-google yayi-google marked this pull request as ready for review February 21, 2021 01:59
@yayi-google yayi-google requested a review from a team February 21, 2021 01:59
@yayi-google yayi-google requested a review from a team as a code owner February 21, 2021 01:59
bigang-li
bigang-li reviewed Feb 21, 2021
View reviewed changes
@codecov
Copy link

codecov bot commented Feb 22, 2021 •
edited
Loading

Codecov Report

Merging #867 (eabad67) into master (9c103df) will increase coverage by 0.60%.
The diff coverage is 95.71%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #867      +/-   ##
============================================
+ Coverage     80.03%   80.64%   +0.60%     
- Complexity      979     1005      +26     
============================================
  Files            74       76       +2     
  Lines          5290     5430     +140     
  Branches        408      420      +12     
============================================
+ Hits           4234     4379     +145     
- Misses          872      878       +6     
+ Partials        184      173      -11
Impacted Files Coverage Δ Complexity Δ
...cloud/bigquery/storage/v1beta2/StreamWriterV2.java 94.95% <94.95%> (ø) 23.00 <23.00> (?)
...oud/bigquery/storage/v1beta2/StreamConnection.java 100.00% <100.00%> (ø) 3.00 <3.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%> (ø%)

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 9c103df...b98d6be. Read the comment docs.

yirutang
yirutang reviewed Feb 22, 2021
View reviewed changes
@yirutang yirutang self-requested a review February 23, 2021 00:44
bigang-li
bigang-li approved these changes Feb 23, 2021
View reviewed changes
...-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/StreamWriterV2.java
log.info("User closing stream: " + streamName);
this.lock.lock();
try {
this.userClosed = true;
Copy link

@bigang-li bigang-li Feb 23, 2021

Choose a reason for hiding this comment

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

adding a local var to remember the previous value, so if user closed twice, the second time will be no_op (don't call the appendThread.join() multiple times).

...-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/StreamWriterV2.java

if (localQueue.isEmpty()) {
continue;
}
Copy link

@bigang-li bigang-li Feb 23, 2021

Choose a reason for hiding this comment

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

maybe be save a few lines of code here? it's useless according to context.

@stephaniewang526 stephaniewang526 merged commit 7c01f45 into googleapis:master Feb 23, 2021
@yayi-google yayi-google deleted the stream-writer-v2 branch February 25, 2021 01:28
shubhwip pushed a commit to shubhwip/java-bigquerystorage that referenced this pull request Oct 7, 2023
@renovate-bot
build(deps): update dependency com.google.cloud:google-cloud-shared-c…
f255c07 
…onfig to v1.3.2 (googleapis#867)

[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [com.google.cloud:google-cloud-shared-config](https://togithub.com/googleapis/java-shared-config) | `1.3.0` -> `1.3.2` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-shared-config/1.3.2/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-shared-config/1.3.2/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-shared-config/1.3.2/compatibility-slim/1.3.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-shared-config/1.3.2/confidence-slim/1.3.0)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>googleapis/java-shared-config</summary>

### [`v1.3.2`](https://togithub.com/googleapis/java-shared-config/blob/HEAD/CHANGELOG.md#&#8203;132-httpsgithubcomgoogleapisjava-shared-configcomparev131v132-2022-03-28)

[Compare Source](https://togithub.com/googleapis/java-shared-config/compare/v1.3.1...v1.3.2)

### [`v1.3.1`](https://togithub.com/googleapis/java-shared-config/blob/HEAD/CHANGELOG.md#&#8203;131-httpsgithubcomgoogleapisjava-shared-configcomparev130v131-2022-03-25)

[Compare Source](https://togithub.com/googleapis/java-shared-config/compare/v1.3.0...v1.3.1)

</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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yirutang yirutang yirutang approved these changes

@bigang-li bigang-li bigang-li approved these changes

+1 more reviewer

@stephaniewang526 stephaniewang526 stephaniewang526 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: bigquerystorage Issues related to the googleapis/java-bigquerystorage API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@yayi-google @stephaniewang526 @yirutang @bigang-li