-----------------------------------------------------------
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
> 
>

Reply via email to