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

[BUG] [Servicebus|Azure.Messaging.ServiceBus] CreateMessageBatchAsync does not calculate the size correctly #18038

Closed
reza-esfandyari opened this issue Jan 19, 2021 · 5 comments · Fixed by #18062
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. needs-team-attention Workflow: 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

Comments

@reza-esfandyari
Copy link

reza-esfandyari commented Jan 19, 2021

Here is the sample code:
I am not sure if the method is not calculating the size correctly or there is some mismatch between the way that CreateMessageBatchAsync and SendMessagesAsync validate the size

            for (var i = 0; i < integrationEvents.Length; i++)
            {
                if (!messageBatch.TryAddMessage((GenerateMessage(integrationEvents[i]))))
                {
                    try
                    {
                        sender.SendMessagesAsync(messageBatch).GetAwaiter().GetResult();
                    }
                    catch (Exception e)
                    {
                        _logger.LogError(e,
                            "ServiceBus - Failed to send the message");
                    }
                    
                    geBatch.Dispose();
                    messageBatch = sender.CreateMessageBatchAsync().GetAwaiter().GetResult();

                    if (!messageBatch.TryAddMessage((GenerateMessage(integrationEvents[i]))))
                    {
                        Console.WriteLine($"Failed to fit message number in a batch {i}");
                        break;
                    }
                }

            }

            try
            {
                sender.SendMessagesAsync(messageBatch).GetAwaiter().GetResult();
                messageBatch.Dispose();
            }
            catch (Exception e)
            {
                _logger.LogError(e,
                    "ServiceBus - Failed to send the message");
            }

image

image

