You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "yossireg (GitHub)" <gi...@apache.org> on 2018/11/08 08:55:46 UTC

[GitHub] [geode] yossireg opened pull request #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?

- [ ] Is your initial contribution a single, squashed commit?

- [ ] Does `gradlew build` run cleanly?

- [ ] Have you written or updated unit tests to verify your changes?

- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.


[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] yossireg commented on pull request #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Posted by "yossireg (GitHub)" <gi...@apache.org>.
fixed

[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jhuynh1 commented on pull request #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Posted by "jhuynh1 (GitHub)" <gi...@apache.org>.
Minor nitpick, shouldn't this be on the line above?  There seems to be an unnecessary line break.

[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] yossireg commented on pull request #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Posted by "yossireg (GitHub)" <gi...@apache.org>.
requested change was done

[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] yossireg commented on pull request #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Posted by "yossireg (GitHub)" <gi...@apache.org>.
name was changed

[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] yossireg commented on pull request #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Posted by "yossireg (GitHub)" <gi...@apache.org>.
properties gets populated upon the JVM's startup, I simply followed the same syntax that I saw was in used in geode code.
same I did for the new thread monitor functionality that I inserted not long ago.

[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jhuynh1 commented on pull request #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Posted by "jhuynh1 (GitHub)" <gi...@apache.org>.
should the comment default specify 0 instead of 60000?

[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jhuynh1 commented on pull request #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Posted by "jhuynh1 (GitHub)" <gi...@apache.org>.
when does the nonDefaultProperties get populated?  I think I'm misunderstanding how a nonDefault value actually is configured.

[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] yossireg commented on issue #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Posted by "yossireg (GitHub)" <gi...@apache.org>.
Bruce - as per your request i have removed the DistributionConfig property and fetching the parameter value from System property in AcceptorImpl.

[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] yossireg commented on pull request #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Posted by "yossireg (GitHub)" <gi...@apache.org>.
fixed

[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jhuynh1 commented on pull request #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Posted by "jhuynh1 (GitHub)" <gi...@apache.org>.
nitpick, maybe this can be renamed to lastCheckedTime?  

[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] yossireg commented on issue #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Posted by "yossireg (GitHub)" <gi...@apache.org>.
for some reason from time to time following error occures during the CI

geode-core:distributedTest
org.apache.geode.internal.cache.DeltaPropagationDUnitTest > testBug40165ClientReconnects
Could not initialize a primary queue on startup. No queue servers available.

i can see this in other PR's as well
is there any option to trigger only the failed CI? because each time this happens i submit a dummy change - just to trigger the CI again

[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] metatype commented on pull request #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Posted by "metatype (GitHub)" <gi...@apache.org>.
I think this needs an additional check just in case `System.currentTimeMillis()` returns the same value.

`if (checkRegisteredKeysInterval == 0 || ...)`

[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] bschuchardt closed pull request #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Posted by "bschuchardt (GitHub)" <gi...@apache.org>.
[ pull request closed by bschuchardt ]

[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] yossireg commented on pull request #2810: checkRegisteredKeys function consumes alot of CPU - GEODE-5887

Posted by "yossireg (GitHub)" <gi...@apache.org>.
requested change was done

[ Full content available at: https://github.com/apache/geode/pull/2810 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org