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
----
---