Skip to content

Conversation

@yvgopal
Copy link
Member

@yvgopal yvgopal commented Sep 24, 2019

Session pump keeps trying to accept sessions and if a session is not available on the service, link creation will timeout. CoreMessageReceiver object is created, and then discarded as link creation fails. But the scheduled timer objects that are created in the constructor are not cancelled and after some time, there will too many timers taking CPU cycles and holding reference to CoreMessageRecevier objects preventing them from being garbage collected.

@yvgopal yvgopal requested a review from nemakam as a code owner September 24, 2019 23:25
@yvgopal yvgopal requested review from binzywu and mssfang September 24, 2019 23:25
@mssfang
Copy link
Contributor

mssfang commented Sep 25, 2019

Add Hemant to the PR since he is looking at service bus track 2

@binzywu
Copy link
Contributor

binzywu commented Sep 26, 2019

guys, this is an urgent fix we need to rollout. Please review it asap. I will push for the merge by end of today.


this.timedOutUpdateStateRequestsDaemon = () -> {
try {
if (CoreMessageReceiver.this.getIsClosed())
Copy link
Contributor

@mssfang mssfang Sep 26, 2019

Choose a reason for hiding this comment

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

Suggested change
if (CoreMessageReceiver.this.getIsClosed())
if (CoreMessageReceiver.this.getIsClosed()) {
``` #Resolved

// CONTRACT: message should be delivered to the caller of MessageReceiver.receive() only from prefetched messages
this.returnMesagesLoopDaemon = () -> {
try {
if (CoreMessageReceiver.this.getIsClosed())
Copy link
Contributor

@mssfang mssfang Sep 26, 2019

Choose a reason for hiding this comment

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

Suggested change
if (CoreMessageReceiver.this.getIsClosed())
if (CoreMessageReceiver.this.getIsClosed()) {
``` #Resolved

@mssfang
Copy link
Contributor

mssfang commented Sep 26, 2019

Just a few nit. LGTM

@scbedd
Copy link
Member

scbedd commented Sep 28, 2019

/azp run java - servicebus - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssfang mssfang merged commit ba7da44 into Azure:master Sep 28, 2019
@yvgopal yvgopal deleted the vijay-session-pump-high-cpu branch October 21, 2019 22:28
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