You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/06/24 03:29:15 UTC

[GitHub] [beam] byronellis opened a new pull request, #22035: Fix DEADLINE_EXCEEDED flakiness

byronellis opened a new pull request, #22035:
URL: https://github.com/apache/beam/pull/22035

   Fixes #22008 
   
   Addresses DEADLINE_EXCEEDED flakiness by testing for "at least" rather than…"exactly" the number of retries. 
   
   Randomness in the FluentBackoff implementation empirically allows (with current defaults) the number of actual retries in this test to drift between 8 and 10 retries. Other approaches to making this test deterministic (e.g. by setting the random number generator seed or explicitly specifying a maximum number of retries) would likely require fairly substantial refactoring of the SpannerIO configuration as none of these parameters appear to be plumbed through at present.
   
   
   
   **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:
   
    - [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [ ] Mention the appropriate issue in your description (for example: `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, comment `fixes #<ISSUE NUMBER>` instead.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #22035:
URL: https://github.com/apache/beam/pull/22035#discussion_r906368058


##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteExceptionHandlingTest.java:
##########
@@ -73,34 +75,36 @@ public static Collection<Object[]> data() {
     return Arrays.asList(
         new Object[][] {
           // DEADLINE_EXCEEDED is the only exception type that generates retries in-SDK
-          // The default backoff generates 9 retries, and then errors out the pipeline.
-          {ErrorCode.DEADLINE_EXCEEDED, "deadline passed!", 9, 10},
+          // The default backoff generates at least 8 retries, but can sometimes generate as many as 10 retries due to

Review Comment:
   I mean I agree that "generates > 1 attempt " and the current form is good to me. spotless is the check (here)[https://ci-beam.apache.org/job/beam_PreCommit_Spotless_Commit/21571/] and could be resolved by simply run `./gradlew :sdks:java:io:google-cloud-platform:spotlessApply` as hinted by the log there. Could push a commit to resolve this. btw this reminds me that may be we should config the repo such that automatically run this whenever do a push



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #22035:
URL: https://github.com/apache/beam/pull/22035#discussion_r906368058


##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteExceptionHandlingTest.java:
##########
@@ -73,34 +75,36 @@ public static Collection<Object[]> data() {
     return Arrays.asList(
         new Object[][] {
           // DEADLINE_EXCEEDED is the only exception type that generates retries in-SDK
-          // The default backoff generates 9 retries, and then errors out the pipeline.
-          {ErrorCode.DEADLINE_EXCEEDED, "deadline passed!", 9, 10},
+          // The default backoff generates at least 8 retries, but can sometimes generate as many as 10 retries due to

Review Comment:
   I agree what we intend to test is that "generates > 1 attempt " and the current form is good to me. 
   spotless is the check (here)[https://ci-beam.apache.org/job/beam_PreCommit_Spotless_Commit/21571/] and could be resolved by simply run `./gradlew :sdks:java:io:google-cloud-platform:spotlessApply` as hinted by the log there. Could push a commit to resolve this. 
   btw this reminds me that may be we should config the repo such that automatically run this whenever do a push



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] pabloem merged pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
pabloem merged PR #22035:
URL: https://github.com/apache/beam/pull/22035


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] byronellis commented on a diff in pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
byronellis commented on code in PR #22035:
URL: https://github.com/apache/beam/pull/22035#discussion_r906370585


##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteExceptionHandlingTest.java:
##########
@@ -73,34 +75,36 @@ public static Collection<Object[]> data() {
     return Arrays.asList(
         new Object[][] {
           // DEADLINE_EXCEEDED is the only exception type that generates retries in-SDK
-          // The default backoff generates 9 retries, and then errors out the pipeline.
-          {ErrorCode.DEADLINE_EXCEEDED, "deadline passed!", 9, 10},
+          // The default backoff generates at least 8 retries, but can sometimes generate as many as 10 retries due to

Review Comment:
   Great, thanks will do!
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] asf-ci commented on pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #22035:
URL: https://github.com/apache/beam/pull/22035#issuecomment-1165156532

   Can one of the admins verify this patch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] pabloem closed pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
pabloem closed pull request #22035: Fix DEADLINE_EXCEEDED flakiness 
URL: https://github.com/apache/beam/pull/22035


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] byronellis commented on a diff in pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
byronellis commented on code in PR #22035:
URL: https://github.com/apache/beam/pull/22035#discussion_r906141440


##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteExceptionHandlingTest.java:
##########
@@ -73,34 +75,36 @@ public static Collection<Object[]> data() {
     return Arrays.asList(
         new Object[][] {
           // DEADLINE_EXCEEDED is the only exception type that generates retries in-SDK
-          // The default backoff generates 9 retries, and then errors out the pipeline.
-          {ErrorCode.DEADLINE_EXCEEDED, "deadline passed!", 9, 10},
+          // The default backoff generates at least 8 retries, but can sometimes generate as many as 10 retries due to

Review Comment:
   From the test description I'm not sure we care about anything other than DEADLINE_EXCEEDED generates > 1 attempt since the actual number of attempts depends on the settings of the backoff algorithm which could change over time. 
   
   TBH it seems to me that the thing we really want to test is that the number of retries is one less than the number of actual calls at which point the whole approach changes



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] asf-ci commented on pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #22035:
URL: https://github.com/apache/beam/pull/22035#issuecomment-1165156533

   Can one of the admins verify this patch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] byronellis commented on a diff in pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
byronellis commented on code in PR #22035:
URL: https://github.com/apache/beam/pull/22035#discussion_r906364337


##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteExceptionHandlingTest.java:
##########
@@ -73,34 +75,36 @@ public static Collection<Object[]> data() {
     return Arrays.asList(
         new Object[][] {
           // DEADLINE_EXCEEDED is the only exception type that generates retries in-SDK
-          // The default backoff generates 9 retries, and then errors out the pipeline.
-          {ErrorCode.DEADLINE_EXCEEDED, "deadline passed!", 9, 10},
+          // The default backoff generates at least 8 retries, but can sometimes generate as many as 10 retries due to

Review Comment:
   I'm not sure what the spotless is (it's my second day ;-)) but happy to fix if you can point me in the right direction.
   
   From your comment it also sounds like we're amenable to fixing this test up to really test that there are retries+1 calls in general with a check that retries happen when they are expected rather than fixing on specific numbers of retries? That should be a pretty straightforward change I think (well, the +1 thing might take a little work)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] pabloem commented on pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #22035:
URL: https://github.com/apache/beam/pull/22035#issuecomment-1165959009

   closing and reopening to hopefully pick up the latest change from master fixnig checkstyle


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #22035:
URL: https://github.com/apache/beam/pull/22035#issuecomment-1165969193

   > closing and reopening to hopefully pick up the latest change from master fixnig checkstyle
   
   Unfortunately checkStyle warning appears in master again due to changes of #17641. Precommit would not pass at this point.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #22035:
URL: https://github.com/apache/beam/pull/22035#discussion_r906118379


##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteExceptionHandlingTest.java:
##########
@@ -73,34 +75,36 @@ public static Collection<Object[]> data() {
     return Arrays.asList(
         new Object[][] {
           // DEADLINE_EXCEEDED is the only exception type that generates retries in-SDK
-          // The default backoff generates 9 retries, and then errors out the pipeline.
-          {ErrorCode.DEADLINE_EXCEEDED, "deadline passed!", 9, 10},
+          // The default backoff generates at least 8 retries, but can sometimes generate as many as 10 retries due to

Review Comment:
   Do we want to test it retries at most 10 times?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] asf-ci commented on pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #22035:
URL: https://github.com/apache/beam/pull/22035#issuecomment-1165156527

   Can one of the admins verify this patch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] asf-ci commented on pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #22035:
URL: https://github.com/apache/beam/pull/22035#issuecomment-1165156525

   Can one of the admins verify this patch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] asf-ci commented on pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #22035:
URL: https://github.com/apache/beam/pull/22035#issuecomment-1165156528

   Can one of the admins verify this patch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] Abacn commented on a diff in pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #22035:
URL: https://github.com/apache/beam/pull/22035#discussion_r906143686


##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteExceptionHandlingTest.java:
##########
@@ -73,34 +75,36 @@ public static Collection<Object[]> data() {
     return Arrays.asList(
         new Object[][] {
           // DEADLINE_EXCEEDED is the only exception type that generates retries in-SDK
-          // The default backoff generates 9 retries, and then errors out the pipeline.
-          {ErrorCode.DEADLINE_EXCEEDED, "deadline passed!", 9, 10},
+          // The default backoff generates at least 8 retries, but can sometimes generate as many as 10 retries due to

Review Comment:
   > the actual number of attempts depends on the settings of the backoff algorithm which could change over time.
   
   Totally agree. We've exactly bitten by this here. Would you mind fixing the spotless. I will drop my second commit in my PR that duplicates this.
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] pabloem commented on pull request #22035: Fix DEADLINE_EXCEEDED flakiness

Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #22035:
URL: https://github.com/apache/beam/pull/22035#issuecomment-1165995338

   alright fine I'll merge as is thanks y'all LGTM


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org