Skip to content

Conversation

@veblush
Copy link
Contributor

@veblush veblush commented Apr 27, 2022

Upgraded Google Cloud Storage client (GCSIO) to the latest 2.2.6 which contains recent bug fixes and improvements around gRPC implementation. This change adds the grpc-xds dependency because it started using the Traffic Director for Directpath and this should be transparent to users.


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: @suztomo ).
  • 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.

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)

Build python source distribution and wheels
Python tests
Java tests

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

@asf-ci
Copy link

asf-ci commented Apr 27, 2022

Can one of the admins verify this patch?

2 similar comments
@asf-ci
Copy link

asf-ci commented Apr 27, 2022

Can one of the admins verify this patch?

@asf-ci
Copy link

asf-ci commented Apr 27, 2022

Can one of the admins verify this patch?

@github-actions github-actions bot added the build label Apr 27, 2022
@veblush
Copy link
Contributor Author

veblush commented Apr 27, 2022

R: @suztomo
R: @kennknowles

@veblush
Copy link
Contributor Author

veblush commented Apr 27, 2022

Link check

$ ./gradlew -Ppublishing -PjavaLinkageArtifactIds=beam-sdks-java-io-google-cloud-platform,beam-runners-google-cloud-dataflow-java :checkJavaLinkage  
...
* What went wrong:
Execution failed for task ':sdks:java:io:hcatalog:compileJava'.
> Could not resolve all files for configuration ':sdks:java:io:hcatalog:compileClasspath'.
   > Could not find org.pentaho:pentaho-aggdesigner-algorithm:5.1.5-jhyde.
     Searched in the following locations:
       - file:/usr/local/google/home/veblush/git/beam/sdks/java/io/hcatalog/offline-repository/org/pentaho/pentaho-aggdesigner-algorithm/5.1.5-jhyde/pentaho-aggdesigner-algorithm-5.1.5-jhyde.pom
       - file:/usr/local/google/home/veblush/.m2/repository/org/pentaho/pentaho-aggdesigner-algorithm/5.1.5-jhyde/pentaho-aggdesigner-algorithm-5.1.5-jhyde.pom
       - https://public.nexus.pentaho.org/repository/proxy-public-3rd-party-release/org/pentaho/pentaho-aggdesigner-algorithm/5.1.5-jhyde/pentaho-aggdesigner-algorithm-5.1.5-jhyde.pom
       - https://oss.sonatype.org/content/repositories/staging/org/pentaho/pentaho-aggdesigner-algorithm/5.1.5-jhyde/pentaho-aggdesigner-algorithm-5.1.5-jhyde.pom
       - https://repository.apache.org/snapshots/org/pentaho/pentaho-aggdesigner-algorithm/5.1.5-jhyde/pentaho-aggdesigner-algorithm-5.1.5-jhyde.pom
       - https://repository.apache.org/content/repositories/releases/org/pentaho/pentaho-aggdesigner-algorithm/5.1.5-jhyde/pentaho-aggdesigner-algorithm-5.1.5-jhyde.pom
       - https://maven-central.storage-download.googleapis.com/maven2/org/pentaho/pentaho-aggdesigner-algorithm/5.1.5-jhyde/pentaho-aggdesigner-algorithm-5.1.5-jhyde.pom
     Required by:
         project :sdks:java:io:hcatalog > org.apache.hive:hive-exec:2.1.0 > org.apache.calcite:calcite-core:1.6.0

Error doesn't seem to be relevant to this change.

@veblush veblush marked this pull request as ready for review April 27, 2022 22:52
grpc_netty_shaded : "io.grpc:grpc-netty-shaded", // google_cloud_platform_libraries_bom sets version
grpc_stub : "io.grpc:grpc-stub", // google_cloud_platform_libraries_bom sets version
// Once grpc-xds is added to google_cloud_platform_libraries_bom, use google_cloud_platform_libraries_bom instead.
grpc_xds : "io.grpc:grpc-xds:$grpc_version",
Copy link
Contributor

Choose a reason for hiding this comment

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

Declaring this key in the map doesn't have any effect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I added it to sdks/java/io/google-cloud-platform/build.gradle. PTAL

