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