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


Fix it, then Ship it!





ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
 (line 880)
<https://reviews.apache.org/r/52425/#comment219183>

    use @StaticallyInject instead?  I know it's becoming an overused mechanism, 
but we wanted to get away from performStaticInjection()



ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
 (lines 104 - 109)
<https://reviews.apache.org/r/52425/#comment219185>

    nit: new ConcurrentHashMap<>();



ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
 (line 183)
<https://reviews.apache.org/r/52425/#comment219186>

    Hot!  Didn't know that Guice did this!



ambari-server/src/main/java/org/apache/ambari/server/utils/RetryHelper.java 
(line 38)
<https://reviews.apache.org/r/52425/#comment219187>

    Can make @Inject'ed with a Provider?


- Nate Cole


On Sept. 30, 2016, 10:53 a.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52425/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2016, 10:53 a.m.)
> 
> 
> Review request for Ambari, Nate Cole, Robert Levas, and Robert Nettleton.
> 
> 
> Bugs: AMBARI-18495
>     https://issues.apache.org/jira/browse/AMBARI-18495
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Many of the business object implementations include needless locks which 
> simply add the overhead and contention in larger clusters. Some examples of 
> these are :
> 
> - HostImpl
> -- get/set DisksInfo()
> -- get/set TotalMemBytes()
> 
> - ServiceComponentHostImpl
> -- get/set MaintenanceState()
> -- get/set LastOpLastUpdateTime()
> 
> These types of methods are found on other business classes as well, like 
> {{ClusterImpl}} and {{ServiceImpl}}. Additionally, methods like 
> {{convertToResponse()}} and {{debugDump()}} need not acquire locks since they 
> are used mostly for serialization of data to the web client where the data 
> will then immediately become stale anyway.
> 
> The {{Clusters}} and {{Cluster}} business objects should have the following 
> work performed:
> - Remove locking around areas where its no longer required
> - Replace collections with thread-safe concurrent versions
> - Remove some reliance on state-full business objects (caches)
> 
> *In general, the only locks which were left inside of ClusterImpl deal with 
> configurations and/or desired configs which are modeled in such a way that it 
> would require extensive changes to the DDL to eliminate the locks*
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
>  5e498f0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java
>  d37e32b 
>   ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java 
> d141df8 
>   ambari-server/src/main/java/org/apache/ambari/server/state/Clusters.java 
> 2d859b3 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
>  a6f0a3b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
>  6318545 
>   ambari-server/src/main/java/org/apache/ambari/server/utils/RetryHelper.java 
> 877e84d 
>   
> ambari-server/src/test/java/org/apache/ambari/server/api/services/ClusterServiceTest.java
>  c36f5fe 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java
>  c7261ea 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterDeadlockTest.java
>  ec01b80 
> 
> Diff: https://reviews.apache.org/r/52425/diff/
> 
> 
> Testing
> -------
> 
> PENDING
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>

Reply via email to