> On June 13, 2016, 4:51 p.m., 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?

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.


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48266/#review137326
-----------------------------------------------------------


On June 11, 2016, 6:43 a.m., Daniel Gergely wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48266/
> -----------------------------------------------------------
> 
> (Updated June 11, 2016, 6:43 a.m.)
> 
> 
> 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