ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dmytro Shkvyra" <dshkv...@hortonworks.com>
Subject Re: Review Request 35074: ClusterDeadlockTest unit test fails
Date Sat, 06 Jun 2015 14:00:40 GMT


> On June 6, 2015, 3:45 a.m., Jonathan Hurley wrote:
> > ambari-server/src/test/java/org/apache/ambari/server/state/cluster/DeadlockWarningThread.java,
line 87
> > <https://reviews.apache.org/r/35074/diff/1-2/?file=979122#file979122line87>
> >
> >     This won't return deadlocked threads that are deadlocked by a ReadWriteLock

Yes, but this can check "classical" deadlock and I think we shoul use it too.


> On June 6, 2015, 3:45 a.m., Jonathan Hurley wrote:
> > ambari-server/src/test/java/org/apache/ambari/server/state/cluster/DeadlockWarningThread.java,
line 100
> > <https://reviews.apache.org/r/35074/diff/1-2/?file=979122#file979122line100>
> >
> >     3 seconds seems arbitrary and error prone. Can you explain this wait?

Usages of ThreadMXBean functions is too heavy operation (accordingly documentation), so it
is reasonable waiting some milisecs and give ability for other threads finish.


> On June 6, 2015, 3:45 a.m., Jonathan Hurley wrote:
> > ambari-server/src/test/java/org/apache/ambari/server/state/cluster/DeadlockWarningThread.java,
lines 130-136
> > <https://reviews.apache.org/r/35074/diff/1-2/?file=979122#file979122line130>
> >
> >     Stack equality doesn't mean much when you're exercising the same code from different
threads. You're bound to get a false positive here from the fact that the threads are doing
the same work repeatedly.

I'm not comparing stacks. I compare strings of all threads which run in test (include all
child thread) stacks, so if there no deadlock current stacktrace will not the same each time
(extremely low possibility).


> On June 6, 2015, 3:45 a.m., Jonathan Hurley wrote:
> > ambari-server/src/test/java/org/apache/ambari/server/state/cluster/DeadlockedThreadsTest.java,
lines 151-183
> > <https://reviews.apache.org/r/35074/diff/1-2/?file=979123#file979123line151>
> >
> >     This class isn't testing any deadlock scenario. The locks that are passed in
are never used. Instead, it just uses an internal ReadWriteLock with re-entrancy. There's
no sharing of the RWLock which would exercise the deadlock scenario seen in Ambari

I have run this code and it is a real deadlock. Anyway, it is not be detected by mbean.findMonitorDeadlockedThreads()
My test give ability check my approach for detect actual deadlocks.


On June 6, 2015, 3:45 a.m., Vitalyi Brodetskyi wrote:
> > I still think this approach has flaws in it. Also, I'd like to know if you introduced
a real deadlock in Ambari code and this was able to detect it. If so, could you explain the
deadlock that you introduced and how it was similar to the shared ReadWriteLock deadlock that
the original tests were created for.
> > 
> > Also, feel free to add other senior Ambari members to review this.

Jonathan, I have no possibility to add reviewer there, but you have.
I insist that my Kung Fu better :) It is better exit from unittest accordingly of results
than exit by timeout. If we have no deadlocks all threads become !isAlive ends up. But if
we have deadlocks (doesn't metter kind) all the thread in the test or finish or stuck and
their stacktraces will not change.


- Dmytro


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35074/#review86899
-----------------------------------------------------------


On June 5, 2015, 5:24 p.m., Vitalyi Brodetskyi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35074/
> -----------------------------------------------------------
> 
> (Updated June 5, 2015, 5:24 p.m.)
> 
> 
> Review request for Ambari and Jonathan Hurley.
> 
> 
> Bugs: AMBARI-11692
>     https://issues.apache.org/jira/browse/AMBARI-11692
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Fails on 2 ubuntu workstations
> 
> {noformat}
> 
> Tests in error:
>   testDeadlockWhileRestartingComponents(org.apache.ambari.server.state.cluster.ClusterDeadlockTest):
test timed out after 75000 milliseconds
> 
> Tests run: 3016, Failures: 0, Errors: 1, Skipped: 21
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO]
> [INFO] Ambari Views ...................................... SUCCESS [4.863s]
> [INFO] Ambari Server ..................................... FAILURE [1:49:50.358s]
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
> 
> {noformat}
> 
> {noformat}
> java.lang.Exception: test timed out after 75000 milliseconds
> 	at java.lang.Object.wait(Native Method)
> 	at java.lang.Thread.join(Thread.java:1281)
> 	at java.lang.Thread.join(Thread.java:1355)
> 	at org.apache.ambari.server.state.cluster.ClusterDeadlockTest.testDeadlockWhileRestartingComponents(ClusterDeadlockTest.java:241)
> 	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> 	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
> 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> 	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:45)
> 	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
> 	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:42)
> 	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
> 	at org.junit.internal.runners.statements.FailOnTimeout$StatementThread.run(FailOnTimeout.java:62)
> 
> 	at org.apache.ambari.server.state.svccomphost.ServiceComponentHostImpl.getDesiredStateEntity(ServiceComponentHostImpl.java:1555)
> 	at org.apache.ambari.server.state.svccomphost.ServiceComponentHostImpl.isRestartRequired(ServiceComponentHostImpl.java:1478)
> 	at org.apache.ambari.server.state.ConfigHelper.calculateIsStaleConfigs(ConfigHelper.java:927)
> 	at org.apache.ambari.server.state.ConfigHelper.isStaleConfigs(ConfigHelper.java:376)
> 	at org.apache.ambari.server.state.cluster.ClusterImpl.getClusterHealthReport(ClusterImpl.java:2580)
> {noformat}
> 
> Reproduced in trunk, last commit
> {noformat}
> commit a52eb51d1af0edc9273a947535a2a36886e625da
> Author: Oleg Nechiporenko <onechiporenko@apache.org>
> Date:   Thu May 28 18:02:28 2015 +0300
> 
>     AMBARI-11484. Configs: when doing override, it's hard to find config override (onechiporenko)
> 
> {noformat}
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterDeadlockTest.java
2f064ab 
>   ambari-server/src/test/java/org/apache/ambari/server/state/cluster/DeadlockWarningThread.java
PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/state/cluster/DeadlockedThreadsTest.java
PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/35074/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Vitalyi Brodetskyi
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message