[Event Hubs Client] Load Balancer Options #13047
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsquire
added
Event Hubs
Client
jsquire
requested review from
christothes,
kinelski,
AlexGhiondea and
serkantkaraca
jsquire
force-pushed
the
eventhubs/load-balancer-options
branch
from
ca48901 to
c223fea
Compare
jsquire
requested review from
KrzysztofCwalina,
pakrym and
tg-msft
as code owners
jsquire
commented
Jun 26, 2020
| @@ -13,6 +13,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Core.TestFramework", | |||
| EndProject | |||
| Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "External", "External", "{797FF941-76FD-45FD-AC17-A73DFE2BA621}" | |||
| EndProject | |||
| Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Messaging.EventHubs", "..\Azure.Messaging.EventHubs\src\Azure.Messaging.EventHubs.csproj", "{65ECFE8C-4D8C-454B-AC63-4559FBE5AF7A}" | |||
There was a problem hiding this comment.
This is temporary until the Event Hubs core package is shipped and we can return to a package dependency.
jsquire
force-pushed
the
eventhubs/load-balancer-options
branch
from
c223fea to
c36446c
Compare
christothes
approved these changes
Jun 26, 2020
sdk/eventhub/Azure.Messaging.EventHubs.Processor/src/EventProcessorClient.cs
Outdated
Show resolved
Hide resolved
sdk/eventhub/Azure.Messaging.EventHubs.Processor/src/EventProcessorClientOptions.cs
Outdated
Show resolved
Hide resolved
sdk/eventhub/Azure.Messaging.EventHubs/src/Consumer/EventHubConsumerClient.cs
Outdated
Show resolved
Hide resolved
sdk/eventhub/Azure.Messaging.EventHubs/src/Processor/LoadBalancingStrategy.cs
Outdated
Show resolved
Hide resolved
The focus of these changes is to allow for additional options to control load balancing within the event processor types set the stage for implementing different strategies for load balancing. Also included are surfacing options within the consumer client to allow tuning the prefetch count and batch size, to allow performance tuning.
jsquire
force-pushed
the
eventhubs/load-balancer-options
branch
from
c36446c to
4a462e5
Compare
jsquire
commented
Jun 26, 2020
| @@ -538,15 +537,17 @@ await foreach (var partitionEvent in eventChannel.Reader.EnumerateChannel(option | |||
| // Determine the partitions for the Event Hub and create the shared channel. | |||
|
|
|||
| var partitions = await GetPartitionIdsAsync(cancellationToken).ConfigureAwait(false); | |||
| eventChannel = CreateEventChannel((BackgroundPublishReceiveBatchSize * partitions.Length * 2)); | |||
|
|
|||
| var channelSize = options.CacheEventCount * partitions.Length * 2L; | |||
There was a problem hiding this comment.
This method is not performance-critical and we advise not calling it for production scenarios. The potential for having a smaller channel size than is ideal for very large values is not a concern.
jsquire
commented
Jun 26, 2020
| @@ -391,8 +388,10 @@ public virtual async Task<string[]> GetPartitionIdsAsync(CancellationToken cance | |||
|
|
|||
| try | |||
| { | |||
| eventChannel = CreateEventChannel((BackgroundPublishReceiveBatchSize * 4)); | |||
| cancelPublishingAsync = await PublishPartitionEventsToChannelAsync(partitionId, startingPosition, options.TrackLastEnqueuedEventProperties, options.OwnerLevel, eventChannel, cancellationSource).ConfigureAwait(false); | |||
| var channelSize = options.CacheEventCount * 4L; | |||
There was a problem hiding this comment.
This method is due to be restructured to not make use of the background publishing channel before release; the overflow protection here is temporary.
prmathur-microsoft
pushed a commit
that referenced
this pull request
Jul 8, 2020
The focus of these changes is to allow for additional options to control load balancing within the event processor types set the stage for implementing different strategies for load balancing. Also included are surfacing options within the consumer client to allow tuning the prefetch count and batch size, to allow performance tuning.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The focus of these changes is to allow for additional options to control load balancing within the event processor types set the stage for implementing
different strategies for load balancing. Also included are surfacing options within the consumer client to allow tuning the prefetch count and batch size, to allow performance tuning.
Note: The associated API changes were reviewed and approved by the architecture board. Tags have been curated to reflect that.
Last Upstream Rebase
Friday, June 26, 1:22pm (EDT)
References and Related Issues