Re: Review Request 14405: ConstantTimeBackoff test and cleanup

[Darren Shepherd <da...@gmail.com>]

> On Sept. 30, 2013, 6:12 p.m., Darren Shepherd wrote:
> > utils/src/com/cloud/utils/backoff/impl/ConstantTimeBackoff.java, line 60
> > <https://reviews.apache.org/r/14405/diff/1/?file=359645#file359645line60>
> >
> >     I wouldn't log at WARN level for this, can you change to DEBUG.  Hitting that catch block is expected as part of the wakeup behavior
> 
> Laszlo Hornyak wrote:
>     Can we agree in an INFO level log? It seems it is not a usual flow of events, but caused by administrator interaction through a JMX (see ConstantTimeBackoffMBean)

How about INFO, but don't print stack trace (so e.getMessage())?  I'd rather it not look like an error.  Or maybe not log the exception at all as its seems misleading.  I hate to debate over log statements, so change if you want or not.  I'll commit this when you close the issue.


- Darren


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


On Sept. 30, 2013, 8:27 p.m., Laszlo Hornyak wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14405/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2013, 8:27 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> - javadoc changed - the old one was copy-pasted from AgentShell
> - start and stop method removed - they did the same as the overridden methods
> - _counter removed as it was only written, but never read
> - remove from _asleep map was moved to a finally block, to make sure it is removed even in case of the thread gets interrupted
> - Tests created for the above scenarios.
> 
> 
> Diffs
> -----
> 
>   agent/src/com/cloud/agent/AgentShell.java bf1e818 
>   utils/src/com/cloud/utils/backoff/impl/ConstantTimeBackoff.java 976e369 
>   utils/test/com/cloud/utils/backoff/impl/ConstantTimeBackoffTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14405/diff/
> 
> 
> Testing
> -------
> 
> test included
> 
> 
> Thanks,
> 
> Laszlo Hornyak
> 
>