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

Eventhubs java #3046

Merged
merged 560 commits into from
Mar 8, 2019
Merged

Eventhubs java #3046

merged 560 commits into from
Mar 8, 2019

Conversation

mssfang
Copy link
Member

@mssfang mssfang commented Mar 6, 2019

This is pure source code from Eventhubs repo

jtaubensee and others added 30 commits April 3, 2017 16:29
Deleting outdated readme
…naccurate timer issues (Azure#88)

* Fix connection, link and timer object leaks after EntityClose

* fixes stuck behaviors in Sender & Receiver
* MsgReceiver should update the pendingReceive queue before enqueing work to ReactorDispatcher
* fix receiver when prefetch queue is full
* Fix Sender & Receiver dormant links after close
* move messagesender timer out-of reactorDispatcher queue
* handle scenarios where an underlying linkOpen is pending and user invokes receiver.close()
* fix the close paths for send - for dormant links
* Move link open close timers out of ReactorDispatcher
* Clear pendingSends & Receives in case of CloseTimeout
* Don't open amqpconnection if MsgFactory is in Closed() state

* Fix sender stuck issue due to a missing not(!) in if check

* Fix dead links registered for ConnectionHandler errors

* add manual test which can be used to test intermittent connection scanarios effectively

* minor refactor

* don't proactively throw sendlink error on token renewal failure

* nit fixes

* code refactor

* refactoring messagesender

* Update minor release

* minor refactor
…ceiver (Azure#93)

* fix connection open/close timeout paths
* use non ForkJoinPool variants while composing completable futures (Azure/azure-event-hubs-java#4)
* fix thread-safety in MessageSender & MessageReceiver in error cases(Azure/azure-event-hubs-java#4)
Deleting outdated readme
…naccurate timer issues (Azure#88)

* Fix connection, link and timer object leaks after EntityClose

* fixes stuck behaviors in Sender & Receiver
* MsgReceiver should update the pendingReceive queue before enqueing work to ReactorDispatcher
* fix receiver when prefetch queue is full
* Fix Sender & Receiver dormant links after close
* move messagesender timer out-of reactorDispatcher queue
* handle scenarios where an underlying linkOpen is pending and user invokes receiver.close()
* fix the close paths for send - for dormant links
* Move link open close timers out of ReactorDispatcher
* Clear pendingSends & Receives in case of CloseTimeout
* Don't open amqpconnection if MsgFactory is in Closed() state

* Fix sender stuck issue due to a missing not(!) in if check

* Fix dead links registered for ConnectionHandler errors

* add manual test which can be used to test intermittent connection scanarios effectively

* minor refactor

* don't proactively throw sendlink error on token renewal failure

* nit fixes

* code refactor

* refactoring messagesender

* Update minor release

* minor refactor
# Conflicts:
#	azure-eventhubs/src/main/java/com/microsoft/azure/eventhubs/PartitionReceiver.java
…ceiver (Azure#93)

* fix connection open/close timeout paths
* use non ForkJoinPool variants while composing completable futures (Azure/azure-event-hubs-java#4)
* fix thread-safety in MessageSender & MessageReceiver in error cases(Azure/azure-event-hubs-java#4)
…sure environment is deleted after a failed build
* push best practices to samples
* perf benchmark sample
* add gitignore for idea
* improve benchmark sample to vary through put
* address feedback provided by @JamesBirdsall
* Removing samples

* Updating .gitignore

* Removing module from pom.xml

* Providing link for other samples.

* Updating readme link
Add EventHubClient.getRuntimeInfo and getPartitionRuntimeInfo (with retry!)

* CIT for getRuntimeInfo and getPartitionRuntimeInfo

* Make EPH use new getRuntimeInfo for partition ids

* Add retry to getRuntime APIs. Correct capitalization.

* Make mgmt retry async, no sleeps.

* Remove blocking calls in getRuntime*, add security

* Remove extra recreation, FaultTolerantObject handles that

* Use central client id, cleanup, add more tests
…Azure#75)

instances updating.
Finish deprecating PartitionContext.setOffsetAndSequenceNumber.
PartitionContext.checkpoint() now uses last offset of current event batch.
* push best practices to samples
* perf benchmark sample
* add gitignore for idea
* improve benchmark sample to vary through put
* address feedback provided by @JamesBirdsall
* Removing samples
* Updating .gitignore
* Removing module from pom.xml
* Providing link for other samples.
* Updating readme link
Add EventHubClient.getRuntimeInfo and getPartitionRuntimeInfo (with retry!)
* CIT for getRuntimeInfo and getPartitionRuntimeInfo
* Make EPH use new getRuntimeInfo for partition ids
* Add retry to getRuntime APIs. Correct capitalization.
* Make mgmt retry async, no sleeps.
* Remove blocking calls in getRuntime*, add security
* Remove extra recreation, FaultTolerantObject handles that
* Use central client id, cleanup, add more tests
Add spaces after hash/pound signs so that the headers are treated as headers on github
# Conflicts:
#	ConsumingEvents.md
#	PublishingEvents.md
#	azure-eventhubs/src/main/java/com/microsoft/azure/eventhubs/EventHubClient.java
#	azure-eventhubs/src/main/java/com/microsoft/azure/servicebus/ClientConstants.java
#	pom.xml
#	readme.md
SreeramGarlapati and others added 8 commits October 30, 2018 08:16
There is an issue in the logic where operation timeout timer is scheduled as part of a receive call and due to the bug operation timer can be scheduled multiple times although it is supposed to be set only once when there is no pending receive call. Over time as the operation timer keeps getting scheduled during receive API call and scheduled again by the reactor thread while the event is fired and being handled, the number of pending IO operations on the pipe gets incremented significantly and it can cause IO pipe stuck and blocks a write operation on the pipe, which in turn, blocks a receive API. There are two changes to address the issue: a) ensure that operation timer is scheduled at most two times to avoid excessive IO operations on the pipe and b) read all bytes from the channel when signaled so that there are no remaining bytes in the channel.
* Update Apache Proton-J dependency (0.29.0 --> 0.31.0) (Azure#407)

* PartitionReceiver - add a method that provides an EventPosition which corresponds to an EventData returned last by the receiver (Azure#408)

* Support IsPartitionEmpty property for PartitionRuntimeInformation (Azure#399)

* Move setPrefetchCount API to the ReceiverOptions class from the PartitionReceiver and update the settings of Default & Max Prefetch count (Azure#410)

This pull request includes two major changes related to Prefetch API.

1) Move setPrefetchCount API to the ReceiverOptions class so that prefetch value specified by a user can be used instead of using default value when communicating to the service during link open and initializing a receiver. This change also addresses the receiver stuck issue caused by setPrefetchAPI in a race condition.

2) Change the default value and set the upper bound of the prefetch count. Note that prefetch count should be greater than or equal to maxEventCount which can be set when either a) calling receive() API or b) implementing the getMaxEventCount API of the SessionReceiverHandler interface.

* Fixes several issues in the reactor related components (Azure#411)

This pull request contains the following changes.

1) Finish pending tasks when recreating the reactor and make sure pending calls scheduled on the old reactor get complete.
2) Fix the session open timeout issue which can result in NPE in proton-J engine.
3) Make session open timeout configurable and use the value of OperationTimeout.
4) Update the message of exceptions and include an entity name in the exception message.
5) API change - use ScheduledExecutorService.
6) Improve tracing.

* Implement comparable on EventData (Azure#395)

* Update receive/send link creation logic and improve tracing (Azure#414)

* Prep for releasing client 2.0.0 and EPH 2.2.0 (Azure#415)
…f4b0d91f36da'

git-subtree-dir: eventhubs/data-plane
git-subtree-mainline: 3eb32a4
git-subtree-split: e68c319
@mssfang mssfang requested review from duncanjo2002, JonathanGiles and mayurid and removed request for duncanjo2002 March 6, 2019 22:13
@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

Copy link
Member

@JonathanGiles JonathanGiles left a comment

Choose a reason for hiding this comment

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

From the looks of this it is an unchanged copy of the existing event hubs repo, retaining all commit history without any changes by Shawn or others. In this case, I am happy to have this merged and have a follow-up PR to integrate it into our build processes and standardise it as required to meet the transition guidelines and requirements.

@mssfang mssfang requested a review from weshaggard March 7, 2019 01:41
@JonathanGiles
Copy link
Member

@weshaggard Would appreciate you giving this a second +1 before merging, once you are back in the office. I'm assured that this is nothing more than a copy / paste of revision history with no changes or integration into our systems and processes. That will come in a follow-up PR.

@weshaggard
Copy link
Member

Is there any particular reason we feel like we should merge the commit history first and then do the integration into the repo? While I'm not necessarily opposed to it I don't see much value in doing this way as we should be able to clean a lot of this up and get it working in our build in another commit but in the same PR.

Also keep in mind that while it might not be included in our builds yet there are analysis tools like component governance that run on the raw sources and might start flagging that in our internal builds.

@JonathanGiles
Copy link
Member

I like the ability to review a pull request with the integration changes applied. Merging them into this pull request makes it harder to do that, and has in the past made people uneasy about what other changes might have been applied. I know it is possible to review commit by commit, but in my experience so far the transition into our repo had been performed over many commits, making reviewing more difficult than ideal.

I don't strongly prefer doing it this way, but it does seem to make things easier.

@mssfang
Copy link
Member Author

mssfang commented Mar 7, 2019

As I know, there will be three seperate PR;

  1. clean source code without any changes [this PR].
  2. source code with the neccessary code and pom changes by following "Adding Track One Components" doc
  3. there is a ongoing PR in another repo, but has a small change in this repo to corresponding the changes.

With 1) clean code PR, the 2) will be easier to show the diff made.

@weshaggard
Copy link
Member

weshaggard commented Mar 7, 2019

I agree you would have to review commit-by-commit which can make it kind of difficult to see the exact changes but still doable. I'm OK with this approach as long as we have clear work items slated to clean-up the stuff we are bringing in. I don't want all these merges to cause us to have a lot of dead files (i.e. all the various github templates, other CI yml files, other .gitattribute files, etc) and ideally we wouldn't have any git source history for them either but as long as they get cleaned out in a follow-up I can live with that.

@mayurid
Copy link
Member

mayurid commented Mar 7, 2019

I agree @weshaggard. We will create the items. We also have a step 1.5 or 2.5 where we make this code checkstyles and spot bugs clean to meet the overall repo guidelines.

@JonathanGiles
Copy link
Member

Thanks. I'll assume we can now get your approval here and for storage?

Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

First step looks good lets be sure to file follow-up items for the remaining clean-up work.

@mssfang mssfang self-assigned this Mar 7, 2019
@mssfang mssfang added the Client This issue points to a problem in the data-plane of the library. label Mar 7, 2019
@mssfang
Copy link
Member Author

mssfang commented Mar 7, 2019

Following issue filed;

Eventhubs: Integrate event hubs into azure-sdk-for-java repo #3065

@weshaggard weshaggard merged commit 416e8d5 into Azure:master Mar 8, 2019
@mssfang mssfang deleted the eventhubs-java-3 branch May 23, 2019 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.