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

Getting error from service bus on sending message of size greater then 1 MB for premium tier. #22962

Closed
3 of 6 tasks
sabharwal-garv opened this issue Aug 19, 2022 · 7 comments
Closed
3 of 6 tasks
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. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Bus
Milestone

Comments

@sabharwal-garv
Copy link

sabharwal-garv commented Aug 19, 2022

  • Package Name: azsdk-js-azureservicebus
  • Package Version: 7.7.1
  • Operating system: Mac OS BigSur version 11.2.3
  • nodejs
    • version: 12.16.3
  • browser
    • name/version:
  • typescript
    • version: 4.6.3
  • Is the bug related to documentation in

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Steps to reproduce the behavior:

  1. Deploy a Premium SB namespace and topic (with maxMessageSizeInKB = 102400).
  2. create a function app and replace code in index file with following code

import { AzureFunction, Context, HttpRequest } from '@azure/functions'; import { ServiceBusMessage, ServiceBusClient } from '@azure/service-bus'; import * as config from './function.json'; const httpTrigger: AzureFunction = async function (context: Context, req: HttpRequest): Promise<void> { const serviceBusMessage: ServiceBusMessage = { body: req.body } as ServiceBusMessage try { const serviceBusClient = new ServiceBusClient("set this value accordingly"); const sender = serviceBusClient.createSender("set this value accordingly"); await sender.sendMessages(serviceBusMessage); } catch (error) { context.log.error(error SendMessage${error}); throw error; } }; export default httpTrigger;

Expected behavior
sender.sendMessages should not throw error.

Error Body

InvalidOperationError: The link 'G4S2:3993911:hybris.syndication-product.cds-sbt-2ce21e98-e39b-0b4b-a421-48f12923638f' is force detached by the broker because publisher(link9441) received a batch message with no data in it. Detach origin: Publisher.

Additional context
as per the document large-messages-support Batching is not supported for Large Messages and as per the sendMessage code https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/servicebus/service-bus/src/sender.ts#L193-L223, inside else case we are creating batch from message due to which we are getting this error, on my local machine I replaced it with following code :

async sendMessages(
messages:
| ServiceBusMessage
| ServiceBusMessage[]
| ServiceBusMessageBatch
| AmqpAnnotatedMessage
| AmqpAnnotatedMessage[],
options?: OperationOptionsBase
): Promise {
this._throwIfSenderOrConnectionClosed()
throwTypeErrorIfParameterMissing(this._context.connectionId, "messages", messages)
let batch: ServiceBusMessageBatch;
if (isServiceBusMessageBatch(messages)) {
batch = messages;
const spanLinks: TracingSpanLink[] = batch._messageSpanContexts.map((tracingContext) => {
return {
tracingContext,
};
})
return tracingClient.withSpan(
"ServiceBusSender.send",
options ?? {},
(updatedOptions) => this._sender.sendBatch(batch, updatedOptions),
{
spanLinks,
...toSpanOptions(
{ entityPath: this.entityPath, host: this._context.config.host },
"client"
),
}
);
} else {
if (!Array.isArray(messages)) {
messages = [messages];
}
const sendMessagesPromise = messages.map((message) => {
return tracingClient.withSpan(
"ServiceBusSender.send",
options ?? {},
(updatedOptions) => this._sender.send(message, updatedOptions),
{
...toSpanOptions(
{ entityPath: this.entityPath, host: this._context.config.host },
"client"
),
}
);
});
await Promise.all(sendMessagesPromise)
}
}

it worked, request you to please look into this.

Dummy data for issue verification

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Aug 19, 2022
@azure-sdk azure-sdk added Client This issue points to a problem in the data-plane of the library. needs-team-triage Workflow: This issue needs the team to triage. Service Bus labels Aug 19, 2022
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 19, 2022
@xirzec xirzec removed the needs-team-triage Workflow: This issue needs the team to triage. label Aug 19, 2022
@HarshaNalluru
Copy link
Member

Thanks for reaching out @sabharwal-garv and for your investigations.
I'll look into this.

@HarshaNalluru
Copy link
Member

HarshaNalluru commented Aug 23, 2022

Hey @sabharwal-garv, I have investigated the issue, and am acknowledging the problem.
I will reach out to the service-bus service team on "how to manage the message batches better" and probably throw errors while batching large messages(>1 MB) like your case, or whatever would avoid the InvalidOperationError: The link 'G4S2.... error.

Your Suggested Fix

