[BEAM-8889] Upgrade GCSIO to 2.2.2 #14817

veblush · 2021-05-14T20:08:42Z

This is for upgrading GCSIO to 2.2.2

R: @kennknowles

Changes:

Addressed breaking changes:
- Removed ResilientOperation.getGoogleRequestCallable.
- Applied CreateObjectOptions build pattern.
- Applied new StorageResourceId type.
Code clean-up:
- Applied builder pattern for AsyncWriteChannelOptions and GoogleCloudStorageOptions.
- Updated missing types in the comment (AbstractGoogleAsyncWriteChannel)
Passing the Credentials to GoogleCloudStorageOptions so that it can use DirectPath properly with GCSIO 2.2.2 or later.

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

`ValidatesRunner` compliance status (on master branch)

Lang	ULR	Samza	Twister2
Go	---	---	---
Java
Python	---	---	---
XLang		---	---

Examples testing status on various runners

Lang	ULR	Dataflow	Flink	Samza	Spark	Twister2
Go	---	---	---	---	---	---	---
Java	---		---	---	---	---	---
Python	---	---	---	---	---	---	---
XLang	---	---	---	---	---	---	---

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Go	Java	Python

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website	Whitespace	Typescript
Non-portable
Portable	---			---	---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

veblush · 2021-05-14T20:09:51Z

@mprashanthsagar Could you take a look at the changes to address breaking changes from GCSIO 2.2.x?

veblush · 2021-05-15T03:31:37Z

This is tested with the bleeding edge version of GCSIO with the following instruction. It's verified that Beam job uses Directpath for the GCS operations with the lastest version of GCSIO.

Build GCSIO (2.2.1-SNAPSHOT)

$ ./mvnw -P hadoop2 clean install -DskipTests

Build Beam (After modifying the version of gcsio to 2.2.1-SNAPSHOT)

$ ./gradlew -Ppublishing -PjavaLinkageArtifactIds=beam-sdks-java-io-google-cloud-platform --continue :checkJavaLinkage

Build word-count example (After modifying the version of beam to 2.29.0-SNAPSHOT)

$ mvn compile exec:java -Dexec.mainClass=org.apache.beam.examples.WordCount \
  -Dexec.args="--runner=DataflowRunner --project=$PROJECT \
  --region=$REGION \
  --gcpTempLocation=gs://$BUCKET/tmp \
  --inputFile=gs://$BUCKET/shakespeare/* \
  --output=gs://$BUCKET/counts \
  --subnetwork=$SUBNET_PATH \
  --experiments=use_grpc_for_gcs \
  " -Pdataflow-runner

aaltay · 2021-05-27T22:48:48Z

What is the next step on this PR? Do you need a review or wait until the new GCSIO changes are merged.

veblush · 2021-05-28T21:42:50Z

@aaltay This is still a draft and it needs to wait for the next GCSIO release. Once it's released, this PR will be ready to get reviewed.

mprashanthsagar · 2021-07-01T10:21:27Z

...oogle-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtil.java

