ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sebastian Toader" <stoa...@hortonworks.com>
Subject Re: Review Request 39996: Refactor code that caches stale entity references
Date Fri, 06 Nov 2015 09:52:11 GMT

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



ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java (line
92)
<https://reviews.apache.org/r/39996/#comment163972>

    Couldn't this constructor just simply call the second one to avoid code duplication? e.g.
    
    this(service, desiredStateEntity, injector)



ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java (line
353)
<https://reviews.apache.org/r/39996/#comment163973>

    You may forgot this line commented out. In general I think it's a good practice to reload
an entity from db prior modifying it to ensure no stale other entities are referenced (in
case the referenced entities were already deleted in some other place).



ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java (line 108)
<https://reviews.apache.org/r/39996/#comment163977>

    Can this constructor call the other one with the appropriate params to avoid code duplication?



ambari-server/src/test/java/org/apache/ambari/server/testing/DBInconsistencyTests.java (line
72)
<https://reviews.apache.org/r/39996/#comment163986>

    I think this initialisation needs to run only once per test suite so would make it @BeforeClass


- Sebastian Toader


On Nov. 6, 2015, 12:38 a.m., Sid Wagle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39996/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2015, 12:38 a.m.)
> 
> 
> Review request for Ambari, Myroslav Papirkovskyy, Sumit Mohanty, and Sebastian Toader.
> 
> 
> Bugs: AMBARI-13753
>     https://issues.apache.org/jira/browse/AMBARI-13753
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> - Deleted hostcomponent rows re-appear
> - Inconsistencies in hostcomponentstate and hostcomponentdesiredstate tables
> 
> Preliminary patch.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java
6150011 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java bbe2f62

>   ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java
c0804ff 
>   ambari-server/src/test/java/org/apache/ambari/server/testing/DBInconsistencyTests.java
PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39996/diff/
> 
> 
> Testing
> -------
> 
> Unit testing in progress.
> 
> 
> Thanks,
> 
> Sid Wagle
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message