Skip to content

Commit

Permalink
fix(pubsub): always make config check to prevent race (#4606)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
hongalex committed Aug 13, 2021
1 parent 27fc784 commit 8cfcf53
Showing 1 changed file with 2 additions and 4 deletions.
6 changes: 2 additions & 4 deletions pubsub/subscription.go
Expand Up @@ -48,7 +48,6 @@ type Subscription struct {
mu sync.Mutex
receiveActive bool

once sync.Once
enableOrdering bool
}

Expand Down Expand Up @@ -785,6 +784,8 @@ func (s *Subscription) Receive(ctx context.Context, f func(context.Context, *Mes
s.mu.Unlock()
defer func() { s.mu.Lock(); s.receiveActive = false; s.mu.Unlock() }()

s.checkOrdering()

maxCount := s.ReceiveSettings.MaxOutstandingMessages
if maxCount == 0 {
maxCount = DefaultReceiveSettings.MaxOutstandingMessages
Expand Down Expand Up @@ -897,9 +898,6 @@ func (s *Subscription) Receive(ctx context.Context, f func(context.Context, *Mes
}
for i, msg := range msgs {
msg := msg
if msg.OrderingKey != "" {
s.once.Do(s.checkOrdering)
}
// TODO(jba): call acquire closer to when the message is allocated.
if err := fc.acquire(ctx, len(msg.Data)); err != nil {
// TODO(jba): test that these "orphaned" messages are nacked immediately when ctx is done.
Expand Down

0 comments on commit 8cfcf53

Please sign in to comment.