{"action":"created","issue":{"url":"https://api.github.com/repos/apache/spark/issues/23355","repository_url":"https://api.github.com/repos/apache/spark","labels_url":"https://api.github.com/repos/apache/spark/issues/23355/labels{/name}","comments_url":"https://api.github.com/repos/apache/spark/issues/23355/comments","events_url":"https://api.github.com/repos/apache/spark/issues/23355/events","html_url":"https://github.com/apache/spark/pull/23355","id":392937854,"node_id":"MDExOlB1bGxSZXF1ZXN0MjQwMDg3NjEy","number":23355,"title":"[SPARK-26418][SHUFFLE] Only OpenBlocks without any ChunkFetch for one stream will cause memory leak in ExternalShuffleService","user":{"login":"wangshuo128","id":4003322,"node_id":"MDQ6VXNlcjQwMDMzMjI=","avatar_url":"https://avatars0.githubusercontent.com/u/4003322?v=4","gravatar_id":"","url":"https://api.github.com/users/wangshuo128","html_url":"https://github.com/wangshuo128","followers_url":"https://api.github.com/users/wangshuo128/followers","following_url":"https://api.github.com/users/wangshuo128/following{/other_user}","gists_url":"https://api.github.com/users/wangshuo128/gists{/gist_id}","starred_url":"https://api.github.com/users/wangshuo128/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wangshuo128/subscriptions","organizations_url":"https://api.github.com/users/wangshuo128/orgs","repos_url":"https://api.github.com/users/wangshuo128/repos","events_url":"https://api.github.com/users/wangshuo128/events{/privacy}","received_events_url":"https://api.github.com/users/wangshuo128/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2018-12-20T08:26:53Z","updated_at":"2018-12-21T02:35:40Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/apache/spark/pulls/23355","html_url":"https://github.com/apache/spark/pull/23355","diff_url":"https://github.com/apache/spark/pull/23355.diff","patch_url":"https://github.com/apache/spark/pull/23355.patch"},"body":"## What changes were proposed in this pull request?\r\n\r\nIn current code path,  OneForOneStreamManager holds StreamState in a Map named streams. \r\nA StreamState is initialized and put into streams when OpenBlocks request received.\r\nOne specific StreamState is removed from streams in two scenarios below:\r\n1. The last chunk of a stream is fetched\r\n2. The connection of ChunkFetch is closed\r\n\r\nStreamState will never be clean up, if OpenBlocks request is received without any following  ChunkFetch request. This will cause memory leak in server side, which is harmful for long running service such as ExternalShuffleService.\r\nCould we call StreamManager#registerChannel to associate the channel with StreamState when server handles OpenBlocks request? \r\n\r\n## How was this patch tested?\r\nNew test added.\r\n"},"comment":{"url":"https://api.github.com/repos/apache/spark/issues/comments/449220765","html_url":"https://github.com/apache/spark/pull/23355#issuecomment-449220765","issue_url":"https://api.github.com/repos/apache/spark/issues/23355","id":449220765,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0OTIyMDc2NQ==","user":{"login":"beliefer","id":8486025,"node_id":"MDQ6VXNlcjg0ODYwMjU=","avatar_url":"https://avatars0.githubusercontent.com/u/8486025?v=4","gravatar_id":"","url":"https://api.github.com/users/beliefer","html_url":"https://github.com/beliefer","followers_url":"https://api.github.com/users/beliefer/followers","following_url":"https://api.github.com/users/beliefer/following{/other_user}","gists_url":"https://api.github.com/users/beliefer/gists{/gist_id}","starred_url":"https://api.github.com/users/beliefer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/beliefer/subscriptions","organizations_url":"https://api.github.com/users/beliefer/orgs","repos_url":"https://api.github.com/users/beliefer/repos","events_url":"https://api.github.com/users/beliefer/events{/privacy}","received_events_url":"https://api.github.com/users/beliefer/received_events","type":"User","site_admin":false},"created_at":"2018-12-21T02:35:40Z","updated_at":"2018-12-21T02:35:40Z","author_association":"NONE","body":"I check `OneForOneBlockFetcher` again, the method `start` use the same `TransportClient` to sent message `OpenBlocks`  and following message `FetchChunkRequest`.There have no chance to call `TransportClientFactory.createClient`. So I thind your change of code is OK, but still propose to add a parameter to control some timeout of `Channel`.When the time expired,call the following code\r\n```\r\nif (state.associatedChannel == channel) {\r\n  streams.remove(entry.getKey());\r\n\r\n  // Release all remaining buffers.\r\n  while (state.buffers.hasNext()) {\r\n      state.buffers.next().release();\r\n  }\r\n}\r\n```"}}
{"actor":{"display_login":"beliefer"}}