GitHub user srdo opened a pull request:

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

    STORM-2803: Fix leaking threads from Nimbus/TimeCacheMap, slightly refactor 
Time to use more final fields, replaced uses of deprecated classes/methods and 
added a few tests.

    I'll squash once review is done. Ran this a few times on Travis, and it 
looks like SlotTest is stable now. The integration test is still failing 
occasionally. 
    
    Changes: 
    * TimeCacheMap threads were leaking from Nimbus, which caused time 
simulation to be unreliable in later tests, because the leaked threads call 
Time.sleep. Made sure to kill the threads.
    * Replaced TimeCacheMap with RotatingMap in ShellBasedGroupsMapping, since 
that class has the same problem as Nimbus but also can't be cleaned up. Also 
added some tests for this class.
    * As part of adding tests for ShellBasedGroupsMapping, moved some static 
functions from ShellUtils to an interface so they can be stubbed in tests.
    * Removed uses of deprecated Time.startSimulating/stopSimulating in 
storm-server.
    * Removed duplicate sudo in travis.yml
    * Slightly refactored Time so all fields are final, and field resetting 
happens in the constructor. It seems easier to me to reason about thread safety 
this way.
    * Add check to SlotTest.testResourcesChanged to ensure that the worker 
heartbeat hasn't expired in the step from WAITING_FOR_WORKER_START to RUNNING. 
It's a little weird that the test is reusing the original worker heartbeat in 
that step, so maybe changing the test to create a second heartbeat would be 
better. Let me know what you think.
    * Removed non-functional (as far as I could tell) Windows groups command 
from ShellUtils and replaced it with an exception. Running "groups" on Windows 
doesn't do anything to my knowledge.

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

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

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

    https://github.com/apache/storm/pull/2403.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 #2403
    
----
commit 8a755d03fa4804a901fa621fc4cf9ad230a01177
Author: Stig Rohde Døssing <[email protected]>
Date:   2017-11-06T17:01:52Z

    STORM:2803: Fix leaking threads from Nimbus/TimeCacheMap, slightly refactor 
Time to use more final fields, replaced uses of deprecated classes/methods and 
added a few tests.

commit 176c7324ebba8fc0064d83bd8f8e7c5c14b27902
Author: Stig Rohde Døssing <[email protected]>
Date:   2017-11-07T17:45:45Z

    STORM-2803: Add tests for ShellBasedGroupsMapping

----


---

Reply via email to