[GitHub] [spark] yikf commented on pull request #37493: [MINOR] To use the `Utils.tryLog` to eliminate duplicate code in `ExecutorAllocationManager.start` snippet

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

yikf commented on PR #37493:
URL: https://github.com/apache/spark/pull/37493#issuecomment-1213128027

   > The semantics look possibly slightly different - tryLog catches NonFatal and logs an error. Is Throwable vs ControlThrowable the same, or close enough? probably.
   
   ControlThrowable is subclass of the Throwable,  It is usually used as a marker trait for flow control, where catch-all behaviour is required ControlThrowable  should be propagated.
   
   tryLog catching ControlThrowable will throw it otherwise logs an error, this is much consistent with `ExecutorAllocationManager.start`(except for the log level, but it's ok probably)
   
   It seems you may have said tryLogNonFatalError? probably. NonFatal extract non-fatal Throwables, Will not match fatal errors like ControlThrowable and other fatal throwable, it seems the semantics is different with `ExecutorAllocationManager.start`(NonFatal don't match ControlThrowable but `ExecutorAllocationManager.start` will throw it)
   


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

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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