[GitHub] [spark] Ngone51 edited a comment on pull request #28629: [SPARK-31769][CORE] Add MDC support for driver threads

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

Ngone51 edited a comment on pull request #28629:
URL: https://github.com/apache/spark/pull/28629#issuecomment-653385789


   I'm fine to remove the prefix if we want to inherit the MDC properties directly since I agree API consistent is more important. And I think we need to document it clearly that which MDC users should use as Spark actually has `org.slf4j.MDC` and `org.apache.log4j.MDC`(though, `org.apache.log4j.MDC` is implemented by `org.slf4j.MDC` by default).
   
   Besides, I have two questions:
   
   I just realize that MDC is actually backed by the `InheritableThreadLocal`. That's why when we set up MDC properties in the main thread, those thread pools could also get the updated properties. I can also imagine how this works for the case mentioned above(job server) when jobs come in sequence. For example, we could clean up the MDC properties after one Job finishes and sets up the new MDC properties for the next job. However, I'm just wondering how it works if jobs come in concurrently. MDC properties could mess up in this way, right?
   
   And it seems this PR also can't work with the loop running runnable, e.g.  `MessageLoop.receiveLoopRunnable`. This kind of runnable can only get the MDC properties at the time it was created. That also means it can only reflect the MDC properties before the initialization of `SparkContext`. So I'm not sure if it could work for jobs that are created after the SparkContext.


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



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