> 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. > > Jaimin Jetly wrote: > ok. I will address this in next revision of the patch.
This has been addressed in new 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 > > Jaimin Jetly wrote: > I will address this in next revision of the patch. This has been addressed in new 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 > > Jaimin Jetly wrote: > I will address this in next revision of the patch. This has been addressed in new 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 > > Jaimin Jetly wrote: > I will address this in next revision of the patch. This has been addressed in new 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) > > Jaimin Jetly wrote: > 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 This has been addressed in new revision of the patch. - Jaimin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53686/#review159228 ----------------------------------------------------------- On Jan. 17, 2017, 9:43 p.m., Jaimin Jetly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53686/ > ----------------------------------------------------------- > > (Updated Jan. 17, 2017, 9:43 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 > 7837a7b > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionScheduler.java > dabcb98 > > 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 b79c945 > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 1c502bc > ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql c6d4ad0 > ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 1be87bb > ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql abe48e8 > ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 169a464 > > 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 > 6cc511e > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java > a702e6f > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeSummaryResourceProviderTest.java > e398a54 > > 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 > ae85241 > > 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 > >
