Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(pubsub): respect subscription message ordering field in scheduler #3886

Merged
merged 7 commits into from Apr 5, 2021

Conversation

@hongalex
Copy link
Member

@hongalex hongalex commented Apr 3, 2021

Fixes #3884

@hongalex hongalex requested a review from kamalaboulhosn Apr 3, 2021
codyoss
codyoss approved these changes Apr 5, 2021
Copy link
Member

@codyoss codyoss left a comment

Just one small thing, LGTM

Loading

pubsub/subscription.go Outdated Show resolved Hide resolved
Loading
pubsub/integration_test.go Outdated Show resolved Hide resolved
Loading
@indera-shsp
Copy link

@indera-shsp indera-shsp commented Apr 5, 2021

@hongalex
Thank you for this fix.

Loading

if err != nil {
return fmt.Errorf("sub.Config err: %v", err)
}
s.enableOrdering = cfg.EnableMessageOrdering

Choose a reason for hiding this comment

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

Loading

Copy link
Member Author

@hongalex hongalex Apr 5, 2021

Choose a reason for hiding this comment

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

This page does not explicitly call out why we're setting EnableMessageOrdering, aside from the code sample, so I'm inclined to leave this out.

Loading

Choose a reason for hiding this comment

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

👍

My comment was more like - look there is an example where this property is mentioned.

This was a good read too:
https://medium.com/google-cloud/google-cloud-pub-sub-ordered-delivery-1e4181f60bc8

These two properties allow publishers and subscribers to decide independently if messages are ordered. If the publisher does not specify ordering keys with messages or the subscriber does not enable ordered delivery, then message delivery is not in order and behaves just like Cloud Pub/Sub without the ordered delivery feature. Not all subscriptions on a topic need to have the same setting for enable_message_ordering. Therefore, different use cases that receive the same messages can determine if they need ordered delivery without impacting each other.

Loading

@hongalex hongalex merged commit 1fcc78a into googleapis:master Apr 5, 2021
3 checks passed
Loading
@hongalex hongalex deleted the subscription-ordering-fix branch Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants