test: fixes DatabaseClientImpl stuck test #798
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The DatabaseClientImpl#singleUseBoundAsync is sometimes getting stuck during the execution. This is simple to reproduce by executing the test class multiple times. This is due to the static single threaded executor that is shared among all the test cases. In this fix, we create an executor in the setup method, avoiding the sharing of the same amongt different tests.
Contributor
Author
|
@olavloite could you let me know if this makes sense or was the intent of the test to exercise a single shared executor among all the test cases? |
Codecov Report
@@ Coverage Diff @@
## master #798 +/- ##
============================================
+ Coverage 85.01% 85.08% +0.06%
+ Complexity 2562 2561 -1
============================================
Files 143 143
Lines 14015 14043 +28
Branches 1341 1341
============================================
+ Hits 11915 11948 +33
+ Misses 1537 1533 -4
+ Partials 563 562 -1
Continue to review full report at Codecov.
|
Collaborator
There's no specific reason that the test was using a single shared executor, but at the same time I also don't see any specific reason that it should be necessary to create a new executor for each test, which makes me worry that this fix is actually just covering some other underlying problem. |
Collaborator
|
@thiagotnunes How are you running the test when it gets stuck? I am not able to reproduce it locally, both when running it through Maven ( |
Contributor
Author
|
@olavloite that is interesting. I am running it through Intellij by changing the runner to repeat the test execution until Failure (it does not actually fail, but it gets stuck). I can consistently reproduce this, on the second execution of the test. I will see if I have the time to do some more investigation on what's going on. |
olavloite
approved these changes
Jan 17, 2021
Collaborator
olavloite
left a comment
There was a problem hiding this comment.
I tried it out in the test runner in IntelliJ in the way that you suggested, and I can also reproduce it there. The problem seems to be that IntelliJ in that case will never unload the class, which again means that the executor is only initialized during the first run and then shutdown at the end. So the problem is not some underlying bug, and this fix is safe.
I would however suggest that we consider changing it in a slightly different way: Keep the executor static (but not final) and initialize it in the startStaticServer() method. Then we can also keep the executor.shutdown() in the stopServer() method.
Contributor
Author
|
@olavloite good suggestion, this way we cover if the thread could be hanging somewhere in between tests |
thiagotnunes
added a commit
that referenced
this pull request
May 6, 2021
* test: fixes DatabaseClientImpl stuck test The DatabaseClientImpl#singleUseBoundAsync is sometimes getting stuck during the execution. This is simple to reproduce by executing the test class multiple times. This is due to the static single threaded executor that is shared among all the test cases. In this fix, we create an executor in the setup method, avoiding the sharing of the same amongt different tests. * test: executor as static in test
ansh0l
pushed a commit
to ansh0l/java-spanner
that referenced
this pull request
Nov 10, 2022
This is an auto-generated regeneration of the .pb.go files by cloud.google.com/go/internal/gapicgen. Once this PR is submitted, genbot will update the corresponding PR to depend on the newer version of go-genproto, and assign reviewers. Whilst this or any regen PR is open in go-genproto, genbot will not create any more regeneration PRs. If all regen PRs are closed, gapicgen will create a new set of regeneration PRs once per night. If you have been assigned to review this PR, please: - Ensure that CI is passing. If it's failing, it requires your manual attention. - Approve and submit this PR if you believe it's ready to ship. That will prompt genbot to assign reviewers to the google-cloud-go PR. Corresponding google-cloud-go PR: googleapis/google-cloud-go#5924 Changes: fix(compute): revert proto3_optional, required removal on parent_id (googleapis#714) Source-Link: googleapis/googleapis@6b3fdce build(servicemanagement): fix broken links PiperOrigin-RevId: 443188324 Source-Link: googleapis/googleapis@775267e
rajatbhatta
pushed a commit
to rajatbhatta/java-spanner
that referenced
this pull request
Nov 17, 2022
…cies to v2.10.0 (googleapis#798) [](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.cloud:google-cloud-shared-dependencies](https://togithub.com/googleapis/java-shared-dependencies) | `2.9.0` -> `2.10.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>googleapis/java-shared-dependencies</summary> ### [`v2.10.0`](https://togithub.com/googleapis/java-shared-dependencies/blob/HEAD/CHANGELOG.md#​2100-httpsgithubcomgoogleapisjava-shared-dependenciescomparev290v2100-2022-04-15) [Compare Source](https://togithub.com/googleapis/java-shared-dependencies/compare/v2.9.0...v2.10.0) ##### Dependencies - google-cloud-core 2.6.0 ([#​668](https://togithub.com/googleapis/java-shared-dependencies/issues/668)) ([22f4d18](https://togithub.com/googleapis/java-shared-dependencies/commit/22f4d1809cbb9848174b3569ab527e4bef00d443)) - reverting protobuf to 3.19.4 ([#​657](https://togithub.com/googleapis/java-shared-dependencies/issues/657)) ([8501e6d](https://togithub.com/googleapis/java-shared-dependencies/commit/8501e6d842c10d2370bbd5d5246070134336bddd)) - update dependency com.fasterxml.jackson:jackson-bom to v2.13.2.20220328 ([#​646](https://togithub.com/googleapis/java-shared-dependencies/issues/646)) ([7bfd6d7](https://togithub.com/googleapis/java-shared-dependencies/commit/7bfd6d7073859d1955b91b368c6713a72ffa14b6)) - update dependency com.google.api-client:google-api-client-bom to v1.34.0 ([#​662](https://togithub.com/googleapis/java-shared-dependencies/issues/662)) ([1b8e378](https://togithub.com/googleapis/java-shared-dependencies/commit/1b8e378fe0ccf2a28c759868caaf5ba593a95728)) - update dependency com.google.errorprone:error_prone_annotations to v2.12.1 ([#​652](https://togithub.com/googleapis/java-shared-dependencies/issues/652)) ([1cc80ee](https://togithub.com/googleapis/java-shared-dependencies/commit/1cc80ee984ebcad9bc2a95e2f28c0a49fe924b37)) - update dependency com.google.errorprone:error_prone_annotations to v2.13.0 ([#​669](https://togithub.com/googleapis/java-shared-dependencies/issues/669)) ([61b7834](https://togithub.com/googleapis/java-shared-dependencies/commit/61b78341b34a251722be4805a6bdd895cd64836c)) - update dependency com.google.http-client:google-http-client-bom to v1.41.6 ([#​654](https://togithub.com/googleapis/java-shared-dependencies/issues/654)) ([140ef40](https://togithub.com/googleapis/java-shared-dependencies/commit/140ef405bc17ed83f5ce177df59affca14fbe49c)) - update dependency com.google.http-client:google-http-client-bom to v1.41.7 ([#​658](https://togithub.com/googleapis/java-shared-dependencies/issues/658)) ([f6f93e5](https://togithub.com/googleapis/java-shared-dependencies/commit/f6f93e5b9172c9684623c4c148e4a8fe2fae1e94)) - update dependency com.google.oauth-client:google-oauth-client-bom to v1.33.2 ([#​655](https://togithub.com/googleapis/java-shared-dependencies/issues/655)) ([20cd9ed](https://togithub.com/googleapis/java-shared-dependencies/commit/20cd9eda112c96836a5ab7485a4247ed2bc0edb8)) - update dependency com.google.oauth-client:google-oauth-client-bom to v1.33.3 ([#​663](https://togithub.com/googleapis/java-shared-dependencies/issues/663)) ([f011a46](https://togithub.com/googleapis/java-shared-dependencies/commit/f011a46c551dba16851b4f8c919c40452fc5d5c3)) - update dependency com.google.protobuf:protobuf-bom to v3.20.0 ([#​651](https://togithub.com/googleapis/java-shared-dependencies/issues/651)) ([ad2ff73](https://togithub.com/googleapis/java-shared-dependencies/commit/ad2ff73207dd6493321c77d8eca0022baf13b4ce)) - update dependency io.grpc:grpc-bom to v1.45.1 ([#​647](https://togithub.com/googleapis/java-shared-dependencies/issues/647)) ([38e46fc](https://togithub.com/googleapis/java-shared-dependencies/commit/38e46fc4f03af0a02f30ce4a2fa222c71797ae15)) - update dependency org.checkerframework:checker-qual to v3.21.4 ([#​650](https://togithub.com/googleapis/java-shared-dependencies/issues/650)) ([125e80a](https://togithub.com/googleapis/java-shared-dependencies/commit/125e80ab2c3225a00c03f5ff5de94872ebb94303)) - update gax.version to v2.15.0 ([#​649](https://togithub.com/googleapis/java-shared-dependencies/issues/649)) ([c7f32a6](https://togithub.com/googleapis/java-shared-dependencies/commit/c7f32a68b14520104432282ac9598643700162eb)) - update gax.version to v2.16.0 ([#​664](https://togithub.com/googleapis/java-shared-dependencies/issues/664)) ([caaf941](https://togithub.com/googleapis/java-shared-dependencies/commit/caaf941643af04295f5527a0144587d7bf040862)) - update google.common-protos.version to v2.8.1 ([#​656](https://togithub.com/googleapis/java-shared-dependencies/issues/656)) ([df4a4a2](https://togithub.com/googleapis/java-shared-dependencies/commit/df4a4a2130a3cdb2965ea42962d1ea6a85506ba7)) - update google.common-protos.version to v2.8.2 ([#​659](https://togithub.com/googleapis/java-shared-dependencies/issues/659)) ([b499e2b](https://togithub.com/googleapis/java-shared-dependencies/commit/b499e2bc99506d48d26e35bf6e68c09409ce8b11)) - update google.common-protos.version to v2.8.3 ([#​660](https://togithub.com/googleapis/java-shared-dependencies/issues/660)) ([461081c](https://togithub.com/googleapis/java-shared-dependencies/commit/461081c0cf73057c1f6e07fc573453ad467a60ae)) - update iam.version to v1.3.0 ([#​648](https://togithub.com/googleapis/java-shared-dependencies/issues/648)) ([6670c4f](https://togithub.com/googleapis/java-shared-dependencies/commit/6670c4f61fcf075c543bfd148eea49796e0662ce)) - update iam.version to v1.3.1 ([#​661](https://togithub.com/googleapis/java-shared-dependencies/issues/661)) ([cc8fbe6](https://togithub.com/googleapis/java-shared-dependencies/commit/cc8fbe6eae03341c2ece7d3356febc843a74a812)) </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-spanner-jdbc).
rajatbhatta
pushed a commit
to rajatbhatta/java-spanner
that referenced
this pull request
Nov 17, 2022
🤖 I have created a release *beep* *boop* --- ### [2.6.4](googleapis/java-spanner-jdbc@v2.6.3...v2.6.4) (2022-04-21) ### Dependencies * update dependency com.google.cloud:google-cloud-shared-dependencies to v2.10.0 ([googleapis#798](googleapis/java-spanner-jdbc#798)) ([a77024c](googleapis/java-spanner-jdbc@a77024c)) --- 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
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The DatabaseClientImpl#singleUseBoundAsync is sometimes getting stuck during the execution. This is simple to reproduce by executing the test class multiple times.
This is due to the static single threaded executor that is shared among all the test cases.
In this fix, we create an executor in the setup method, avoiding the sharing of the same among different tests.