You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Jaimin Jetly <ja...@hortonworks.com> on 2017/01/06 21:50:01 UTC
Re: Review Request 53686: Stage and Request status should be
persisted in the database
> On Dec. 15, 2016, 3:40 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionScheduler.java, line 186
> > <https://reviews.apache.org/r/53686/diff/5/?file=1584762#file1584762line186>
> >
> > What I meant was this isn't a typically-named java variable. Just use something like "taskStatusLoaded", and make the field private.
ok. I will address this in next revision of the patch.
> On Dec. 15, 2016, 3:40 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/events/TaskEvent.java, line 34
> > <https://reviews.apache.org/r/53686/diff/5/?file=1584768#file1584768line34>
> >
> > make private
I will address this in next revision of the patch.
> On Dec. 15, 2016, 3:40 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/events/TaskEvent.java, lines 46-48
> > <https://reviews.apache.org/r/53686/diff/5/?file=1584768#file1584768line46>
> >
> > javadoc
I will address this in next revision of the patch.
> On Dec. 15, 2016, 3:40 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/events/listeners/tasks/TaskStatusListener.java, lines 616-620
> > <https://reviews.apache.org/r/53686/diff/5/?file=1584770#file1584770line616>
> >
> > This enum probably shouldn't be defined here since it's use is mostly by CalculatedStatus
I will address this in next revision of the patch.
> On Dec. 15, 2016, 3:40 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java, lines 658-665
> > <https://reviews.apache.org/r/53686/diff/5/?file=1584772#file1584772line658>
> >
> > method is nearly identical to merge() - could just use a boolean overload: merge(HRCE, boolean)
well I did not want to change the signature of merge() method but I see your point about code duplicacy.
I will make necessary changes in next revision of the patch so that merge(HostRoleCommandEntity entity) calls mergeWithoutPublishEvent(HostRoleCommandEntity entity) to get the work done rather than doing almost same work plus one extra line of code
> On Dec. 15, 2016, 3:40 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/events/listeners/tasks/TaskStatusListener.java, lines 120-121
> > <https://reviews.apache.org/r/53686/diff/5/?file=1584770#file1584770line120>
> >
> > This affects a set of requests and stages. If that's true, then this should be marked @Transactional as well.
Yes. It can affect multiple stages and requests. But updating of one stage/request is independent from updating another stage/request.
Basically I am not sure that doing transaction rollback on failure of one of the stage update will help in keeping database consistent.
But not doing so might help keep databse relatively more updated
- Jaimin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53686/#review159228
-----------------------------------------------------------
On Dec. 14, 2016, 8:20 p.m., Jaimin Jetly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53686/
> -----------------------------------------------------------
>
> (Updated Dec. 14, 2016, 8:20 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Nate Cole, Sumit Mohanty, and Sid Wagle.
>
>
> Bugs: AMBARI-18868
> https://issues.apache.org/jira/browse/AMBARI-18868
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Stage and Request status should be persisted in the database.
>
> upgrading to ambari-3.0.0 should add status for all present stages and request for the cluster.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java 2c87583
> ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionScheduler.java e80b020
> ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleStatus.java 3656bfe
> ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Request.java 31e11c1
> ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Stage.java 4a05b32
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/CalculatedStatus.java 3c415df
> ambari-server/src/main/java/org/apache/ambari/server/events/TaskCreateEvent.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/events/TaskEvent.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/events/TaskUpdateEvent.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/events/listeners/tasks/TaskStatusListener.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/events/publishers/TaskEventPublisher.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java 02c4091
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RequestDAO.java 1c4d0a3
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/StageDAO.java d2f899f
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostRoleCommandEntity.java 74271b9
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/RequestEntity.java 7944d21
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/StageEntity.java f9c8810
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/StageEntityPK.java 9ca0470
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java 4f90ef3
> ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql 8cf2c0d
> ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 82ce31e
> ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql e2c2dd5
> ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 4e9a535
> ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql 0ba7df6
> ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql d8cad6f
> ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java 1ca777d
> ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java f86c02e
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java e23ba62
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeSummaryResourceProviderTest.java baec7df
> ambari-server/src/test/java/org/apache/ambari/server/events/listeners/tasks/TaskStatusListenerTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java cc49cbd
> ambari-server/src/test/java/org/apache/ambari/server/state/services/RetryUpgradeActionServiceTest.java 7dd9932
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog300Test.java d7979e8
>
> Diff: https://reviews.apache.org/r/53686/diff/
>
>
> Testing
> -------
>
> Verified manually on a cluster by making api requests and upgrading ambari.
> Add unit tests.
> Verified that the patch does not break any existing unit tests on dev box. Jenkins job overall unit test result pending..
> Verified on 1000 node cluster that the patch does not regress big operations. Executed Stop Services and Start Services API call which gernerated around 9000 tasks and compared request completion time for these operations. There was a minor performance gain with the patch. As part of https://issues.apache.org/jira/browse/AMBARI-18889, I will look if we can use the request status and stage status to further enhance performance.
>
>
> Thanks,
>
> Jaimin Jetly
>
>