Coming to the fix that you are considering... using Promise.all and resolving many promises at once in the SDK will not give a nice/simple/intuitive experience for the customers when a few of the send calls fail.
We want to keep the SDK as simple as possible, one typical client call should make one request to the service for instance (ignoring the retry or equivalent logics).
If not for this errors experience, I like your fix.

Workaround

I would instead recommend you a workaround on similar lines.
Instead of touching the SDK src code, apply the same logic in your application..
Call sender.sendMessages() per message and resolve them with Promise.all.

// WORKAROUND

//... messages is an array with messages of size > 1MB
const sendMessagesPromises = messages.map((message) => { return sender.sendMessages(message); });
await Promise.all(sendMessagesPromises);

I hope this workaround helps your case.

Future Steps

Meanwhile, I will contact the service team on the

  • recommendation for the large messages such as >1MB?
  • at what message size, we should say no to batching?
  • could there be a better error message in this case?
  • should we throw an error instead and let users follow the workaround like we are discussing here?
    Based on the discussions with the service team, I'll(we'll) work on updating the SDK code on the sendMessages API.

I really appreciate your efforts in digging around investigating the issue and suggesting a fix. Will keep this issue open and keep you posted on what the service team recommends.

But the workaround I suggested would help you get unblocked asap.

@sabharwal-garv
Copy link
Author

Hi @HarshaNalluru , Thank you for your response, The workaround suggested by you will not fix our issue when single message size is of greater then 1 MB, till the message size limit for batch is increased or else case inside SDK code on the sendMessages API is updated, as it is mentioned in issue description as per the documentation large-messages-support batching is not supported for large messages.

Issue with Workaround

Please refer to the code:

same code attached in the issue description to reproduce the issue.

import { AzureFunction, Context, HttpRequest } from '@azure/functions'; import { ServiceBusMessage, ServiceBusClient } from '@azure/service-bus'; import * as config from './function.json'; const httpTrigger: AzureFunction = async function (context: Context, req: HttpRequest): Promise<void> { const serviceBusMessage: ServiceBusMessage = { body: req.body } as ServiceBusMessage try { const serviceBusClient = new ServiceBusClient("set this value accordingly"); const sender = serviceBusClient.createSender("set this value accordingly"); await sender.sendMessages(serviceBusMessage); } catch (error) { context.log.error(error SendMessage${error}); throw error; } }; export default httpTrigger;

as you can see in the code we are trying to send single message of size greater then 1 MB not array of messages still we are getting error, it is due to the else case of SDK code for sendMessages API function

} else {
if (!Array.isArray(messages)) {
messages = [messages];
}
batch = await this.createMessageBatch(options);
for (const message of messages) {
throwIfNotValidServiceBusMessage(message, errorInvalidMessageTypeSingleOrArray);
if (!batch.tryAddMessage(message, options)) {
// this is too big - throw an error
throw new ServiceBusError(
"Messages were too big to fit in a single batch. Remove some messages and try again or create your own batch using createBatch(), which gives more fine-grained control.",
"MessageSizeExceeded"
);
}
}
}
, as inside else block every message is getting converted into batch of messages, and as mentioned in the documentation batching is not supported for large message hence it is throwing error.

Request you to please suggest a different workaround if any as we are still blocked.

@HarshaNalluru
Copy link
Member

HarshaNalluru commented Aug 26, 2022

Hey @sabharwal-garv,

Thanks for the message.

After talking to others in the feature crew for service-bus SDKs, we decided to go with "allow sending a single message without creating a batch for it", PR created for it #23014. Feel free to provide feedback.

While we have this solution, we are also going to ask the service team for new properties on the link to manage batching of messages better in the sendMessages method. I'll post more on this if there is progress on these discussions.

But, for now, I believe this #23014 should work for you.
I'll provide you with a dev package version for you to test the fix before checking in the PR.

@sabharwal-garv
Copy link
Author

sabharwal-garv commented Aug 26, 2022

Hi @HarshaNalluru ,
Thank you for the response.

I think this solution will work and will resolve our issue and I have gone through the code, and added one suggestions on the PR if you think that will not be helpful please mark that as resolve

@HarshaNalluru
Copy link
Member

Yes, @sabharwal-garv. I'll be updating the function description.

@HarshaNalluru HarshaNalluru added this to the 2022-10 milestone Sep 7, 2022
@HarshaNalluru
Copy link
Member

@sabharwal-garv, the fix has been published with the @azure/[email protected] version, please pull down the latest version and let us know if the fix works for you.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Bus
Projects
None yet
Development

No branches or pull requests

4 participants