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

[Feature Req] ServiceBusMessageSender should refresh maximum message size when service settings change #43983

Open
SeanFeldman opened this issue May 10, 2024 · 4 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved. needs-team-attention This issue needs attention from Azure service team or SDK team Service Bus
Milestone

Comments

@SeanFeldman
Copy link
Contributor

Library name and version

Azure.Messaging.ServiceBus

Describe the bug

When using a ServiceBusMessageSender instance while there's a change to the entity's maximum message size, the Batch API doesn't allow message adding despite the message being within the allowed size. The client holds on to the old maximum message size it has retrieved from the entity, which is confusing.

Expected behavior

Messages that previously couldn't be sent to be sent w/o the need to restart the process.

Actual behavior

The process has to be restarted to get the updated max message size.

Reproduction Steps

On a premium namespace, a queue of size S1. Start a process that sends a message larger than S1.
Increase the entity maz size to S2 (S2 > S1). Adding messages to a batch messageBatch.TryAddMessage(message) be unsuccessful.

Environment

No response

@github-actions github-actions bot added Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Bus labels May 10, 2024
Copy link

Thank you for your feedback. Tagging and routing to the team member best able to assist.

@jsquire jsquire assigned jsquire and unassigned JoshLove-msft May 10, 2024
@jsquire jsquire changed the title [BUG] ServiceBusMessageSender doesn't refresh maximum message size, causing valid messages to fail to be sent using Batch API [Feature Req] ServiceBusMessageSender should refresh maximum message size when service settings change May 10, 2024
@jsquire
Copy link
Member

jsquire commented May 10, 2024

Hi @SeanFeldman. Thanks for letting us know. The current implementation is based on the now outdated service limitation that the maximum message size was based on the SKU and was not configurable. As the service feature set changed, this was overlooked in the client.

There's a bit of a tricky balance here. I don't think that we want to force every call to CreateMessageBatchAsync to be checking the link for the unlikely case that the message size could change. I do think that it would be reasonable to reevaluate the maximum message size on send, where we're already network bound. This would mean that the client won't refresh in real-time but would adjust before each send operation. I think that's a reasonable trade-off to avoid impacting performance each time a batch is created, but I'd appreciate your thoughts.

Regardless of how the client adjusts, we'd also need a service-side feature for this to work. Currently, the entity does not detatch existing AMQP connections/links when settings change. Since the maximum message size is communicated to the client via metadata on the link, for the client to be aware of changes, it needs to open a new link on which the service has set that updated metadata. I've opened #705 for the Service Bus team.

In the meantime, the proposed client changes are safe to make, so we'll look at getting them in.

@jsquire jsquire added feature-request This issue requires a new behavior in the product in order be resolved. and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that needs-team-attention This issue needs attention from Azure service team or SDK team labels May 10, 2024
@jsquire jsquire added this to the 2024-06 milestone May 10, 2024
@github-actions github-actions bot added the needs-team-attention This issue needs attention from Azure service team or SDK team label May 10, 2024
@SeanFeldman
Copy link
Contributor Author

@jsquire, that's a good approach 👍

@jsquire
Copy link
Member

jsquire commented May 30, 2024

Client portions addressed in #44328. It should help a bit, but won't be fully effective until the service forces client disconnection when the size is updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved. needs-team-attention This issue needs attention from Azure service team or SDK team Service Bus
Projects
Status: In Progress
Development

No branches or pull requests

3 participants