{"action":"created","issue":{"url":"https://api.github.com/repos/apache/spark/issues/14396","repository_url":"https://api.github.com/repos/apache/spark","labels_url":"https://api.github.com/repos/apache/spark/issues/14396/labels{/name}","comments_url":"https://api.github.com/repos/apache/spark/issues/14396/comments","events_url":"https://api.github.com/repos/apache/spark/issues/14396/events","html_url":"https://github.com/apache/spark/pull/14396","id":168250608,"number":14396,"title":"[SPARK-16787] SparkContext.addFile() should not throw if called twice with the same file","user":{"login":"JoshRosen","id":50748,"avatar_url":"https://avatars.githubusercontent.com/u/50748?v=3","gravatar_id":"","url":"https://api.github.com/users/JoshRosen","html_url":"https://github.com/JoshRosen","followers_url":"https://api.github.com/users/JoshRosen/followers","following_url":"https://api.github.com/users/JoshRosen/following{/other_user}","gists_url":"https://api.github.com/users/JoshRosen/gists{/gist_id}","starred_url":"https://api.github.com/users/JoshRosen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JoshRosen/subscriptions","organizations_url":"https://api.github.com/users/JoshRosen/orgs","repos_url":"https://api.github.com/users/JoshRosen/repos","events_url":"https://api.github.com/users/JoshRosen/events{/privacy}","received_events_url":"https://api.github.com/users/JoshRosen/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":7,"created_at":"2016-07-29T04:55:35Z","updated_at":"2016-07-29T22:54:08Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/apache/spark/pulls/14396","html_url":"https://github.com/apache/spark/pull/14396","diff_url":"https://github.com/apache/spark/pull/14396.diff","patch_url":"https://github.com/apache/spark/pull/14396.patch"},"body":"## What changes were proposed in this pull request?\r\n\r\nThe behavior of `SparkContext.addFile()` changed slightly with the introduction of the Netty-RPC-based file server, which was introduced in Spark 1.6 (where it was disabled by default) and became the default / only file server in Spark 2.0.0.\r\n\r\nPrior to 2.0, calling `SparkContext.addFile()` with files that have the same name and identical contents would succeed. This behavior was never explicitly documented but Spark has behaved this way since very early 1.x versions.\r\n\r\nIn 2.0 (or 1.6 with the Netty file server enabled), the second `addFile()` call will fail with a requirement error because NettyStreamManager tries to guard against duplicate file registration.\r\n\r\nThis problem also affects `addJar()` in a more subtle way: the `fileServer.addJar()` call will also fail with an exception but that exception is logged and ignored; I believe that the problematic exception-catching path was mistakenly copied from some old code which was only relevant to very old versions of Spark and YARN mode.\r\n\r\nI believe that this change of behavior was unintentional, so this patch weakens the `require` check so that adding the same filename at the same path will succeed.\r\n\r\nAt file download time, Spark tasks will fail with exceptions if an executor already has a local copy of a file and that file's contents do not match the contents of the file being downloaded / added. As a result, it's important that we prevent files with the same name and different contents from being served because allowing that can effectively brick an executor by preventing it from successfully launching any new tasks. Before this patch's change, this was prevented by forbidding `addFile()` from being called twice on files with the same name. Because Spark does not defensively copy local files that are passed to `addFile` it is vulnerable to files' contents changing, so I think it's okay to rely on an implicit assumption that these files are intended to be immutable (since if they _are_ mutable then this can lead to either explicit task failures or implicit incorrectness (in case new executors silently get newer copies of the file while old executors continue to use an older version)). To guard against this, I have decided to only update the file addition timestamps on the first call to `addFile()`; duplicate calls will succeed but will not update the timestamp. This behavior is fine as long as we assume files are immutable, which seems reasonable given the behaviors described above.\r\n\r\nAs part of this change, I also improved the thread-safety of the `addedJars` and `addedFiles` maps; this is important because these maps may be concurrently read by a task launching thread and written by a driver thread in case the user's driver code is multi-threaded. \r\n\r\n## How was this patch tested?\r\n\r\nI added regression tests in `SparkContextSuite`."},"comment":{"url":"https://api.github.com/repos/apache/spark/issues/comments/236313459","html_url":"https://github.com/apache/spark/pull/14396#issuecomment-236313459","issue_url":"https://api.github.com/repos/apache/spark/issues/14396","id":236313459,"user":{"login":"SparkQA","id":8108735,"avatar_url":"https://avatars.githubusercontent.com/u/8108735?v=3","gravatar_id":"","url":"https://api.github.com/users/SparkQA","html_url":"https://github.com/SparkQA","followers_url":"https://api.github.com/users/SparkQA/followers","following_url":"https://api.github.com/users/SparkQA/following{/other_user}","gists_url":"https://api.github.com/users/SparkQA/gists{/gist_id}","starred_url":"https://api.github.com/users/SparkQA/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SparkQA/subscriptions","organizations_url":"https://api.github.com/users/SparkQA/orgs","repos_url":"https://api.github.com/users/SparkQA/repos","events_url":"https://api.github.com/users/SparkQA/events{/privacy}","received_events_url":"https://api.github.com/users/SparkQA/received_events","type":"User","site_admin":false},"created_at":"2016-07-29T22:54:08Z","updated_at":"2016-07-29T22:54:08Z","body":"**[Test build #63024 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63024/consoleFull)** for PR 14396 at commit [`9aa32b3`](https://github.com/apache/spark/commit/9aa32b394cd80c776ac567a662c9471351228b93)."}}
{"actor":{"display_login":"SparkQA"}}