-
1. Re: High CPU usage due to QueueImpl Runnable modifying checkDirect by ScheduledExecutorService
clebert.suconic Jan 31, 2013 9:27 PM (in response to alex75)hmmmm.. this needs to be optimized!
Can you have your code patched for now until we find a better solution?
Can you open a JIRA?
-
2. Re: High CPU usage due to QueueImpl Runnable modifying checkDirect by ScheduledExecutorService
clebert.suconic Jan 31, 2013 9:47 PM (in response to clebert.suconic)I'm going to change this on master right away. (the next release that is due on the next few days).
I will evaluate bringing that change into a 2.2 stable branch (usually for EAP)
-
3. Re: High CPU usage due to QueueImpl Runnable modifying checkDirect by ScheduledExecutorService
clebert.suconic Jan 31, 2013 9:47 PM (in response to clebert.suconic)I have changed this on master (currently on my git only):
https://github.com/clebertsuconic/hornetq/commit/0b39e275b05c252138b50d89bfac9ef4b0eb3f00
Will merge this as soon as I complete the testsuite.. probably tomorrow morning.
-
4. Re: High CPU usage due to QueueImpl Runnable modifying checkDirect by ScheduledExecutorService
clebert.suconic Jan 31, 2013 9:52 PM (in response to clebert.suconic)Just done a small squah on that commit, the ID changed:
https://github.com/clebertsuconic/hornetq/commit/8c07f5a89904702ce7ff7dc94bb4116f3c93e98f
-
5. Re: High CPU usage due to QueueImpl Runnable modifying checkDirect by ScheduledExecutorService
alex75 Feb 1, 2013 1:54 AM (in response to clebert.suconic)First of all: you (and your HornetQ colleagues) are amazing! Such fast feedbacks. That's more than I ever would've expected!
Now about your commit: I guess I've overseen something because currently I don't know how your condition ever may become true?
lastDirectDeliveryCheck - System.currentTimeMillis() > CHECK_QUEUE_SIZE_PERIOD
Because lastDirectDeliverCheck is initialized by 0 and 0 - System.currentTimeMillis() will never be > CHECK_QUEUE_SIZE_PERIOD. But I still think it's my fault and I've missed some important code.
Now my answers to your questions:
"Can you have your code patched for now until we find a better solution?"
Yes, no problem: I'll patch the current 2.2 version we're using at our customer. We already had to do this for https://issues.jboss.org/browse/HORNETQ-1008 which was opened by my colleague.
"Can you open a JIRA?"
Should I still open a JIRA even after you've already commited your change? If so, it's no problem I'll create the issue.
-
6. Re: High CPU usage due to QueueImpl Runnable modifying checkDirect by ScheduledExecutorService
alex75 Feb 1, 2013 4:29 AM (in response to alex75)I would suggest 2 modifications for your commit:
I think the variable lastDirectDeliverCheck should be volatile and the second is about the condition I already wrote. I think you just mixed up the substraction:
lastDirectDeliveryCheck - System.currentTimeMillis() > CHECK_QUEUE_SIZE_PERIOD
should be changed to
System.currentTimeMillis() - lastDirectDeliveryCheck > CHECK_QUEUE_SIZE_PERIOD
-
7. Re: High CPU usage due to QueueImpl Runnable modifying checkDirect by ScheduledExecutorService
clebert.suconic Feb 1, 2013 8:59 AM (in response to alex75)nice catch!
I actually had a test failing because of the subtraction... (it is not merged on the main branch yet.. it still on my fork at this point).
And I also meant to make it volatile...
The commit was squashed on my github, and I will send a PR if everything is ok on the next tests on my box here.
https://github.com/clebertsuconic/hornetq/commit/b178552b3ecaec3b374def0ecb96e2ff5198aacb
Also: Case you make your own patch, make a fork on github, and make a tag for what you put in production. If later you see any issues it will be easier for us to import your fork and compare stack-traces.. etc.
I would also change the versions (look at a small script called change-versions.sh on Branch_2_2_EAP)
It is common we have to evaluate stack traces and now knowing what branch it came from.
-
8. Re: High CPU usage due to QueueImpl Runnable modifying checkDirect by ScheduledExecutorService
clebert.suconic Feb 1, 2013 8:59 AM (in response to clebert.suconic)And if you can still open a JIRA? I will squah the commit with the JIRA id when you do it.
-
10. Re: High CPU usage due to QueueImpl Runnable modifying checkDirect by ScheduledExecutorService
clebert.suconic Feb 1, 2013 4:45 PM (in response to clebert.suconic)merged on the three branches (master (2.3), 2.2 for EAP5 and 2.2 for AS7 (EAP5))
Thanks!
let me know if you have any other issues...
-
11. Re: High CPU usage due to QueueImpl Runnable modifying checkDirect by ScheduledExecutorService
alex75 Feb 2, 2013 9:53 AM (in response to clebert.suconic)Thanks for opening and closing the jira issue with your fix.
I'll try to create a git branch including our modifications - never tried to work git on my own before ...
We already installed a self created patch for the issue https://issues.jboss.org/browse/HORNETQ-1122 at our customer (having the problems) yesterday and it seems to run fine.