ifndef-SleePy commented on issue #9147: [FLINK-11631][test] Harden 
TaskExecutorITCase
URL: https://github.com/apache/flink/pull/9147#issuecomment-521596092
 
 
   Hi @tillrohrmann , thanks for reviewing! Just a simple discussion below.
   
   > ... it violates encapsulation of the MiniCluster.
   
   Yes, it's by design. Actually the encapsulation of `MiniCluster` is already 
broken somehow, IMO. For example, there are several unnecessary `protected` 
methods exposed to `TestingMiniCluster`. The boundary of encapsulation here 
seems to be a bit fuzzy to me :(
   
   > Moreover, I think the existing behaviour is good because otherwise we 
would not have noticed the problem with `Tasks` terminating too late.
   
   I have to admit that it indeed found something although I'm not a fan of the 
behavior of keeping the corrupt `TaskExecutor` even we terminated it. I always 
think it might bring us some trouble someday.
   
   Actually I think this PR is not a big deal because I believe 
https://github.com/apache/flink/pull/9072 could be merged soon. The unstable 
scenario should not exist anymore after merging that. 
   
   Anyway, thanks again for the suggestion :)
   This PR and the JIRA ticket can be closed.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to