Upgrade Spanner to the latest version#37433
Upgrade Spanner to the latest version#37433sakthivelmanii wants to merge 8 commits intoapache:masterfrom
Conversation
Summary of ChangesHello @sakthivelmanii, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the dependency management for Google Cloud Spanner within the project's Gradle build system. Instead of explicitly declaring and pinning the Spanner version, the changes delegate this responsibility to the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
b36d8fc to
636fccc
Compare
636fccc to
789e4f4
Compare
...io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java
Outdated
Show resolved
Hide resolved
.../google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java
Outdated
Show resolved
Hide resolved
...gle-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java
Outdated
Show resolved
Hide resolved
dc8a042 to
b930c89
Compare
| } | ||
| SpannerAccessor sa = SpannerAccessor.getOrCreate(spannerConfigWithCredential); | ||
| DatabaseClient databaseClient = sa.getDatabaseClient(); | ||
| return databaseClient.getDialect(); |
There was a problem hiding this comment.
Is there a reason to undo this change? It seems like it is still probably better to force the SpannerAccessor class to close
| def google_cloud_bigdataoss_version = "2.2.26" | ||
| // [bomupgrader] determined by: com.google.cloud:google-cloud-spanner, consistent with: google_cloud_platform_libraries_bom | ||
| def google_cloud_spanner_version = "6.104.0" | ||
| def google_cloud_spanner_version = "6.107.0" |
There was a problem hiding this comment.
Do we need this version to be tracked at all now or can we remove this line?
.../google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java
Show resolved
Hide resolved
| .map(ValueProvider::get) | ||
| .orElse(DEFAULT_SESSION_WAIT_DURATION); | ||
| builder.setSessionPoolOption( | ||
| SessionPoolOptions.newBuilder().setWaitForMinSessionsDuration(waitDuration).build()); |
There was a problem hiding this comment.
Is this change related to the hangs?
There was a problem hiding this comment.
No. This is related to DEADLINE EXCEEDED in dataflow workers. b/462499883
c938e2b to
a61409b
Compare
|
Assigning reviewers: R: @ahmedabu98 for label java. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
| * @return {@link SpannerConfig} | ||
| * <p>Sets the wait time for multiplexed session to be available while creating a database | ||
| * client. Setting this will block the {@link com.google.cloud.spanner.DatabaseClient} | ||
| * creation. By default, We will be setting 5 mins as minimum wait time. |
There was a problem hiding this comment.
Link to the variable in these docs, just in case this changes at a later date, the docs need to be in Sync
cbdcee0 to
1bc651a
Compare
damccorm
left a comment
There was a problem hiding this comment.
Thanks, LGTM. Just waiting on checks to succeed before merging.
1bc651a to
736e6d0
Compare
|
Please don't force push since that:
I'll retry tests if there are flakes |
|
Spotless and precommit java failures are unrelated, fixed by #37505 |
|
The GCP IO one seems maybe related though: @sakthivelmanii could you please take a look? This looks mostly green at head - https://github.com/apache/beam/actions/workflows/beam_PreCommit_Java_GCP_IO_Direct.yml?query=branch%3Amaster+event%3Aschedule You can download the test results from https://github.com/apache/beam/actions/runs/21685692071?pr=37433 |
dc69dc1 to
45e1d86
Compare
|
I'm still seeing Spanner ChangeStreams test failures Also, as mentioned above - please avoid force pushing, it makes reviewing the PR harder |
|
The same tests are failing in master branch as well. |
|
I had to rebase my branch with the latest master to make spotless tests to pass |
https://github.com/apache/beam/actions/workflows/beam_PreCommit_Java_GCP_IO_Direct.yml?query=event%3Aschedule - it looks like
Note that you can do this by merging in the master branch without losing commit history (which is the thing that GitHub's review feature relies on). ( |
|
@damccorm I am seeing that setting SessionPoolOptions is causing the issue. I am trying to debug the issue. I will move this into two different PRs.
|
|
Sounds good, thanks - please comment once they are ready for review |
|
Reminder, please take a look at this pr: @ahmedabu98 @damccorm @nielm |
|
Closing this PR for now since it was split up |
Please add a meaningful description for your change here
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.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)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.