Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/4869#issuecomment-77032669
@JoshRosen and I discussed more about this offline. `sc.stop()` actually
will not hang indefinitely because we use the Akka timeout when awaiting the
`BlockManager*` messages. Also, it is semantically more correct to wait for
clean up to finish properly before potentially terminating the JVM abruptly
(cleaner thread is a daemon). Otherwise, an application may have leftover state
that we never clean up, and this state may pile up over time.
We considered a few alternatives:
(1) Use a global broadcast ID counter per JVM instead of per application.
This ensures no conflict in broadcast ID spaces across applications. This
doesn't actually fix the root cause of the problem, which is that the context
cleaner thread is still leaked across applications. This also changes the
semantics of the broadcast ID, and this may not be a safe change to back port
to older branches.
(2) Refactor the broadcast factories to take in a `SparkEnv` instead of
getting it from `SparkEnv.get`. This requires breaking a developer API, which
precludes us from back porting any potential fix to older branches.
(3) Introduce some identifier in `SparkEnv`, and clean only if the env is
the one that we expect. This requires passing the identifier everywhere in the
code and seems like an invasive change. As with (1), it doesn't fix the root
cause.
In summary, we will merge this current patch as is since it fixes flaky
test suites that have been failing throughout the project and slowing
development.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
