Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/20039#discussion_r159075179
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/LiveListenerBus.scala ---
@@ -125,13 +128,39 @@ private[spark] class LiveListenerBus(conf: SparkConf)
{
/** Post an event to all queues. */
def post(event: SparkListenerEvent): Unit = {
- if (!stopped.get()) {
- metrics.numEventsPosted.inc()
- val it = queues.iterator()
- while (it.hasNext()) {
- it.next().post(event)
+ if (stopped.get()) {
+ return
+ }
+
+ metrics.numEventsPosted.inc()
+
+ // If the event buffer is null, it means the bus has been started and
we can avoid
+ // synchronization and post events directly to the queues. This should
be the most
+ // common case during the life of the bus.
+ if (queuedEvents == null) {
+ postToQueues(event)
--- End diff --
I don't think this would help at all. IIUC, you're saying that its
possible that a `post()` and a `stop()` are racing against each other, and we
might post to a queue after its been stopped. But thats OK -- the queues are
prepared to deal with this.
Its also possible that during that race, the event makes it one queue
before that queue is stopped, but to another queue after its stopped. Which
means that final event only makes it to some queues. Again, I think that is
fine, and your change wouldn't help anyway, that would still be possible.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
