> On jún. 13, 2016, 4:51 du, Robert Nettleton wrote: > > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartbeatProcessor.java, > > line 191 > > <https://reviews.apache.org/r/48266/diff/5/?file=1415529#file1415529line191> > > > > I'm a little concerned that this approach could have performance issues > > down the road. > > > > I'm wondering if it might be worth considering launching a separate > > task thread when the cluster is created by the TopologyManager, that can > > monitor the state of the LogicalRequest as the deployment progresses. > > > > I would recommend asking Sumit to review this as well, since it's not > > clear to me that this is the approach that should be taken. > > Sebastian Toader wrote: > This loop simply iterates through the received reports extracts cluster > id and publishes an event for further processing. The event is consumed on a > separate thread 'ambari-event-bus' which is set up in > ```AmbariEventPublisher```. Do you think that the extraction of cluster name > and cluster id may slow the the heartbeat handler? > > Robert Nettleton wrote: > Hi Sebastian, Thanks for this information. > > I guess I'd be concerned about any extraneous additions to the > HeartBeatHandler, especially given that it seems like there would be other > alternatives. > > Was the possibility of launching a thread in the TopologyManager to > monitor a cluster deployment's status considered? Since the TopologyManager > already has access to the LogicalRequest and the underlying physical requests > as well, it seems like the most natural fit for monitoring status, and > providing a way to log this information. > > My overall concern is that the heartbeat handlers are being modified just > to support logging a message when the cluster has completed. As I mentioned > above, it seems like it would be simpler and more straightforward to resolve > this with a thread that monitors the progress, using the resource APIs that > already exist in Ambari. > > If the approach I've just detailed was considered, can you post why it > was not chosen? Are there some technical concerns that make using a separate > thread not feasible? > > Thanks.
Hi Bob, HeartbeatHandler is not modified only to support logging. Now it sends an event (which is basically free: a list is created and cluster ids are stored in that. Cluster ids are from a cache, so there is no performance issue there either), and this event is handled asynchronously (logging part of the code is executed by a different thread). So it does not affect HeartbeatHandler. So it is indeed a separate thread. However if you still have concerns, let's talk, I am open to any other working solution. - Daniel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48266/#review137326 ----------------------------------------------------------- On jún. 11, 2016, 6:43 de, Daniel Gergely wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48266/ > ----------------------------------------------------------- > > (Updated jún. 11, 2016, 6:43 de) > > > Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Nettleton, > Sandor Magyari, Sumit Mohanty, and Sebastian Toader. > > > Bugs: AMBARI-17053 > https://issues.apache.org/jira/browse/AMBARI-17053 > > > Repository: ambari > > > Description > ------- > > Add a log message to see when the cluster is ready to use (cluster creation > finishes). > > An event after each heartbeat. At that time cluster provision request is > checked if it is finished or not. In case of an ambari server restart, the > replayed requests are used to determine which one was the provision request. > I could not find a nice way to do it, but I could make a workaround. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartbeatProcessor.java > c6036c2 > > ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java > 1079806 > > ambari-server/src/main/java/org/apache/ambari/server/events/HeartbeatProcessingFinishedEvent.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedState.java > 77419d8 > > ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedStateImpl.java > 324a397 > > ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java > e3f5b49 > > ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java > fd8653c > > Diff: https://reviews.apache.org/r/48266/diff/ > > > Testing > ------- > > Succeeded locally > > > Thanks, > > Daniel Gergely > >