HeartSaVioR edited a comment on issue #27004: [SPARK-30348][CORE] Fix flaky 
test failure on "MasterSuite.SPARK-27510: Master should avoid ..."
URL: https://github.com/apache/spark/pull/27004#issuecomment-568808853
 
 
   I don't think adding delay to adjust timing would work; 9 ms may not be the 
fastest run, and it can be pretty much slower than that. For example, one of 
passed case, submitting driver to removing app from master took more than 30 
ms, more than 3 times slower than the failure case. As you might notice, we 
can't predict how long will the execution takes when we increases 
MAX_EXECUTOR_RETRIES, so that's not an option.
   
   There have been so many timing issues while investigating flaky test 
failures, and in my experience adding delay without exact calculation of timing 
(only work the timing is predictable) doesn't fix the issue. I've seen couple 
of issues being reopened if the fix was just adding sleep. If the test fails 
due to timing issue, we should try not to rely on timing. (Ideally all tests 
shouldn't rely on timing, but unfortunately sometimes we have to.)
   
   Exposing field to package private for testing might not be cool, though we 
have been allowed it in various spots. We could leverage PrivateMethodTester if 
we don't want to expose it.
   
   Btw, looks like Master already exposes some fields including `idToApp` as 
public (even not package private) which are "mutable", worse than the change. 
Would we want to clean up these as well? Either just making package private in 
this PR, or find better way to expose safely, like only exposing method which 
provides copied version of the instance.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to