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

Reply via email to