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

test: increase timeout to prevent flakiness #709

Merged
merged 1 commit into from Dec 13, 2020
Merged

Conversation

olavloite
Copy link
Collaborator

The StatementTimeoutTest.testTimeoutExceptionReadWriteTransactionMultipleStatements had become flaky because of a combination of two other changes:

  1. The first statement of a read/write transaction now includes a BeginTransaction option. If that statement fails with an error, the entire transaction will be retried, but then with an explicit BeginTransaction RPC. The latter is done to ensure that the statement that returned an error will be included in the transaction.
  2. The Connection API did not correctly handle a situation where a statement first returned an error unequal to Aborted, and then during a retry would return Aborted.

The combination of the two above changes means that the first statement of a transaction can take longer than expected when it returns an error, as it will automatically trigger a retry of the transaction, and thereby also be executed twice without the client application knowing.

Fixes #708

@olavloite olavloite requested a review from a team as a code owner December 10, 2020 16:17
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Dec 10, 2020
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/java-spanner API. label Dec 10, 2020
@olavloite olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 10, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 10, 2020
@codecov
Copy link

codecov bot commented Dec 13, 2020

Codecov Report

Merging #709 (7cf3748) into master (fa19940) will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #709      +/-   ##
============================================
+ Coverage     85.05%   85.10%   +0.05%     
- Complexity     2556     2560       +4     
============================================
  Files           142      142              
  Lines         13930    13930              
  Branches       1326     1326              
============================================
+ Hits          11848    11855       +7     
+ Misses         1526     1519       -7     
  Partials        556      556              
Impacted Files Coverage Δ Complexity Δ
.../google/cloud/spanner/SpannerExceptionFactory.java 85.56% <0.00%> (+3.09%) 48.00% <0.00%> (+2.00%)
...m/google/cloud/spanner/connection/FailedQuery.java 79.31% <0.00%> (+3.44%) 3.00% <0.00%> (+1.00%)
...r/AbortedDueToConcurrentModificationException.java 85.71% <0.00%> (+42.85%) 2.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 fa19940...7cf3748. Read the comment docs.

@thiagotnunes thiagotnunes merged commit bfb7617 into master Dec 13, 2020
@thiagotnunes thiagotnunes deleted the issue-708 branch December 13, 2020 23:33
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#5035

Changes:

chore: regenerate API index

  Source-Link: googleapis/googleapis@c33c435

feat: Cloud IDS client v1 Based on: https://cloud.google.com/ids/docs
  PiperOrigin-RevId: 405832509
  Source-Link: googleapis/googleapis@a61dbc0

chore(security/privateca): Update privateca BUILD.bazel package name to google-cloud-private-ca
  PiperOrigin-RevId: 405697254
  Source-Link: googleapis/googleapis@4ee0e57

docs(security/privateca): fix docstring formatting
  Committer: @parthea
  PiperOrigin-RevId: 405687537
  Source-Link: googleapis/googleapis@5f897f0

feat: Publish Cloud Integrations logging proto for documentation
  Committer: @jinmichaelgu
  PiperOrigin-RevId: 405667694
  Source-Link: googleapis/googleapis@7f52277
rajatbhatta pushed a commit to rajatbhatta/java-spanner that referenced this pull request Nov 17, 2022
…v6.17.4 (googleapis#709)

[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [com.google.cloud:google-cloud-spanner-bom](https://togithub.com/googleapis/java-spanner) | `6.17.3` -> `6.17.4` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-spanner-bom/6.17.4/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-spanner-bom/6.17.4/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-spanner-bom/6.17.4/compatibility-slim/6.17.3)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-spanner-bom/6.17.4/confidence-slim/6.17.3)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>googleapis/java-spanner</summary>

### [`v6.17.4`](https://togithub.com/googleapis/java-spanner/blob/HEAD/CHANGELOG.md#&#8203;6174-httpswwwgithubcomgoogleapisjava-spannercomparev6173v6174-2022-01-07)

[Compare Source](https://togithub.com/googleapis/java-spanner/compare/v6.17.3...v6.17.4)

</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
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.

StatementTimeoutTest is flaky
3 participants