Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.26k stars 4.61k forks source link

EventHubProducerClient.CreateBatchAsync() takes too much time #11018

Closed haohighview closed 4 years ago

haohighview commented 4 years ago

If I attempt to create an EventDataBatch via

await using var producerClient = new EventHubProducerClient(AppConstants.EventHunEndpoint, AppConstants.EventHunName);
using EventDataBatch eventBatch = await producerClient.CreateBatchAsync();

var trackEvent = new EventData(Encoding.UTF8.GetBytes(Newtonsoft.Json.JsonConvert.SerializeObject(new
{
    trackerType = string.IsNullOrEmpty(errorCode) ? eventName : "PlayerError",
    videoType = isLive ? 0 : 1,
    videoId = id,
    userId = AppConstants.AppSettings.UserId,
    tenancyName = AppConstants.AppTemplate.TenancyName,
    deviceType = Xamarin.Forms.Device.RuntimePlatform == Xamarin.Forms.Device.iOS ? "iOSApp" : "AndroidApp",
    fingerprint = AppConstants.DeviceService.DeviceId,
    isLive = isLiving,
    isFullscreen = isFullScreen,
    progress = progress,
    videoLength = videoLength,
    errorCode = errorCode,
    errorMessage = errorMsg
})));

eventBatch.TryAdd(trackEvent);
await producerClient.SendAsync(eventBatch);

It takes too much time to call CreateBatchAsync on Xamarin.Android app. it works great on Xamarin.iOS app. This is using Azure.Messaging.EventHubs 5.0.1 in Xamarin.Android with Microsoft Visual Studio Enterprise 2019 version 16.5.0

jsquire commented 4 years ago

Hi @haohighview. We're sorry that your experiencing difficulties and appreciate you reaching out. Based on your snippet, the latency that you're experiencing is network I/O between your EventHubProducerClient and the Event Hubs service as the connection is established.

The Event Hubs clients defer connecting to the Event Hubs service until the first time an operation that requires communication is invoked. In this case, the Event Hubs service owns the decision for the maximum allowable size of a batch, so the first call to CreateBatchAsync establishes the connection and queries the value. Subsequent calls to CreateBatchAsync use a cached value and will execute more quickly, but the first call must pay the connection tax.

The SendAsync call also benefits, as it uses the established connection to send the batch to the service. Were we to estimate the maximum allowed batch size instead of querying the authoritative value, you would still see a similar amount of latency in your application, it would just shift to the SendAsync call.

jsquire commented 4 years ago

One more thought that I'd like to add is that if you're not running this operation on the UI thread, where the synchronization context is important, I'd highly recommend adding .ConfigureAwait(false) to each of those async calls. You'll potentially save a non-trivial amount of time not waiting on a contended thread for what appears to be intended as a background operation.

haohighview commented 4 years ago

thanks @jsquire. I added .ConfigureAwait(false) but it took a long time to create.

jsquire commented 4 years ago

Unfortunately, I don't believe there's much direct help that we can offer from the client library perspective; speculatively, it sounds like the network stack on Xamarin.Android is the bottleneck.

May I ask if that snippet is representative of your application use? Creating the producer to send a single event is going to incur a high cost, due to the need to establish the connection each time. Unless you're using the producer client in a very small and limited scope, or network connections are a scarce resource, I'd definitely recommend creating one application-level producer client, caching it, and reusing it for all of your send operations.

You may want to consider creating the client an invoking an operation like GetPartitionIdsAsync during your application startup, which would pay the connection tax and allow your batch creation and send to use them. Your application tear-down would then hold responsibility for closing the client, which would give it a clean shutdown.

haohighview commented 4 years ago

I added ConnectionOptions = new EventHubConnectionOptions { TransportType = EventHubsTransportType.AmqpWebSockets, Proxy = (IWebProxy)null }, now it works great.