pubsub: TestIntegration_OrderedKeys_JSON failed #3626
Comments
flaky-bot
bot
added
flakybot: issue
|
Looks like this issue is flaky. 😟 I'm going to leave this open and stop commenting. A human should fix and close this. When run at the same commit (98637e0), this test passed in one build (Build Status, Sponge) and failed in another build (Build Status, Sponge). |
flaky-bot
bot
added
the
flakybot: flaky
yoshi-automation
added
🚨
hongalex
added
priority: p2
yoshi-automation
added
🚨
From discussion with @kamalaboulhosn, increasing the test timeout for this ordering key test should help with flakiness. Fixes #3626
|
Oops! Looks like this issue is still flaky. It failed again. 😬 I reopened the issue, but a human will need to close it again. commit: 5996846 Test output==================
WARNING: DATA RACE
Write at 0x00c000b32148 by goroutine 215:
cloud.google.com/go/pubsub.(*Subscription).checkOrdering()
/tmpfs/src/google-cloud-go/pubsub/subscription.go:968 +0x150
cloud.google.com/go/pubsub.(*Subscription).checkOrdering-fm()
/tmpfs/src/google-cloud-go/pubsub/subscription.go:918 +0x41
sync.(*Once).Do()
/usr/local/go/src/sync/once.go:44 +0xde
cloud.google.com/go/pubsub.(*Subscription).Receive.func2()
/tmpfs/src/google-cloud-go/pubsub/subscription.go:918 +0x674
golang.org/x/sync/errgroup.(*Group).Go.func1()
/go/pkg/mod/golang.org/x/sync@v0.0.0-20210220032951-036812b2e83c/errgroup/errgroup.go:57 +0x64
|
We use `sync.once.Do(s.checkOrdering)` to perform one config check for a subscription's message ordering field when a message with an ordering key is received. This way, we know if we should deliver messages in order. However, since `sub.Receive` generates multiple goroutines, `s.enableOrdering` is not guaranteed to be read after the first invocation of `s.checkOrdering`. This PR makes this race happen less often by making the config check closer to when the message is received. This cannot be fully eliminated without making the config call at the start of every single `Receive`. This behavior is less desirable, as we want to make the config call only when we have to, that is when we receive a message with an ordering key. Fixes #3626
|
Oops! Looks like this issue is still flaky. It failed again. 😬 I reopened the issue, but a human will need to close it again. commit: a38d037 Test output==================
WARNING: DATA RACE
Write at 0x00c000364538 by goroutine 207:
cloud.google.com/go/pubsub.(*Subscription).checkOrdering()
/tmpfs/src/google-cloud-go/pubsub/subscription.go:968 +0x150
cloud.google.com/go/pubsub.(*Subscription).checkOrdering-fm()
/tmpfs/src/google-cloud-go/pubsub/subscription.go:901 +0x41
sync.(*Once).Do()
/usr/local/go/src/sync/once.go:44 +0xde
cloud.google.com/go/pubsub.(*Subscription).Receive.func2()
/tmpfs/src/google-cloud-go/pubsub/subscription.go:901 +0x79d
golang.org/x/sync/errgroup.(*Group).Go.func1()
/go/pkg/mod/golang.org/x/sync@v0.0.0-20210220032951-036812b2e83c/errgroup/errgroup.go:57 +0x64
|
In the previous iteration of this PR, #4602, the assumption was made that making the config check earlier will prevent race conditions in the test, which turned out to be incorrect. In addition, although previously ruled out as a solution, this PR makes it so that a config check is always made on the first call to `Receive`. I originally thought this would result in a poor experience for those users who don't have the `roles/pubsub.viewer` permission, but we default `enableOrdering` to `true` anyway if the config call fails due to lack of permissions. Using a default of `true` only negatively impacts those who don't want ordering on their subscription, and this behavior is [already documented](https://github.com/googleapis/google-cloud-go/blob/pubsub/v1.14.0/pubsub/subscription.go#L235-L239). Fixes #3626
🤖 I have created a release \*beep\* \*boop\* --- ## [1.15.0](https://www.github.com/googleapis/google-cloud-go/compare/pubsub/v1.14.0...pubsub/v1.15.0) (2021-08-13) ### Features * **pubsub:** Add topic retention options ([5996846](https://www.github.com/googleapis/google-cloud-go/commit/59968462a3870c6289166fa1161f9b6d9c10e093)) ### Bug Fixes * **pubsub:** always make config check to prevent race ([#4606](https://www.github.com/googleapis/google-cloud-go/issues/4606)) ([8cfcf53](https://www.github.com/googleapis/google-cloud-go/commit/8cfcf53d03b9b442e7f0bc1c1b20c791e31c07b0)), refs [#3626](https://www.github.com/googleapis/google-cloud-go/issues/3626) * **pubsub:** mitigate race in checking ordering config ([#4602](https://www.github.com/googleapis/google-cloud-go/issues/4602)) ([112eea2](https://www.github.com/googleapis/google-cloud-go/commit/112eea20b46bbc34e5f8f65b9812fb3e60107409)), refs [#3626](https://www.github.com/googleapis/google-cloud-go/issues/3626) * **pubsub:** replace IAMPolicy in API config ([5996846](https://www.github.com/googleapis/google-cloud-go/commit/59968462a3870c6289166fa1161f9b6d9c10e093)) This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This test failed!
To configure my behavior, see the Flaky Bot documentation.
If I'm commenting on this issue too often, add the
flakybot: quietlabel andI will stop commenting.
commit: 98637e0
buildURL: Build Status, Sponge
status: failed
Test output
The text was updated successfully, but these errors were encountered: