Skip to content

Conversation

@skuruppu
Copy link
Contributor

@skuruppu skuruppu commented Jul 7, 2020

Use Docker image instead of manually downloading the emulator binary and running it.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 7, 2020
Use Docker image instead of manually downloading the emulator binary and
running it.
@skuruppu skuruppu force-pushed the cleanup-emulator-github-action branch from 059ecb5 to 5a13452 Compare July 7, 2020 02:50
@skuruppu skuruppu requested a review from olavloite July 7, 2020 02:57
@skuruppu skuruppu added the type: cleanup An internal cleanup or hygiene concern. label Jul 7, 2020
@skuruppu skuruppu changed the title test: change system-test setup against emulator test: clean up system-test setup against emulator Jul 7, 2020
@olavloite
Copy link
Collaborator

olavloite commented Jul 7, 2020

EDIT: This seems to be caused by a race condition where the Rollback call is executed before the UPDATE ... statement has executed

The timeout in the integration tests against the emulator is caused by an eternal aborted-retry loop in ITReadWriteAutocommitSpannerTest. What happens in the following:

  1. Transaction 1 sends UPDATE TEST SET NAME='test18' WHERE ID=1000 and then almost directly calls Rollback without waiting for the answer for the UPDATE ... statement.
  2. Transaction 2 executes DELETE FROM TEST WHERE ID=1000. This statement returns an Aborted error. No Rollback or Commit is called, as the transaction has already been aborted.
  3. Transaction 2 is retried and executes DELETE FROM TEST WHERE ID=1000 in a new transaction. This statement returns an Aborted error. No Rollback or Commit is called, as the transaction has already been aborted.
  4. ... this repeats for ever ...

It seems that the above happens when the Rollback in step 1 is executed before the UPDATE ... statement has executed (fully), as adding a small delay between the two RPCs does fix the problem.

Copy link
Collaborator

@olavloite olavloite left a comment

Choose a reason for hiding this comment

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

That really cleans it up a lot :-)

The test failure is not really related to this change, although it could be that the latest version of the emulator is behaving slightly differently from the previous version.

@skuruppu skuruppu added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 9, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 9, 2020
@skuruppu skuruppu added the automerge Merge the pull request once unit tests and other checks pass. label Jul 9, 2020
@skuruppu skuruppu merged commit 8cd79e7 into master Jul 9, 2020
@skuruppu skuruppu deleted the cleanup-emulator-github-action branch July 9, 2020 05:21
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, genmgr will
update the corresponding CL at gocloud to depend on the newer version of
go-genproto, and assign reviewers. Whilst this or any regen PR is open in
go-genproto, gapicgen will not create any more regeneration PRs or CLs. If all
regen PRs are closed, gapicgen will create a new set of regeneration PRs and
CLs once per night.

If you have been assigned to review this CL, 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
  genmgr to assign reviewers to the gocloud CL.

Corresponding gocloud CL: https://code-review.googlesource.com/c/gocloud/+/53010
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

automerge Merge the pull request once unit tests and other checks pass. cla: yes This human has signed the Contributor License Agreement. type: cleanup An internal cleanup or hygiene concern.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants