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

samples(pubsub): Add publisher flow control sample #12005

Merged

Conversation

quartzmo
Copy link
Member

@quartzmo quartzmo requested a review from a team as a code owner June 17, 2021 19:45
@snippet-bot
Copy link

snippet-bot bot commented Jun 17, 2021

Here is the summary of changes.

You are about to add 1 region tag.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jun 17, 2021
@product-auto-label product-auto-label bot added api: pubsub Issues related to the Pub/Sub API. samples Issues that are directly related to samples. labels Jun 17, 2021
@quartzmo quartzmo self-assigned this Jun 17, 2021
@anguillanneuf
Copy link

The delete schema rpc is flaky and is a known issue.

@@ -290,6 +290,37 @@ def publish_messages_async_with_concurrency_control topic_id:
# [END pubsub_publisher_concurrency_control]
end

def publish_messages_async_with_flow_control topic_id:
# [START pubsub_publisher_flow_control]
# topic_id = "your-topic-id"
Copy link
Member

Choose a reason for hiding this comment

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

I would put this line a bit further down, just above the topic = ... line where it is used, because that's where we'd expect to see it in actual code.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the style throughout these samples, so it will be best not to change it in this PR.

Copy link
Member

Choose a reason for hiding this comment

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

grrr... well, I guess I'll open an issue.

google-cloud-pubsub/samples/topics.rb Outdated Show resolved Hide resolved

topic = pubsub.topic topic_id, async: {
# Configure how many messages the publisher client can hold in memory
# before publishing succeeds, and what to do when messages exceed the limit.
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean by "before publishing succeeds?"

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed, thanks!

limit_exceeded_behavior: :block
}
}
# Publish 1000 messages in quick succession to trigger publisher flow control.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't say "to trigger flow control." It's possible that flow control will be triggered, but it's also entirely possible it won't be if messages get out quickly enough. Either way, the fact that flow control was triggered isn't something that will be easily visible to the user. Maybe say "Rapidly publishing 1000 messages in a loop may be constrained by flow control."

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

@quartzmo
Copy link
Member Author

CI error is delete_schema flake.

@quartzmo quartzmo merged commit 774d430 into googleapis:master Jun 18, 2021
@quartzmo quartzmo deleted the pubsub-publisher-flow-control-sample branch June 18, 2021 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the Pub/Sub API. cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pub/Sub: add publisher flow control sample
4 participants