@@ -763,7 +751,7 @@ public void onSuccess(StorageObject response, HttpHeaders httpHeaders)
          @Override
          public void onFailure(GoogleJsonError e, HttpHeaders httpHeaders) throws IOException {
            IOException ioException;
-            if (errorExtractor.itemNotFound(e)) {
+            if (e.getCode() == HttpStatusCodes.STATUS_CODE_NOT_FOUND) {


itemNotFound(e) does a recursive check to find the exception, we could have a STATUS_CODE_NOT_FOUND in the cause but not the root exception, Can we retain usage of errorExtractor.itemNotFound() ?

This is not possible anymore because ApiErrorExtrator doesn't support GoogleJsonError anymore by GoogleCloudDataproc/hadoop-connectors#327. I copied the same routine from the old ApiErrorExtrator to do the same thing.

So this is a regression ? Note that Beam file IO connectors are very sensitives to changes in behavior of rename/copy/delete etc. since current behavior is carefully implemented (after many bugs) to be correct when there are step failures and retries.

(unresloving)

Yeah, itemNotFound method was removed by the recent gcsio so I copied the actual implementation of the method to keep it consistent.

...e-cloud-platform-core/src/test/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilTest.java

chamikaramj

Thanks!

chamikaramj · 2021-07-02T19:23:31Z

...oogle-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtil.java

@@ -107,6 +110,7 @@ public GcsUtil create(PipelineOptions options) {
          storageBuilder.getHttpRequestInitializer(),
          gcsOptions.getExecutorService(),
          hasExperiment(options, "use_grpc_for_gcs"),
+          gcsOptions.getGcpCredential(),


This is not backwards compatible. What if gcpCredentials is not provided ? (I assume default credentials will be used but we should make sure that this does not result in a regression).

chamikaramj · 2021-07-02T19:27:45Z

...oogle-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtil.java

+   * @return a SeekableByteChannel that can read the object data
+   */
+  @VisibleForTesting
+  SeekableByteChannel open(GcsPath path, GoogleCloudStorageReadOptions readOptions)


Where is this used ?

This is used for the test. Previously test access the implementation detail (e.g. GoogleCloudStorageReadChannel) in testGCSChannelCloseIdempotent() but it doesn't need to do it anymore with this function.

chamikaramj · 2021-07-02T19:31:50Z

...oogle-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtil.java

@@ -763,7 +751,7 @@ public void onSuccess(StorageObject response, HttpHeaders httpHeaders)
          @Override
          public void onFailure(GoogleJsonError e, HttpHeaders httpHeaders) throws IOException {
            IOException ioException;
-            if (errorExtractor.itemNotFound(e)) {
+            if (e.getCode() == HttpStatusCodes.STATUS_CODE_NOT_FOUND) {


So this is a regression ? Note that Beam file IO connectors are very sensitives to changes in behavior of rename/copy/delete etc. since current behavior is carefully implemented (after many bugs) to be correct when there are step failures and retries.

chamikaramj · 2021-07-07T23:30:59Z

Retest this please

chamikaramj · 2021-07-07T23:31:07Z

Run Java PostCommit

chamikaramj · 2021-07-07T23:31:16Z

Run Dataflow ValidatesRunner

codecov · 2021-07-07T23:51:11Z

Codecov Report

Merging #14817 (b6e2281) into master (7b30339) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #14817   +/-   ##
=======================================
  Coverage   83.78%   83.78%           
=======================================
  Files         441      441           
  Lines       59500    59500           
=======================================
+ Hits        49852    49855    +3     
+ Misses       9648     9645    -3

Impacted Files	Coverage Δ
.../python/apache_beam/testing/test_stream_service.py	`88.37% <0.00%> (-4.66%)`	⬇️
...che_beam/runners/interactive/interactive_runner.py	`90.65% <0.00%> (-1.87%)`	⬇️
...ython/apache_beam/examples/kafkataxi/kafka_taxi.py	`0.00% <0.00%> (ø)`
...hon/apache_beam/runners/direct/test_stream_impl.py	`94.02% <0.00%> (ø)`
...ks/python/apache_beam/runners/worker/sdk_worker.py	`89.00% <0.00%> (+0.15%)`	⬆️
sdks/python/apache_beam/io/range_trackers.py	`94.33% <0.00%> (+0.19%)`	⬆️
...hon/apache_beam/runners/worker/bundle_processor.py	`93.85% <0.00%> (+0.37%)`	⬆️
sdks/python/apache_beam/io/fileio.py	`95.89% <0.00%> (+0.42%)`	⬆️
sdks/python/apache_beam/io/kafka.py	`80.00% <0.00%> (+0.83%)`	⬆️
sdks/python/apache_beam/internal/metrics/metric.py	`87.23% <0.00%> (+1.06%)`	⬆️
... and 1 more

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 7b30339...b6e2281. Read the comment docs.

chamikaramj · 2021-07-08T19:33:48Z

Run PythonDocker PreCommit

chamikaramj · 2021-07-08T19:34:39Z

Run Java_Examples_Dataflow PreCommit

chamikaramj · 2021-07-11T15:43:31Z

Thanks. LGTM

(please make sure that all internal tests pass as well)

veblush force-pushed the gcsio-upgrade branch from 46c6d2f to f1a92b8 Compare May 14, 2021 20:35

veblush force-pushed the gcsio-upgrade branch from a30bffa to b675d82 Compare June 11, 2021 20:06

veblush force-pushed the gcsio-upgrade branch from b675d82 to 4e22d8c Compare June 30, 2021 21:18

veblush added 2 commits June 30, 2021 14:19

Integration

3115078

Pass credentials to gcsio

3315839

veblush force-pushed the gcsio-upgrade branch from 4e22d8c to 3315839 Compare June 30, 2021 21:19

veblush changed the title ~~Upgrade GCSIO to 2.2.x~~ [BEAM-8889] Upgrade GCSIO to 2.2.2 Jun 30, 2021

veblush marked this pull request as ready for review June 30, 2021 21:25

mprashanthsagar reviewed Jul 1, 2021

View reviewed changes

Update by review

b6e2281

mprashanthsagar approved these changes Jul 2, 2021

View reviewed changes

chamikaramj self-requested a review July 2, 2021 17:51

chamikaramj reviewed Jul 2, 2021

View reviewed changes

veblush requested a review from chamikaramj July 7, 2021 23:31

chamikaramj merged commit 2f57eec into apache:master Jul 11, 2021

stormy-ua mentioned this pull request Aug 26, 2021

Shade updated AvroCoder until Beam 2.33.0 is released spotify/scio#3957

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BEAM-8889] Upgrade GCSIO to 2.2.2 #14817

[BEAM-8889] Upgrade GCSIO to 2.2.2 #14817

veblush commented May 14, 2021 •

edited

Loading

veblush commented May 14, 2021

veblush commented May 15, 2021

aaltay commented May 27, 2021

veblush commented May 28, 2021

mprashanthsagar Jul 1, 2021

veblush Jul 1, 2021

chamikaramj Jul 2, 2021

chamikaramj Jul 2, 2021

veblush Jul 7, 2021

chamikaramj left a comment

chamikaramj Jul 2, 2021

chamikaramj Jul 2, 2021

veblush Jul 7, 2021

chamikaramj Jul 2, 2021

chamikaramj commented Jul 7, 2021

chamikaramj commented Jul 7, 2021

chamikaramj commented Jul 7, 2021

codecov bot commented Jul 7, 2021 •

edited

Loading

chamikaramj commented Jul 8, 2021

chamikaramj commented Jul 8, 2021

chamikaramj commented Jul 11, 2021

[BEAM-8889] Upgrade GCSIO to 2.2.2 #14817

[BEAM-8889] Upgrade GCSIO to 2.2.2 #14817

Conversation

veblush commented May 14, 2021 • edited Loading

ValidatesRunner compliance status (on master branch)

Examples testing status on various runners

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

GitHub Actions Tests Status (on master branch)

veblush commented May 14, 2021

veblush commented May 15, 2021

Build GCSIO (2.2.1-SNAPSHOT)

Build Beam (After modifying the version of gcsio to 2.2.1-SNAPSHOT)

Build word-count example (After modifying the version of beam to 2.29.0-SNAPSHOT)

aaltay commented May 27, 2021

veblush commented May 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chamikaramj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chamikaramj commented Jul 7, 2021

chamikaramj commented Jul 7, 2021

chamikaramj commented Jul 7, 2021

codecov bot commented Jul 7, 2021 • edited Loading

Codecov Report

chamikaramj commented Jul 8, 2021

chamikaramj commented Jul 8, 2021

chamikaramj commented Jul 11, 2021

veblush commented May 14, 2021 •

edited

Loading

`ValidatesRunner` compliance status (on master branch)

codecov bot commented Jul 7, 2021 •

edited

Loading