> On Sept. 30, 2016, 12:39 p.m., Robert Levas wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java, > > lines 1104-1110 > > <https://reviews.apache.org/r/52425/diff/1/?file=1516872#file1516872line1104> > > > > If I invalidate a cluster instance for which some reference is in use, > > won't this cause issues unless the referenced invalidated cluser instance > > is refreshed? > > > > Would it be useful to tell the cluster instance that it has been > > invalidated so that when calls are issued on it, it will throw an > > exception. This might be a good debugging tool. > > Jonathan Hurley wrote: > This will use the ClusterFactory to essentially replace the cluster which > now has outdated information in the Clusters internal collections. If the > ClusterImpl is in use, no state data is being changed in it in this method; > it will just return stale information (as it would have already done if it > was serialized out to a web page). > > With your idea, we'd need to lock around the "refresh" of the internal > data - that's what we wanted to get away from.
Thanks for the clarification... dropping this. > On Sept. 30, 2016, 12:39 p.m., Robert Levas wrote: > > ambari-server/src/main/java/org/apache/ambari/server/utils/RetryHelper.java, > > lines 92-94 > > <https://reviews.apache.org/r/52425/diff/1/?file=1516873#file1516873line92> > > > > Aren't the affectedClusters in this collection left with potentially > > invalid data since they reference the old cluster instance? > > Jonathan Hurley wrote: > They are - they will be removed the next time a call comes in through the > API though since the cluster will match on the name in the map. So I didn't > think it was necessary to remove them. I could re-request them from Clusters > in the case of invalidation. I really dislike this class. Thanks for the clarification. Dropping this. - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52425/#review151033 ----------------------------------------------------------- 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 > >
