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