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




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

    I would have like to change this to use real Guice injection, but it would 
have cause too many other changes, so I just followed the anti-pattern.



ambari-server/src/main/java/org/apache/ambari/server/state/Clusters.java (lines 
281 - 288)
<https://reviews.apache.org/r/52425/#comment219144>

    This is the replacement method which, instead of trying to keep the 
ClusterImpl in memory and just clear caches, ClustersImpl will actually 
instantiate a new one, re-initializing the in-memory caches.



ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
 (lines 318 - 323)
<https://reviews.apache.org/r/52425/#comment219145>

    All initialization of in-memory state is done in the constructor now, 
removing the need to check it all over the place. Can you imagine this anti 
pattern:
    
    1) Write new method
    2) See what collections the new method uses
    3) For every collection make sure to call `checkCollectionLoaded()` first
    
    ... yikes



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

    This is done on object construction now (yay) but because the factories 
reference each other (booooo!), we have to initialize the cached objects a 
little smarter. So I just re-organized how the lists are populated.


- Jonathan Hurley


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