Github user ifndef-SleePy commented on the issue:
https://github.com/apache/flink/pull/3395
Hi Till, I almost miss this comments! I didn't see it until a few minutes
ago.
I fully understand your concern. Just let me explain more about your
comments.
1. I agree most of your suggestions. Such as null check, formatting problem
and TestLogger.
2. Currently synchronize problem will not happen. I think replacing the
field value is safe. That's a atomic operation. Correct me if I'm wrong.
3. This PR will not work with other reconciliation PRs. Nobody will notify
these listeners. Actually we implemented reconnection between TM and JM. It
will work with those codes. The reason I make this PR without other
reconciliation PRs is that I think this PR is independent with other parts. I
believe filing a huge PR is both terrible for reviewer and writer. However this
single PR makes you confused. Sorry about that.
4. Actually I'm not sure listener pattern is the best way to do this. But I
think it's the simplest way which makes least modifications of current
implementation. If the TM reconnected with new JM, how can we update the
JobMasterGateway handled by components? I can't figure out a better way except
reimplementing these components.
Anyway, thank you for reviewing and commenting so many! I agree with you
that we should close this PR at this moment. After making an agreement about
main reconciliation PRs, we can talk about what this PR try to implement.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
