You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by srdo <gi...@git.apache.org> on 2016/05/02 15:10:26 UTC

[GitHub] storm pull request: STORM-1756: Explicitly null KafkaServer refere...

GitHub user srdo opened a pull request:

    https://github.com/apache/storm/pull/1387

    STORM-1756: Explicitly null KafkaServer reference in KafkaTestBroker \u2026

    \u2026to prevent out of memory on large test classes.
    
    I'm not clear on whether JUnit keeps the reference for the lifetime of the module test, or whether they're released after all tests in a class have run. Either way, a test class with a KafkaTestBroker field and many tests would likely still run out of memory.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/srdo/storm STORM-1756

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/storm/pull/1387.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1387
    
----
commit 0f760c20f2a5d1376d20f1dcb259bdb000cc1d35
Author: Stig Rohde D�ssing <st...@gmail.com>
Date:   2016-05-02T13:07:07Z

    STORM-1756: Explicitly null KafkaServer reference in KafkaTestBroker to prevent out of memory on large test classes

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-1756: Explicitly null KafkaServer refere...

Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the pull request:

    https://github.com/apache/storm/pull/1387#issuecomment-216508276
  
    I did see OOME due to KafkaServer resources. 8x 130MB byte arrays caused the VM to run out of memory. As I said, I can't reproduce it. It's probably not an issue on master when storm-kafka gets downgraded to 0.8.2.1 (here https://github.com/apache/storm/pull/1386), since one of the changes in Kafka 0.9.0.1 was to enable the log cleaner thread by default (and thus allocate some very large byte arrays when starting a server).
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-1756: Explicitly null KafkaServer refere...

Posted by srdo <gi...@git.apache.org>.
GitHub user srdo reopened a pull request:

    https://github.com/apache/storm/pull/1387

    STORM-1756: Explicitly null KafkaServer reference in KafkaTestBroker \u2026

    \u2026to prevent out of memory on large test classes.
    
    I'm not clear on whether JUnit keeps the reference for the lifetime of the module test, or whether they're released after all tests in a class have run. Either way, a test class with a KafkaTestBroker field and many tests would likely still run out of memory.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/srdo/storm STORM-1756

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/storm/pull/1387.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1387
    
----
commit 0f760c20f2a5d1376d20f1dcb259bdb000cc1d35
Author: Stig Rohde D�ssing <st...@gmail.com>
Date:   2016-05-02T13:07:07Z

    STORM-1756: Explicitly null KafkaServer reference in KafkaTestBroker to prevent out of memory on large test classes

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-1756: Explicitly null KafkaServer refere...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:

    https://github.com/apache/storm/pull/1387#issuecomment-216460320
  
    Hi, @srdo 
    Since I didn't see any OOMEs, I'm curious that it is based on assumption or you could provide scenario to reproduce.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-1756: Explicitly null KafkaServer refere...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:

    https://github.com/apache/storm/pull/1387#issuecomment-216505586
  
    Please reopen this issue if you saw intermittent test failure. What I asked is that it's a theory or it occurred some issues.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-1756: Explicitly null KafkaServer refere...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/storm/pull/1387


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-1756: Explicitly null KafkaServer refere...

Posted by srdo <gi...@git.apache.org>.
Github user srdo closed the pull request at:

    https://github.com/apache/storm/pull/1387


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-1756: Explicitly null KafkaServer refere...

Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the pull request:

    https://github.com/apache/storm/pull/1387#issuecomment-216488648
  
    @HeartSaVioR I saw OOME on one of my branches, because each KafkaServerStartable allocates a 130MB byte array for the log cleaner thread. A heap dump showed that there were 8 of these in memory at the time of the exception. I was going off of http://junit.org/junit4/faq.html#atests_18 to explain it, but I can't reproduce it now. I'll close this. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-1756: Explicitly null KafkaServer refere...

Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the pull request:

    https://github.com/apache/storm/pull/1387#issuecomment-216490426
  
    If this turns out to be a problem later, disabling the log cleaner thread during tests is probably a better fix. The tests don't make logs long enough to be compacted anyway.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: STORM-1756: Explicitly null KafkaServer refere...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:

    https://github.com/apache/storm/pull/1387#issuecomment-219342913
  
    +1. It doesn't hurt though we downgrade kafka version for storm-kafka to 0.8.x. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---