@codecov
Copy link

codecov bot commented Apr 27, 2022

Codecov Report

Merging #17486 (9721910) into master (53786bc) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #17486      +/-   ##
==========================================
- Coverage   73.83%   73.83%   -0.01%     
==========================================
  Files         690      690              
  Lines       90829    90843      +14     
==========================================
+ Hits        67068    67076       +8     
- Misses      22552    22558       +6     
  Partials     1209     1209              
Flag Coverage Δ
python 83.65% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...eam/runners/portability/fn_api_runner/execution.py 92.44% <0.00%> (-0.65%) ⬇️
...hon/apache_beam/runners/worker/bundle_processor.py 93.26% <0.00%> (-0.38%) ⬇️
...eam/runners/interactive/interactive_environment.py 90.18% <0.00%> (-0.31%) ⬇️
.../python/apache_beam/ml/inference/sklearn_loader.py
...thon/apache_beam/ml/inference/sklearn_inference.py 92.68% <0.00%> (ø)
sdks/python/apache_beam/transforms/util.py 96.06% <0.00%> (+0.08%) ⬆️
...dks/python/apache_beam/examples/cookbook/coders.py 63.15% <0.00%> (+0.99%) ⬆️
sdks/python/apache_beam/internal/metrics/metric.py 93.00% <0.00%> (+1.00%) ⬆️
sdks/python/apache_beam/utils/interactive_utils.py 95.12% <0.00%> (+2.43%) ⬆️

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 53786bc...9721910. Read the comment docs.

@suztomo
Copy link
Contributor

suztomo commented Apr 27, 2022

I haven't run the linkage check on Beam for a while.

