Backoff timing verification can flake

[dbolduc]

googleapis/google-cloud-cpp#12913

• GCB: https://console.cloud.google.com/cloud-build/builds;region=us-east1/640f891f-3026-4ac9-8ccf-bdfb2457da34;tab=detail?project=cloud-cpp-testing-resources
• Raw: https://storage.googleapis.com/cloud-cpp-community-publiclogs/logs/google-cloud-cpp/12900/7a399f73cbb119b86e9d6902c847cfc76f1fda1b/fedora-latest-bazel-asan/log-640f891f-3026-4ac9-8ccf-bdfb2457da34.txt

=== RUN   TestMutateRows_Retry_ExponentialBackoff
    mutaterows_test.go:347: The three retry delays are: 111ms, 106ms, 353ms
    mutaterows_test.go:350: 
        	Error Trace:	/var/tmp/downloads/cloud-bigtable-clients-test/tests/mutaterows_test.go:350
        	Error:      	"111" is not less than "106"
        	Test:       	TestMutateRows_Retry_ExponentialBackoff
--- FAIL: TestMutateRows_Retry_ExponentialBackoff (0.63s)

C++ has unit tests in our code that verify that the retry policy always increases. There is jitter (randomness) involved.

I think it is very likely that the computer running the build had a hiccup during the first attempt to add an extra few ms of slowness. And the backoff duration on the second attempt happened to be very close to the backoff duration of the first attempt. Thus the proxy detects a failure.

To bandaid, we can add a few ms of tolerance to the backoff expectations. But ultimately we are at the mercy of a time based test.

[daniel-sanche]

Python also flakes on this test, because the shared gapic api-core library uses random(0, next_max_value) when calculating backoff. I opened a PR to change the backoff calculations to be in line with the tests here, but then I saw that the official grpc service_config spec says to use Python's strategy. So maybe C++ is doing something similar?

[dbolduc]

Python also flakes on this test

Oh yeah... this test was probably written against the C++ client.

We should set up the test to pass in the "full jitter" case (0, N]. And by "we" I mean "someone other than me". 👃 👈

So maybe C++ is doing something similar?

Not in bigtable¹. We do it in other libraries though.

https://github.com/googleapis/google-cloud-cpp/blob/2c26eb99458e32b22ebcb6c865cfa74b88a7ed5f/google/cloud/bigtable/internal/defaults.cc#L214-L215

For way too much context, see googleapis/google-cloud-cpp#8755

Footnotes

1. I did not update the bigtable defaults, because I think they are perfectly fine as is. Customers can set a full jitter policy if they want to. ↩

[liujiongxin]

I think to support the "full jitter" case, the test should drop the time check: different clients use different default, so it's not possible to check the absolute value in an agnostic way. I will make the change to just use logging for now.

[liujiongxin]

I made the change: eaf9477

In your presubmit, please checkout the new release v0.0.2

[dbolduc]

I made the change: eaf9477

Thanks!

In your presubmit, please checkout the new release v0.0.2

The robots did it for us.... Nice! googleapis/google-cloud-cpp#12960