Azure.Messaging.ServiceBus.ServiceBusException: The message (id:56329192, size:293920 bytes) is larger than is currently allowed (262144 bytes). (MessageSizeExceeded)
   at Azure.Messaging.ServiceBus.Amqp.AmqpSender.SendBatchInternalAsync(Func`1 messageFactory, TimeSpan timeout, CancellationToken cancellationToken)
   at Azure.Messaging.ServiceBus.Amqp.AmqpSender.SendBatchInternalAsync(Func`1 messageFactory, TimeSpan timeout, CancellationToken cancellationToken)
   at Azure.Messaging.ServiceBus.Amqp.AmqpSender.<>c__DisplayClass18_0.<<SendBatchAsync>b__1>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at Azure.Messaging.ServiceBus.ServiceBusRetryPolicy.RunOperation(Func`2 operation, TransportConnectionScope scope, CancellationToken cancellationToken)
   at Azure.Messaging.ServiceBus.ServiceBusRetryPolicy.RunOperation(Func`2 operation, TransportConnectionScope scope, CancellationToken cancellationToken)
   at Azure.Messaging.ServiceBus.Amqp.AmqpSender.SendBatchAsync(ServiceBusMessageBatch messageBatch, CancellationToken cancellationToken)
   at Azure.Messaging.ServiceBus.ServiceBusSender.SendMessagesAsync(ServiceBusMessageBatch messageBatch, CancellationToken cancellationToken)
   at Adia.Core.IntegrationService.ServiceBus.EventBusServiceBus.Publish(IEnumerable`1 events, QueueOrTopicName queueOrTopicName) in D:\repos\Adia-Service-Bulk-Timesheet-Validation\Source\Adia.Core\IntegrationService\ServiceBus\EventBusServiceBus.cs:line 136

there is not much in message custom property but I assume when the batch calculate the size it should consider it too
image

@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 Jan 19, 2021
@JoshLove-msft
Copy link
Member

@reza-esfandyari, thanks for submitting the issue. Any chance you can include the logic behind your GenerateMessage method? I haven't been able to repro the issue when setting a message body and application properties.

@reza-esfandyari
Copy link
Author

reza-esfandyari commented Jan 19, 2021

Hi @JoshLove-msft, Thanks for getting back to me quickly, here is the content

 private static ServiceBusMessage GenerateMessage(IntegrationEvent @event)
        {
            var message = new ServiceBusMessage
            {
                MessageId = Guid.NewGuid().ToString(),
                Body = new BinaryData(JsonConvert.SerializeObject(@event)),
                ApplicationProperties =
                {
                    {"EventName", @event.GetType().Name.Replace(IntegrationEventSuffix, "")},
                    {"ServiceName", "TODO"}
                }
            };
            return message;
        }

I include my sample payload structure and hope it help

 public class IntegrationEvent
    {
        public IntegrationEvent()
        {
            Id = Guid.NewGuid();
            CreationDate = DateTime.UtcNow;
            Version = 1;
        }

        [JsonConstructor]
        public IntegrationEvent(Guid id, DateTime createDate)
        {
            Id = id;
            CreationDate = createDate;
        }

        [JsonProperty] public Guid Id { get; private set; }

        [JsonProperty] public DateTime CreationDate { get; private set; }

        [JsonProperty] public int Version { get; private set; }
    }

 public class CandidateCreatedIntegrationEvent : IntegrationEvent
    {
        public int CandidateId { get; set; }
        public string FirstName { get; set; }
        public string LastName { get; set; }
        public DateTime BirthDate { get; set; }
        public byte[] RowVersion { get; set; }
    }


 var sampleIntegrationMessages = new Collection<IntegrationEvent>();
            for (var i = 0; i < 1000; i++)
            {
                var sampleIntegrationMsg1 = new CandidateCreatedIntegrationEvent
                {
                    FirstName = "Reza" + i,
                    CandidateId = i,
                    BirthDate = DateTime.Now,
                    LastName = "Esfandyari" + i
                };

                sampleIntegrationMessages.Add(sampleIntegrationMsg1);
            }
       
          _eventBus.Publish(sampleIntegrationMsg);

@jsquire jsquire added Client This issue points to a problem in the data-plane of the library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Service Bus labels Jan 19, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jan 19, 2021
@JoshLove-msft
Copy link
Member

It looks like the issue is that we are not reserving space for properties that go on the batch envelope, namely the messageId, sessionId, and partitionKey from the first message in the batch. Thanks @jsquire for pointing this out and thank you @reza-esfandyari for submitting the issue! We will get this patched up.

@JoshLove-msft JoshLove-msft added this to the [2021] February milestone Jan 19, 2021
@reza-esfandyari
Copy link
Author

Thanks, guys.
any estimate on when I can have the fix @JoshLove-msft? Are you going to update this issue ticket?

@JoshLove-msft
Copy link
Member

Thanks, guys.
any estimate on when I can have the fix @JoshLove-msft? Are you going to update this issue ticket?

Yep, it should be available in our next release which is slated for the second week of Feb.

openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-net that referenced this issue Mar 22, 2022
Review request for Microsoft.ContainerService to add version 2022-02-01 (Azure#18324)

* Adds base for updating Microsoft.ContainerService from version stable/2022-01-01 to version 2022-02-01

* Updates readme

* Updates API version in new specs and examples

* Update readmes for the 2022-02-01 dev branch of container service (Azure#17887)

* update readme

* remove useless -only tags

* Fix violated rule R2026 for 2022-02-01 managedCluster swagger (Azure#18024)

* fix allof

* add missing type

* GA alias minor version (Azure#18038)

* GA alias minor version

* polish more details on kubernetesVersion and currentKubernetesVersion

* Update specification/containerservice/resource-manager/Microsoft.ContainerService/stable/2022-02-01/managedClusters.json

minor revision

Co-authored-by: Matthew Christopher <[email protected]>

Co-authored-by: Matthew Christopher <[email protected]>

* add missing type object (Azure#18115)

* Revert "GA alias minor version (Azure#18038)" (Azure#18291)

This reverts commit 3a99cda3cb48e05c74923f5467c0737014322b0f.

* Align modifications of several common definitions with 2022-02-02-preview for container service 2022-02-01 (Azure#18216)

* sync modification

* fix ref

Co-authored-by: Jianping Zeng <[email protected]>
Co-authored-by: Matthew Christopher <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 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. needs-team-attention Workflow: 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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants