Github user HeartSaVioR commented on the issue:

    https://github.com/apache/storm/pull/2433
  
    @revans2 
    Thanks again for your detailed explanation and taking up the hard security 
work which makes the patch stuck to go on. 
    
    I see you are thinking about deprecating pacemaker after addressing the 
metrics load in ZK, then you are totally right we may want to avoid making it 
as default. At least I think now you are OK to go with this patch with some 
follow-up works, then metrics load in ZK would be removed when we finish all 
the necessary works and merge this in, which is OK for me.
    
    TBH I'm still unsure the migration plan from Metrics V1 to V2 (including 
built-in) since it also opens the possibility for metrics to be transferred in 
different way (maybe via metrics reporter), but that might be out of topic for 
this patch, and we could discuss that after merging Metrics V2 to both 1.x and 
master.
    
    Please double-check my understanding. If my understanding is right, you're 
suggesting to modify below things:
    
    1. (a little unsure whether it is also mandatory for non-container) modify 
supervisor to pick a free port (range should be configurable) for thrift server 
instead of configured static value
    2. change supervisor to include picked port information in heartbeat, and 
change nimbus to leverage the port information instead of configured static 
value for communicating via thrift RPC
    3. change worker to report heartbeat to supervisor via RPC, not via local 
state (disk) which might be problematic from storm-mesos or other container 
solutions
    4. address heartbeat fail-back mechanism for old version workers (reading 
from ZK)
    
    @danny0405 
    Could you check comments from @revans2 and provide inputs? Please also let 
me know if you would not want to deal with supporting for old version workers. 
I'll see I can address it on top of your patch.


---

Reply via email to