Skip to content

Conversation

@feywind
Copy link
Collaborator

@feywind feywind commented Mar 21, 2022

As the title says, this sets a default value for grpc.max_metadata_size.

While fixing that, I discovered that we are incorrectly raising a client error for ack/modack failures. Rather than making these go silent for users (for debug purposes), I moved them to a 'debug' channel that can also be subscribed on Subscription instances.

Fixes #1504 🦕

@feywind feywind added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Mar 21, 2022
@feywind feywind requested review from a team as code owners March 21, 2022 22:06
@product-auto-label product-auto-label bot added the api: pubsub Issues related to the googleapis/nodejs-pubsub API. label Mar 21, 2022
Copy link

@pradn pradn left a comment

Choose a reason for hiding this comment

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

Looks good. Please test:

  1. subscribing for a long time from a exactly-once delivery-enabled subscription
  2. subscribing from a regular subscription and turning on exactly-once for the subscription

In both cases, the subscribing should continue as normal.

@feywind feywind added the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 24, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 24, 2022
@feywind feywind added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 1, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 1, 2022
@feywind feywind removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 1, 2022
@feywind feywind changed the title fix: update grpc.max_metadata_size to 4MiB for exactly-once fix: update grpc.max_metadata_size to 4MiB for exactly-once, and shift ack/modack errors to 'debug' stream channel Apr 5, 2022
@feywind
Copy link
Collaborator Author

feywind commented Apr 5, 2022

Looks good. Please test:

  1. subscribing for a long time from a exactly-once delivery-enabled subscription
  2. subscribing from a regular subscription and turning on exactly-once for the subscription

In both cases, the subscribing should continue as normal.

These are tested and pass after the fix for the ack/modack errors.

Copy link

@bcoe bcoe left a comment

Choose a reason for hiding this comment

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

Left one nit that might be worth fixing.

@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Apr 5, 2022
@feywind feywind added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 6, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 6, 2022
@feywind feywind added the automerge Merge the pull request once unit tests and other checks pass. label Apr 6, 2022
@feywind feywind merged commit abd10cc into googleapis:main Apr 6, 2022
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Apr 6, 2022
@feywind feywind deleted the gh1504-grpc-metadata branch April 6, 2022 19:56
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 googleapis/nodejs-pubsub API. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Increase "grpc.max_metadata_size" GRPC parameter to 4 MB

3 participants