[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[HeartSaVioR]

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1697
  
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.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[revans2]

Github user revans2 commented on the issue:

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


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[srdo]

Github user srdo commented on the issue:

https://github.com/apache/storm/pull/1697
  
@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.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[HeartSaVioR]

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1697
  
@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.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[revans2]

Github user revans2 commented on the issue:

https://github.com/apache/storm/pull/1697
  
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.  


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[srdo]

Github user srdo commented on the issue:

https://github.com/apache/storm/pull/1697
  
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.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[revans2]

Github user revans2 commented on the issue:

https://github.com/apache/storm/pull/1697
  
@HeartSaVioR good catch, I thought I had deleted it.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[HeartSaVioR]

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1697
  
@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.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[revans2]

Github user revans2 commented on the issue:

https://github.com/apache/storm/pull/1697
  
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.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[revans2]

Github user revans2 commented on the issue:

https://github.com/apache/storm/pull/1697
  
Just pushed the upmerged code.  Will look into pulling in #1724 too.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[HeartSaVioR]

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1697
  
@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.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[HeartSaVioR]

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1697
  
@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.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[HeartSaVioR]

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1697
  
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.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[HeartSaVioR]

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1697
  
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.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[HeartSaVioR]

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1697
  
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.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[revans2]

Github user revans2 commented on the issue:

https://github.com/apache/storm/pull/1697
  
The test failures look unrelated.  Some are rat failures caused by test 
logs not being excluded.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[revans2]

Github user revans2 commented on the issue:

https://github.com/apache/storm/pull/1697
  
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.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[HeartSaVioR]

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1697
  
@revans2 Relevant PRs (#1699 #1700 #1705 #1712) are all merged to master. 
Please pull them here.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[revans2]

Github user revans2 commented on the issue:

https://github.com/apache/storm/pull/1697
  
Still have #1699 and #1712 to backport before this is ready


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] storm issue #1697: STORM-2018: Supervisor V2

[Patricia McDade]

Can you please let me know how to unsubscribe I have followed instructions
without success

On Sep 21, 2016 11:08 AM, "revans2"  wrote:

> Github user revans2 commented on the issue:
>
> https://github.com/apache/storm/pull/1697
>
> 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.
>
>
> ---
> 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 infrastruct...@apache.org or file a JIRA ticket
> with INFRA.
> ---
>

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[revans2]

Github user revans2 commented on the issue:

https://github.com/apache/storm/pull/1697
  
We also found #1700 so once that goes in I'll pull it in here too.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1697: STORM-2018: Supervisor V2

[revans2]

Github user revans2 commented on the issue:

https://github.com/apache/storm/pull/1697
  
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.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---