Skip to content
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.

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

fix: query could hang transaction if ResultSet#next() is not called #643

Merged
merged 1 commit into from Nov 18, 2020

Conversation

olavloite
Copy link
Collaborator

If the first statement of a read/write transaction was a query or a read operation, and the application would not call ResultSet#next() on the return result, the transaction would hang indefinitely as the query would be marked as the one that should initiate the transaction (inline the BeginTransaction option). The query would however never be executed, as the actual query execution is deferred until the first call to ResultSet#next().

Fixes #641

If the first statement of a read/write transaction was a query or a read operation,
and the application would not call ResultSet#next() on the return result, the transaction
would hang indefinetely as the query would be marked as the one that should initiate the
transaction (inline the BeginTransaction option). The query would however never be
executed, as the actual query execution is deferred until the first call to ResultSet#next().

Fixes #641
@olavloite olavloite requested a review from a team as a code owner November 17, 2020 17:29
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/java-spanner API. label Nov 17, 2020
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 17, 2020
@codecov
Copy link

codecov bot commented Nov 17, 2020

Codecov Report

Merging #643 (39a3a85) into master (79fad94) will increase coverage by 0.01%.
The diff coverage is 94.73%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #643      +/-   ##
============================================
+ Coverage     84.04%   84.06%   +0.01%     
+ Complexity     2503     2498       -5     
============================================
  Files           141      141              
  Lines         13810    13812       +2     
  Branches       1318     1317       -1     
============================================
+ Hits          11607    11611       +4     
+ Misses         1657     1655       -2     
  Partials        546      546              
Impacted Files Coverage Δ Complexity Δ
...va/com/google/cloud/spanner/AbstractResultSet.java 83.48% <85.71%> (+0.09%) 28.00 <0.00> (ø)
.../com/google/cloud/spanner/AbstractReadContext.java 86.15% <100.00%> (-0.05%) 47.00 <0.00> (-6.00)
...om/google/cloud/spanner/TransactionRunnerImpl.java 84.27% <100.00%> (ø) 9.00 <0.00> (ø)
.../google/cloud/spanner/SpannerExceptionFactory.java 82.92% <0.00%> (+2.43%) 40.00% <0.00%> (+1.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 79fad94...39a3a85. Read the comment docs.

@thiagotnunes thiagotnunes merged commit 48f92e3 into master Nov 18, 2020
@thiagotnunes thiagotnunes deleted the issue-641 branch November 18, 2020 00:16
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#4478

Changes:

chore: regenerate API index

  Source-Link: googleapis/googleapis@3859723

feat!: storagetransfer public protos update BREAKING CHANGE: field annotations are added. Allowing this since no client libraries were generated based on old protos.
  Committer: @alexander-fenster
  PiperOrigin-RevId: 386124227
  Source-Link: googleapis/googleapis@2d0aae2

fix: add missing dependency configmanagement.proto, cloudauditlogging.proto, metering.proto, multiclusteringress.proto, servicemesh.proto to v1alpha client
  fix: add missing dependency configmanagement.proto, metering.proto, multiclusteringress.proto to v1beta client

  Committer: @chingor13
  PiperOrigin-RevId: 386025193
  Source-Link: googleapis/googleapis@e8d2996
rajatbhatta pushed a commit to rajatbhatta/java-spanner that referenced this pull request Nov 17, 2022
🤖 I have created a release \*beep\* \*boop\*
---
## [2.5.0](https://www.github.com/googleapis/java-spanner-jdbc/compare/v2.4.5...v2.5.0) (2021-10-25)


### Features

* support VIEW in metadata queries ([googleapis#633](https://www.github.com/googleapis/java-spanner-jdbc/issues/633)) ([b929191](https://www.github.com/googleapis/java-spanner-jdbc/commit/b929191a7b6699f9daf9a7c06097e9794c79ff8d)), closes [googleapis#632](https://www.github.com/googleapis/java-spanner-jdbc/issues/632)


### Bug Fixes

* **java:** java 17 dependency arguments ([googleapis#1266](https://www.github.com/googleapis/java-spanner-jdbc/issues/1266)) ([googleapis#645](https://www.github.com/googleapis/java-spanner-jdbc/issues/645)) ([0474502](https://www.github.com/googleapis/java-spanner-jdbc/commit/0474502936ff1a43244fcb830fecfc5f42895899))


### Dependencies

* update dependency com.google.cloud:google-cloud-shared-dependencies to v2.4.0 ([googleapis#641](https://www.github.com/googleapis/java-spanner-jdbc/issues/641)) ([ab26010](https://www.github.com/googleapis/java-spanner-jdbc/commit/ab26010ba107d4ba9591d661743ad542ae3b227f))
* update dependency com.google.cloud:google-cloud-spanner-bom to v6.13.0 ([googleapis#637](https://www.github.com/googleapis/java-spanner-jdbc/issues/637)) ([d981c8c](https://www.github.com/googleapis/java-spanner-jdbc/commit/d981c8c744829aa039b16df0b150caf49a99f1cc))
* update dependency com.google.cloud:google-cloud-spanner-bom to v6.14.0 ([googleapis#647](https://www.github.com/googleapis/java-spanner-jdbc/issues/647)) ([3cda837](https://www.github.com/googleapis/java-spanner-jdbc/commit/3cda83737c25c8878d19bee1727de84e086065b6))
* upgrade Mockito to support Java17 ([googleapis#635](https://www.github.com/googleapis/java-spanner-jdbc/issues/635)) ([d78792f](https://www.github.com/googleapis/java-spanner-jdbc/commit/d78792f7acdd2fdac8a655fba06789ba50457679))
---


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
Labels
api: spanner Issues related to the googleapis/java-spanner API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TransactionRunner can get stuck if the first statement is a query and ResultSet#next() is never called
2 participants