STORM-2018: Supervisor V2 #1697

revans2 · 2016-09-20T22:05:41Z

Still need to do some more manual testing but the unit tests passed for me.

revans2 · 2016-09-20T22:06:55Z

pom.xml

-                    <source>1.7</source>
-                    <target>1.7</target>
+                    <source>1.8</source>
+                    <target>1.8</target>


Oops forgot to revert this, will go back to 1.7

revans2 · 2016-09-21T15:08:46Z

I ran the same set of manual tests as before, but I now want to wait on #1699 to go into master, and then I will pull it in here. We are in the process of rolling essentially what is this same patch out to staging at Yahoo, and plan to roll it out to production shortly too. If others are feeling uncomfortable about merging this into the 1.x line I am happy to wait until we have it in production.

revans2 · 2016-09-21T16:21:35Z

We also found #1700 so once that goes in I'll pull it in here too.

…o STORM-2117 STROM-2117: Supervisor V2 with local mode extracts resources directory to the wrong directory

… into STORM-2110 Conflicts: storm-core/test/jvm/org/apache/storm/daemon/supervisor/BasicContainerTest.java

revans2 · 2016-09-23T18:22:31Z

Still have #1699 and #1712 to backport before this is ready

HeartSaVioR · 2016-09-25T22:04:31Z

@revans2 Relevant PRs (#1699 #1700 #1705 #1712) are all merged to master. Please pull them here.

… into STORM-2109

… into STORM-2122

revans2 · 2016-09-26T13:44:38Z

Just pulled in the latest set of bug fixes from master. All known issues have been addressed and we have been running in staging with various versions of this patch for over a week now. Expect to roll out to production fairly soon.

revans2 · 2016-09-26T15:30:49Z

The test failures look unrelated. Some are rat failures caused by test logs not being excluded.

HeartSaVioR · 2016-10-01T00:48:50Z

I just cherry-picked commit which excludes logs from RAT. It's merged to master but was part of port work so didn't port back.

HeartSaVioR · 2016-10-01T00:50:52Z

While running build in storm-core I found that null/storm-local directory is created in storm-core. Maybe there's a case base path is set to null.

HeartSaVioR · 2016-10-01T03:20:47Z

storm-core/src/jvm/org/apache/storm/command/HealthCheck.java

@@ -0,0 +1,126 @@
+/**


This file seems to be clashed with healthcheck.clj.

2016-10-01 11:53:24.105 timer o.a.s.d.s.DefaultUncaughtExceptionHandler [ERROR] Error when processing event java.lang.NoClassDefFoundError: org/apache/storm/command/HealthCheck (wrong name: org/apache/storm/command/healthcheck) at java.lang.ClassLoader.defineClass1(Native Method) at java.lang.ClassLoader.defineClass(ClassLoader.java:760) at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142) at java.net.URLClassLoader.defineClass(URLClassLoader.java:467) at java.net.URLClassLoader.access$100(URLClassLoader.java:73) at java.net.URLClassLoader$1.run(URLClassLoader.java:368) at java.net.URLClassLoader$1.run(URLClassLoader.java:362) at java.security.AccessController.doPrivileged(Native Method) at java.net.URLClassLoader.findClass(URLClassLoader.java:361) at java.lang.ClassLoader.loadClass(ClassLoader.java:424) at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331) at java.lang.ClassLoader.loadClass(ClassLoader.java:357) at org.apache.storm.daemon.supervisor.timer.SupervisorHealthCheck.run(SupervisorHealthCheck.java:36) at org.apache.storm.StormTimer$1.run(StormTimer.java:190) at org.apache.storm.StormTimer$StormTimerTask.run(StormTimer.java:83)

HeartSaVioR · 2016-10-01T03:43:19Z

I found two issues, but other than that manual tests passed. Code review is already done from PR for master branch. +1 once these are resolved.

HeartSaVioR · 2016-10-11T22:32:47Z

@revans2
It would be better to address STORM-2131 here as well. Please pull #1724 here.
And could you update the pull request according to the review comments? Supervisor V2 is the one I would want to include to 1.1.0.

Thanks in advance.

HeartSaVioR · 2016-10-17T05:49:15Z

@revans2 Do you have any updates on this? I'm occasionally seeing Supervisor failures so would like to get this merged to 1.x, and even 1.0.x.

revans2 · 2016-10-25T18:51:04Z

@HeartSaVioR Sorry this has taken so long. I am going to upmerge this and pull in #1724 next.

Conflicts: storm-core/src/clj/org/apache/storm/pacemaker/pacemaker_state_factory.clj

revans2 · 2016-10-25T19:07:46Z

Just pushed the upmerged code. Will look into pulling in #1724 too.

revans2 · 2016-10-25T19:21:10Z

Merged in #1724 now too (it was a trivial cherry pick).

@HeartSaVioR if you want to take a look this should be good for merging in.

Just as an FYI we have been running with a version of this in production for a little while now with no real issues.

Once this goes in if you want me to I can take a look at pulling it back to 1.0.x too.

HeartSaVioR · 2016-10-26T00:41:13Z

@revans2 Shouldn't healthcheck.clj be deleted? At least for me HealthCheck.java clashes with healthcheck.clj. I can't clearly say why, might be specific issue with OSX, but anyway there's an issue. I left a comment regarding this.

revans2 · 2016-10-26T13:09:38Z

@HeartSaVioR good catch, I thought I had deleted it.

srdo · 2016-10-26T19:47:27Z

I seem to be getting a few new errors when running some of our own unit tests with this branch. The exceptions are intermittent.

java.lang.NullPointerException
    at org.apache.storm.utils.DisruptorQueue$FlusherPool.stop(DisruptorQueue.java:110)
    at org.apache.storm.utils.DisruptorQueue$Flusher.close(DisruptorQueue.java:293)
    at org.apache.storm.utils.DisruptorQueue.haltWithInterrupt(DisruptorQueue.java:410)
    at org.apache.storm.disruptor$halt_with_interrupt_BANG_.invoke(disruptor.clj:77)
    at org.apache.storm.daemon.executor$mk_executor$reify__4923.shutdown(executor.clj:412)
    at sun.reflect.GeneratedMethodAccessor303.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at clojure.lang.Reflector.invokeMatchingMethod(Reflector.java:93)
    at clojure.lang.Reflector.invokeNoArgInstanceMember(Reflector.java:313)
    at org.apache.storm.daemon.worker$fn__5550$exec_fn__1372__auto__$reify__5552$shutdown_STAR___5572.invoke(worker.clj:668)
    at org.apache.storm.daemon.worker$fn__5550$exec_fn__1372__auto__$reify$reify__5598.shutdown(worker.clj:706)
    at org.apache.storm.ProcessSimulator.killProcess(ProcessSimulator.java:66)
    at org.apache.storm.ProcessSimulator.killAllProcesses(ProcessSimulator.java:79)
    at org.apache.storm.testing$kill_local_storm_cluster.invoke(testing.clj:207)
    at org.apache.storm.testing4j$_withLocalCluster.invoke(testing4j.clj:93)
    at org.apache.storm.Testing.withLocalCluster(Unknown Source)

and this kind of error

java.lang.IllegalStateException: Timer is not active
    at org.apache.storm.timer$check_active_BANG_.invoke(timer.clj:87)
    at org.apache.storm.timer$cancel_timer.invoke(timer.clj:120)
    at org.apache.storm.daemon.worker$fn__5550$exec_fn__1372__auto__$reify__5552$shutdown_STAR___5572.invoke(worker.clj:682)
    at org.apache.storm.daemon.worker$fn__5550$exec_fn__1372__auto__$reify$reify__5598.shutdown(worker.clj:706)
    at org.apache.storm.ProcessSimulator.killProcess(ProcessSimulator.java:66)
    at org.apache.storm.ProcessSimulator.killAllProcesses(ProcessSimulator.java:79)
    at org.apache.storm.testing$kill_local_storm_cluster.invoke(testing.clj:207)
    at org.apache.storm.testing4j$_withLocalCluster.invoke(testing4j.clj:93)
    at org.apache.storm.Testing.withLocalCluster(Unknown Source)

Our tests are running Storm in local mode with no time simulation. I've tried running the same tests on 1.x-branch, and these don't seem to occur there.

revans2 · 2016-10-26T20:15:17Z

OK so going through the code in both cases it looks like the only way that can happen is if the workers is somehow being shut down multiple times. My guess is that because the slots are on different threads there is a race now between shutting down a worker through the slot and shutting down the worker through the cluster shutting down.

I'll look into reproducing it. @srdo is there any way you can share your test case with us? It would make my job a lot simpler of trying to reproduce and fix it.

HeartSaVioR · 2016-10-27T00:37:06Z

@revans2 @srdo
I'm even OK if we file an issue regarding intermittent race condition for local cluster and merge this now, since the race condition of Supervisor in 1.x is much critical. It even occurs on clustered environment.

srdo · 2016-10-27T13:16:45Z

@revans2 I can't share our actual test code since it depends on pretty large chunks of our codebase. I'll try reproducing with an example topology.

I'd be fine with filing a separate issue to fix the race so this PR isn't blocked.

revans2 · 2016-10-27T14:11:24Z

@srdo sounds good. I filed https://issues.apache.org/jira/browse/STORM-2175 to address the race condition.

HeartSaVioR · 2016-10-27T15:32:03Z

OK. Unit and integration tests, and manual tests passed on recent commits.
I'm +1 and will merge this now since there's no more reviewer and this was open for fairly long, more than 1 month despite it's kind: backport.

Thanks for the amazing work.

Btw, I'm in favor of just having 1.1.0, not having 1.0.3 unless there's specific request on it. If you would be OK to port back on demand, we could skip it.

STORM-2018: Supervisor V2

318ab5f

revans2 commented Sep 20, 2016

View reviewed changes

STORM-2018: Move back to JDK 1.7

24ff529

revans2 mentioned this pull request Sep 22, 2016

STORM-2117 Supervisor V2 with local mode extracts resources directory to the wrong directory #1705

Merged

Robert (Bobby) Evans and others added 2 commits September 23, 2016 12:58

Merge branch 'STORM-2117' of https://github.com/HeartSaVioR/storm int…

fb42b96

…o STORM-2117 STROM-2117: Supervisor V2 with local mode extracts resources directory to the wrong directory

Merge branch 'STORM-2110' of https://github.com/revans2/incubator-storm…

88619b2

… into STORM-2110 Conflicts: storm-core/test/jvm/org/apache/storm/daemon/supervisor/BasicContainerTest.java

HeartSaVioR added 2 commits September 26, 2016 08:22

Merge branch 'STORM-2109' of https://github.com/revans2/incubator-storm…

2297757

… into STORM-2109

Merge branch 'STORM-2122' of https://github.com/revans2/incubator-storm…

b12c288

… into STORM-2122

Robert (Bobby) Evans added 2 commits September 26, 2016 09:21

Bad include for JDK 7 support

905c443

Another java 7 fix

f12b7da

HeartSaVioR reviewed Oct 1, 2016

View reviewed changes

HeartSaVioR mentioned this pull request Oct 6, 2016

STORM-2131: Add blob command to worker-launcher, make stormdist directory not writeable by topo owner #1724

Merged

Merge branch '1.x-branch' into STORM-2018-1.x

1d4d414

Conflicts: storm-core/src/clj/org/apache/storm/pacemaker/pacemaker_state_factory.clj

Merge branch 'STORM-2131' of https://github.com/knusbaum/incubator-storm

81cd074

Removed healthcheck.clj. Is not needed

df33efd

asfgit merged commit df33efd into apache:1.x-branch Oct 27, 2016

revans2 mentioned this pull request Oct 27, 2016

STORM-2018: Supervisor V2 (1.0.x) #1747

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STORM-2018: Supervisor V2 #1697

STORM-2018: Supervisor V2 #1697

revans2 commented Sep 20, 2016

revans2 Sep 20, 2016

revans2 Sep 21, 2016

revans2 commented Sep 21, 2016

revans2 commented Sep 21, 2016

revans2 commented Sep 23, 2016

HeartSaVioR commented Sep 25, 2016

revans2 commented Sep 26, 2016

revans2 commented Sep 26, 2016

HeartSaVioR commented Oct 1, 2016

HeartSaVioR commented Oct 1, 2016

HeartSaVioR Oct 1, 2016

HeartSaVioR commented Oct 1, 2016

HeartSaVioR commented Oct 11, 2016

HeartSaVioR commented Oct 17, 2016

revans2 commented Oct 25, 2016

revans2 commented Oct 25, 2016

revans2 commented Oct 25, 2016

HeartSaVioR commented Oct 26, 2016

revans2 commented Oct 26, 2016

srdo commented Oct 26, 2016

revans2 commented Oct 26, 2016

HeartSaVioR commented Oct 27, 2016

srdo commented Oct 27, 2016

revans2 commented Oct 27, 2016

HeartSaVioR commented Oct 27, 2016

STORM-2018: Supervisor V2 #1697

STORM-2018: Supervisor V2 #1697

Conversation

revans2 commented Sep 20, 2016

revans2 Sep 20, 2016

Choose a reason for hiding this comment

revans2 Sep 21, 2016

Choose a reason for hiding this comment

revans2 commented Sep 21, 2016

revans2 commented Sep 21, 2016

revans2 commented Sep 23, 2016

HeartSaVioR commented Sep 25, 2016

revans2 commented Sep 26, 2016

revans2 commented Sep 26, 2016

HeartSaVioR commented Oct 1, 2016

HeartSaVioR commented Oct 1, 2016

HeartSaVioR Oct 1, 2016

Choose a reason for hiding this comment

HeartSaVioR commented Oct 1, 2016

HeartSaVioR commented Oct 11, 2016

HeartSaVioR commented Oct 17, 2016

revans2 commented Oct 25, 2016

revans2 commented Oct 25, 2016

revans2 commented Oct 25, 2016

HeartSaVioR commented Oct 26, 2016

revans2 commented Oct 26, 2016

srdo commented Oct 26, 2016

revans2 commented Oct 26, 2016

HeartSaVioR commented Oct 27, 2016

srdo commented Oct 27, 2016

revans2 commented Oct 27, 2016

HeartSaVioR commented Oct 27, 2016