Memo: org.pentaho:pentaho-aggdesigner-algorithm is Spring Plugins repository (https://repo.spring.io/plugins-release/) as per https://mvnrepository.com/artifact/org.pentaho/pentaho-aggdesigner-algorithm/5.1.5-jhyde.

@veblush
Copy link
Contributor Author

veblush commented Apr 28, 2022

I haven't run the linkage check on Beam for a while.

Memo: org.pentaho:pentaho-aggdesigner-algorithm is Spring Plugins repository (https://repo.spring.io/plugins-release/) as per https://mvnrepository.com/artifact/org.pentaho/pentaho-aggdesigner-algorithm/5.1.5-jhyde.

I got the same error when running the linkage check on the master so I think it's not a regression caused by this PR.

@suztomo
Copy link
Contributor

suztomo commented Apr 28, 2022

Run SQL_Java17 PreCommit

@suztomo
Copy link
Contributor

suztomo commented Apr 28, 2022

Run SQL Postcommit

@suztomo
Copy link
Contributor

suztomo commented Apr 28, 2022

Run Java PostCommit

@suztomo
Copy link
Contributor

suztomo commented Apr 28, 2022

Run PostCommit_Java_Dataflow

@suztomo
Copy link
Contributor

suztomo commented Apr 28, 2022

Run PostCommit_Java_DataflowV2

@suztomo suztomo changed the title Upgrade GCSIO to 2.2.6 [BEAM-8688] Upgrade GCSIO to 2.2.6 Apr 28, 2022
@suztomo
Copy link
Contributor

suztomo commented Apr 28, 2022

@veblush The ticket ID in the PR suffix is the ticket that records dependency upgrades. Beam project automatically creates the tickets for each dependencies. In this case, it should be https://issues.apache.org/jira/browse/BEAM-8688 (This seems down now)

https://beam.apache.org/contribute/get-started-contributing/#create-a-pull-request

@veblush
Copy link
Contributor Author

veblush commented Apr 28, 2022

@veblush The ticket ID in the PR suffix is the ticket that records dependency upgrades. Beam project automatically creates the tickets for each dependencies. In this case, it should be https://issues.apache.org/jira/browse/BEAM-8688 (This seems down now)

Tomo, the name of this PR starts with [BEAM-8688] and I thought it's enough. What else it needs to have? Should it be suffix? or a different format?

@veblush
Copy link
Contributor Author

veblush commented Apr 28, 2022

Oh, I thought I changed but it seems that you did. Thanks!

@suztomo
Copy link
Contributor

suztomo commented Apr 28, 2022

My local invocation of suztomo@suztomo:~/beam$ sh sdks/java/build-tools/beam-linkage-check.sh origin/master gcsio-226 found dependency conflicts in gRPC classes.

Class io.grpc.ClientStreamTracer$InternalLimitedInfoFactory is not found;
Exception in thread "main"   referenced by 2 class files
    io.grpc.census.CensusStatsModule (io.grpc:grpc-census:1.43.2)
    io.grpc.census.CensusTracingModule (io.grpc:grpc-census:1.43.2)
  Cause:
    Dependency conflict: io.grpc:grpc-api:1.44.0 does not define Class io.grpc.ClientStreamTracer$InternalLimitedInfoFactory but io.grpc:grpc-api:1.43.2 defines it.
      selected: org.apache.beam:beam-sdks-java-io-google-cloud-platform:jar:2.39.0-SNAPSHOT / io.grpc:grpc-api:1.44.0 (compile
      unselected: org.apache.beam:beam-sdks-java-io-google-cloud-platform:jar:2.39.0-SNAPSHOT / org.apache.beam:beam-sdks-java-extensions-google-cloud-platform-core:2.39.0-SNAPSHOT (compile) / com.google.cloud.bigdataoss:gcsio:2.2.6 (compile) / io.grpc:grpc-census:1.43.2 (compile) / io.grpc:grpc-api:1.43.2 (compile)
    Dependency conflict: io.grpc:grpc-api:1.44.0 does not define Class io.grpc.ClientStreamTracer$InternalLimitedInfoFactory but io.grpc:grpc-api:1.43.2 defines it.
      selected: org.apache.beam:beam-sdks-java-io-google-cloud-platform:jar:2.39.0-SNAPSHOT / io.grpc:grpc-api:1.44.0 (compile)
      unselected: org.apache.beam:beam-sdks-java-io-google-cloud-platform:jar:2.39.0-SNAPSHOT / org.apache.beam:beam-sdks-java-extensions-google-cloud-platform-core:2.39.0-SNAPSHOT (compile) / com.google.cloud.bigdataoss:gcsio:2.2.6 (compile) / io.grpc:grpc-census:1.43.2 (compile) / io.grpc:grpc-api:1.43.2 (compile)
Problematic artifacts in the dependency tree:
io.grpc:grpc-census:1.43.2 is at:
  org.apache.beam:beam-sdks-java-io-google-cloud-platform:jar:2.39.0-SNAPSHOT / org.apache.beam:beam-sdks-java-extensions-google-cloud-platform-core:2.39.0-SNAPSHOT (compile) / com.google.cloud.bigdataoss:gcsio:2.2.6 (compile) / io.grpc:grpc-census:1.43.2 (compile)

https://gist.github.com/suztomo/7593d67629f61ce0395335bf911e6e43

gcsio:2.2.6 depends on io.grpc:grpc-census:1.43.2. io.grpc:grpc-census:1.43.2 has some references to io.grpc.ClientStreamTracer$InternalLimitedInfoFactory, which is not available in io.grpc:grpc-api:1.44.0. Is this significant to gcsio?

(I told wrong; the ticket ID is prefix, not suffix)

@suztomo
Copy link
Contributor

suztomo commented Apr 29, 2022

Run PostCommit_Java_Dataflow

@veblush
Copy link
Contributor Author

veblush commented Apr 29, 2022

@suztomo Huh, that's interesting. Thanks for running the test. (I'm not sure why I couldn't catch it earlier with my test runs) grpc/grpc-java#8768 looks relevant. and this is because grpc-census happens to have a different version of gRPC. Let's me add it as a new dependency as well.

@suztomo
Copy link
Contributor

suztomo commented May 2, 2022

Run PostCommit_Java_Dataflow

@suztomo
Copy link
Contributor

suztomo commented May 2, 2022

Nice. No new linkage errors https://gist.github.com/suztomo/0a2b2939876997dbc2312b69b9490104

@suztomo suztomo merged commit 12dbe92 into apache:master May 2, 2022
suztomo added a commit that referenced this pull request May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants