ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jonathan Hurley <jhur...@hortonworks.com>
Subject Re: Review Request 43425: [preview] Component should support a desired version
Date Thu, 11 Feb 2016 14:37:35 GMT

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




ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java (lines 791
- 794)
<https://reviews.apache.org/r/43425/#comment180184>

    With only 2 lines, can we get rid of this method and just invoke the event publisher directly?



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java
(line 1171)
<https://reviews.apache.org/r/43425/#comment180186>

    I dislike how we have 2 enums to actually track the same thing:
    
    UpgradeState
    RepositoryVersionState
    
    Having the RepositoryVersionState as "UPGRADING" is kind of weird since you're not upgrading
the repo; you're upgrading to the repo. 
    
    But even worse is that we have this other enum, UpgradeState, with values like IN_PROGRESS.
    
    My vote is to actually change our code to eliminate 1 enum entirely. 
    
    It seems like the values from RepositoryVersionState are the ones we want (UPGRADING,
UPGRADED) but we should call the enum UpgradeState to make it clear.
    
    Also, can we get rid of UpgradeState.PENDING since it makes zero sense.



ambari-server/src/main/java/org/apache/ambari/server/events/listeners/upgrade/StackVersionListener.java
(line 104)
<https://reviews.apache.org/r/43425/#comment180187>

    If you're upgrading, shouldn't this be something like UPGRADED (see my earlier comment
about too many enums and what we can do to make it clearer). NONE, to me, means nothing is
happening, which is not the case.



ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java
(lines 1790 - 1794)
<https://reviews.apache.org/r/43425/#comment180188>

    Let's also update the documentation to reflect that this only happens during an upgrade.


- Jonathan Hurley


On Feb. 10, 2016, 1:55 p.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43425/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2016, 1:55 p.m.)
> 
> 
> Review request for Ambari, Jonathan Hurley and Nate Cole.
> 
> 
> Bugs: AMBARI-14996
>     https://issues.apache.org/jira/browse/AMBARI-14996
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> When performing an upgrade, we need to specify that a component's desired version is
getting changed.
> 
> Going to open additional jiras for TODOs in code
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java 210fe17

>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java
db8c079 
>   ambari-server/src/main/java/org/apache/ambari/server/events/HostComponentVersionEvent.java
ee65d3d 
>   ambari-server/src/main/java/org/apache/ambari/server/events/listeners/upgrade/StackVersionListener.java
74d4f4b 
>   ambari-server/src/main/java/org/apache/ambari/server/events/publishers/VersionEventPublisher.java
3a11f38 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostComponentStateEntity.java
f92f645 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ServiceComponentDesiredStateEntity.java
d2d1b42 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/MasterHostResolver.java
561350b 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponent.java 7803045

>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentHost.java
f1e8d62 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java
4afc857 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeState.java ced1dd3

>   ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java
92828af 
>   ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostSummary.java
1c36143 
>   ambari-server/src/test/java/org/apache/ambari/server/events/listeners/upgrade/StackVersionListenerTest.java
ae05a6b 
>   ambari-server/src/test/java/org/apache/ambari/server/events/publishers/VersionEventPublisherTest.java
071c6f0 
> 
> Diff: https://reviews.apache.org/r/43425/diff/
> 
> 
> Testing
> -------
> 
> Pending E2E RU/EU upgrades/downgrades on live cluster
> 
> Did not work on unit tests yet
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


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