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

[Event Hubs] Implement PartitionManager methods #4538

Merged
merged 6 commits into from
Jul 30, 2019

Conversation

ShivangiReja
Copy link
Member

One of the required parameters to the EventProcessor constructor is partitionManager.

PartitionManager is an interface that describes 3 methods. Created a class called InMemoryPartitionManager which implements PartitionManager methods. Internally InMemoryPartitionManager class maintains a map to store partitionOwnerships.

  • listOwnerships: Accepts an eventHubName and consumerGroupName and returns a list of PartitionOwnership
  • claimOwnerships: Accepts a list of PartitionOwnership, check partitionOwnership in the map if it's not present then adds partitionOwnership into the map and returns the same list of PartitionOwnership.
  • updateCheckpoint: Updates the checkpoint

@ShivangiReja ShivangiReja self-assigned this Jul 30, 2019
@Azure Azure deleted a comment from ShivangiReja Jul 30, 2019
Copy link
Contributor

@chradek chradek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor nits

sdk/eventhub/event-hubs/src/inMemoryPartitionManager.ts Outdated Show resolved Hide resolved
sdk/eventhub/event-hubs/src/inMemoryPartitionManager.ts Outdated Show resolved Hide resolved
}

// @public
export enum CloseReason {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this something all languages are exposing? I don't remember seeing it in our API design, that close() takes a reason. 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposed API for javascript, here it takes CloseReason.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@conniey We brought that up on Monday too, because we were curious how Java would know why the subscription was closed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I also wonder what the user could do differently with that reason (ie. user shutdown vs stolen)? I mean, we log it in the EventConsumer but it is not propagated out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory if it was a shutdown they could checkpoint one last time, whereas if it were stolen they wouldn’t be able to update the checkpoint.

@ShivangiReja ShivangiReja merged commit e9d98bc into Azure:master Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants