----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45538/#review126826 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java (line 835) <https://reviews.apache.org/r/45538/#comment189926> It look slike this method and the one above can access this cache concurrently; you might need to make this Map threadsafe. Also, it's an interesting approach to clearing this cache. Component removals are not very often and I think the original point of clearing the cache was just to ensure it didn't stick around forever. You brought up good points that the cache only was mapping a Long to an Enum; maybe a simple expiring cache would have been simpler/safer/better here. ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/AbstractBaseEventCreator.java (line 28) <https://reviews.apache.org/r/45538/#comment189927> I'm actually a fan of composition over inheritance. If these methods truly are part of a common logic for all Creators, then we can keep it. But would it be better to simply have a class which can perform these operations and merely reference it from all of the RequestAuditEventCreator instances? I don't see any shared state in this class, which is why I ask. - Jonathan Hurley On April 4, 2016, 4:24 a.m., Daniel Gergely wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45538/ > ----------------------------------------------------------- > > (Updated April 4, 2016, 4:24 a.m.) > > > Review request for Ambari, Jonathan Hurley, Nate Cole, and Sebastian Toader. > > > Bugs: AMBARI-15646 > https://issues.apache.org/jira/browse/AMBARI-15646 > > > Repository: ambari > > > Description > ------- > > ThreadLocal: > InitialValue() method is used when initializing the ThreadLocal member > variable. > > Multibinder: > The same logic is used for binding multiple classes from a package > "automatically" as in org.apache.ambari.server.cleanup.CleanupModule. > > Creator properties: > Property retrieval from responses now grouped into an abstract baseclass. It > can get properties from namedPropertySets and propertySets. > > Auditlog enabling: > Added checks to more places in the code to skip auditlog related object > creation if auditlog is disabled. > > Cache: > The previously existing 3 variables now groupped into a single data structure > to act as a cache. Every request has a RequestDetails object, which contains > the last status of the request and a map for tasks. A task has a key that is > composed of a component name and a host name, the value is the previous > status of the task. > By using this structure, tasks for components can easily be removed and if > the RequestDetails has no task, the request itself can also be removed. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java > 79d3470 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/BaseService.java > 2e5b920 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/LogoutService.java > 3b449ca > > ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerDefaultImpl.java > 1cfb740 > > ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerModule.java > b20714b > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/RequestAuditEventCreator.java > ccb39de > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/RequestAuditLoggerImpl.java > 56efd18 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/AbstractBaseEventCreator.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/AlertGroupEventCreator.java > 103fd4d > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/AlertTargetEventCreator.java > 29a241e > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/BlueprintEventCreator.java > bdd6dbe > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/BlueprintExportEventCreator.java > 1416021 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ComponentEventCreator.java > 8034d24 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ConfigurationChangeEventCreator.java > 7e58893 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/CredentialEventCreator.java > 3b1f462 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/DefaultEventCreator.java > d0f57f2 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/GroupEventCreator.java > d926d94 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/HostEventCreator.java > 910280d > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/MemberEventCreator.java > a3c3164 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/PrivilegeEventCreator.java > bdc7b59 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/RecommendationIgnoreEventCreator.java > 6b7bb2b > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/RepositoryEventCreator.java > fe6f8cc > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/RepositoryVersionEventCreator.java > 7c9c731 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/RequestAuditEventCreator.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/RequestEventCreator.java > fd13973 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ServiceConfigDownloadEventCreator.java > 681cfb8 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ServiceEventCreator.java > 2e2b91d > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UnauthorizedEventCreator.java > d53aa68 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UpgradeEventCreator.java > b8a6873 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UpgradeItemEventCreator.java > 9f83172 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UserEventCreator.java > 2b4e5c1 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ValidationIgnoreEventCreator.java > 081f3d3 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ViewInstanceEventCreator.java > 611b1ea > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ViewPrivilegeEventCreator.java > 18b860a > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java > 36469c1 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java > 992d33f > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/GroupResourceProvider.java > 1678931 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentResourceProvider.java > 3c33a23 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/MemberResourceProvider.java > 04e5f67 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java > a45b1ac > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java > 714495f > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserResourceProvider.java > fee1826 > > ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java > 9c2b42b > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilter.java > 5663ed2 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java > 96d6131 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/PermissionHelper.java > ecf2d7a > > ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java > 0b84568 > > ambari-server/src/test/java/org/apache/ambari/server/audit/request/AbstractBaseCreator.java > 02ecb00 > > ambari-server/src/test/java/org/apache/ambari/server/audit/request/RequestAuditLogModule.java > 52ad44c > > Diff: https://reviews.apache.org/r/45538/diff/ > > > Testing > ------- > > > Thanks, > > Daniel Gergely > >