[GitHub] [spark] HeartSaVioR commented on issue #27004: [SPARK-30348][CORE] Fix flaky test failure on "MasterSuite.SPARK-27510: Master should avoid ..."

[GitBox <gi...@apache.org>]

HeartSaVioR commented 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 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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org