ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "John Speidel" <jspei...@hortonworks.com>
Subject Re: Review Request 27700: Allow for server-side commands
Date Tue, 11 Nov 2014 17:24:08 GMT


> On Nov. 10, 2014, 4:32 p.m., Robert Nettleton wrote:
> > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java,
line 654
> > <https://reviews.apache.org/r/27700/diff/2/?file=754428#file754428line654>
> >
> >     I'm a little worried by the presence of the sleep here.  
> >     
> >     It might be worth considering making this a functional test instead?  
> >     
> >     Again, just concerned about the unit tests having intermittent failures due
to concurrency problems.
> 
> Robert Levas wrote:
>     We really need to sleep here so that the scheduler and executer have time to run
- which are in different threads.  The idea is that a tasks a scheuled and we are polling
its status to see when it is completed.. then we make sure its status is what was expected.
I don't think there is another way around it.

I agree with Bob on this and feel strongly that using sleep statements like this result in
non-deterministic tests.  
I haven't really looked at the ActionScheduler to provide you a solution but can give some
genral comments on testing of this nature.
- If you can run the test in a single thread, that is always best.  For example, sometimes
it is possible to call run directly.
- If a separate thread must be used, it is best to have a deterministic way to determine if
a task has completed.  For example, if you have access to the task perhaps it has state that
can be queried.
- If you must use another thread and have no way to determine with certainty that a task has
completed, use a "wait for" approach.  The idea is that you have a loop that does an assertion
every couple of ms.  You keep looping until the assertion holds or until you "timeout" and
fail the test.  The timeout should be rather long since it is only applicable in the case
of a test failure. For the successful scenario, the test will not block for long sine we are
checking the condition in small intervals.


- John


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


On Nov. 10, 2014, 2:05 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27700/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2014, 2:05 p.m.)
> 
> 
> Review request for Ambari, dilli dorai, Jonathan Hurley, John Speidel, Nate Cole, Robert
Nettleton, and Sid Wagle.
> 
> 
> Bugs: AMBARI-7985
>     https://issues.apache.org/jira/browse/AMBARI-7985
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Ambari currently handles client-/agent-side commands; however there is no ability to
handle server-side commands. Server-side commands should be specified as a task in a stage
and managed along with the stage.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessor.java
1f99b4a 
>   ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java
5e879cc 
>   ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionManager.java
e2fad5f 
>   ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionScheduler.java
81fee75 
>   ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Stage.java bbc5ac3

>   ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java
52b0ba6 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java
6920a9e 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerAction.java
be885b5 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionManager.java
011cf06 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionManagerImpl.java
3a16c77 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentHostEvent.java
78590fc 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentHostEventType.java
b43ac9c 
>   ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostServerActionEvent.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostUpgradeEvent.java
8b375fe 
>   ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java
36acbc2 
>   ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionManager.java
5a2c467 
>   ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java
7224924 
>   ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java
6e78b1d 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
b2c023f 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/MockServerAction.java
PRE-CREATION 
>   ambari-server/src/test/python/org/apache/ambari/server/serveraction/ServerActionExecutorTest.java
PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/27700/diff/
> 
> 
> Testing
> -------
> 
> -------------------------------------------------------------------------------
> Test set: org.apache.ambari.server.serveraction.ServerActionExecutorTest
> -------------------------------------------------------------------------------
> Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 41.822 sec
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


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