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

pm: Fix busy ticket queue loop #1619

Merged
merged 1 commit into from
Sep 22, 2020
Merged

pm: Fix busy ticket queue loop #1619

merged 1 commit into from
Sep 22, 2020

Conversation

yondonfu
Copy link
Member

@yondonfu yondonfu commented Sep 22, 2020

What does this pull request do? Explain your changes. (required)

This PR fixes a busy ticket queue loop that was causing CPU usage on any node that creates a ticket queue loop (either an O that is redeeming tickets on its own or a transactor that is running a ticket redeemer service) to spike to 100% as soon as the ticket queue loop is created (i.e. if SenderMonitor.MaxFloat() is called). The CPU spike would last until the SenderMonitor stops the ticket queue loop when it stops monitoring an address due to inactivity for a certain period of time.

Specific updates (required)

See commit history.

How did you test each of these updates (required)

Tested manually. Without this change, as soon as you connect an O to a transactor running a redeemer service (i.e. using the -redeemer flag), you can observe the CPU usage of the transactor spike to 100%. With this change, the CPU usage of the transactor in this setup does not spike. The same is true if you start an O that does standalone redemption.

Does this pull request close any open issues?

Fixes #1618

Checklist:

  • README and other documentation updated
  • Node runs in OSX and devenv
  • All tests in ./test.sh pass

Remove default case in select statement which was previously causing the
loop to constantly run. The select statement now blocks until one of the
conditions is fulfilled:

- Subscription error
- Received a quit signal
- New blocks are received via the block subscription
Copy link
Contributor

@kyriediculous kyriediculous left a comment

Choose a reason for hiding this comment

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

Interesting.

So I guess an empty default doesn't mean "do nothing" until a new message is received and there appears to be a difference between that and blocking until a message is received ?

Change looks good to me 👍

@yondonfu
Copy link
Member Author

So I guess an empty default doesn't mean "do nothing" until a new message is received and there appears to be a difference between that and blocking until a message is received ?

Yep. A default case in a switch statement will fire if no other case in the select statement fires first.

@yondonfu yondonfu merged commit 03815e5 into master Sep 22, 2020
@yondonfu yondonfu deleted the yf/fix-queue-busy-loop branch September 22, 2020 18:09
@kyriediculous
Copy link
Contributor

I’m still not entirely sure how this causes 100% CPU load though as there is node code block to be executed. I imagined it would immediately finish and wait for the next iteration of the loop.

@yondonfu
Copy link
Member Author

yondonfu commented Sep 22, 2020

I’m still not entirely sure how this causes 100% CPU load though as there is node code block to be executed. I imagined it would immediately finish and wait for the next iteration of the loop.

The default statement would keep on firing and the loop would keep on running because at the next iteration the default statement would once again fire first. The loop wouldn't be doing anything, but it would take up CPU time that is forced to execute the loop iteration which could otherwise be doing something else.

@kyriediculous
Copy link
Contributor

kyriediculous commented Sep 22, 2020 via email

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.

transactor sits at 100% single-core CPU usage
2 participants