GitHub PullRequestReviewCommentEvent at 2014-12-19 01:08:11
Repository: tachyon
Actor: ShaoshanLiu
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/amplab/tachyon/pulls/comments/22084343","id":22084343,"diff_hunk":"@@ -72,6 +72,21 @@\n   private Object mUFSConf = null;\n \n   /**\n+  * The object for synchronization to avoid race conditions between close() and doneRecache()\n+  */\n+  private Object mFlagDoneRecache = null;","path":"core/src/main/java/tachyon/client/RemoteBlockInStream.java","position":6,"original_position":6,"commit_id":"b51ae064256e5e4fec16e48d6bada331ea555e82","original_commit_id":"b51ae064256e5e4fec16e48d6bada331ea555e82","user":{"login":"ShaoshanLiu","id":6655780,"avatar_url":"https://avatars.githubusercontent.com/u/6655780?v=3","gravatar_id":"","url":"https://api.github.com/users/ShaoshanLiu","html_url":"https://github.com/ShaoshanLiu","followers_url":"https://api.github.com/users/ShaoshanLiu/followers","following_url":"https://api.github.com/users/ShaoshanLiu/following{/other_user}","gists_url":"https://api.github.com/users/ShaoshanLiu/gists{/gist_id}","starred_url":"https://api.github.com/users/ShaoshanLiu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ShaoshanLiu/subscriptions","organizations_url":"https://api.github.com/users/ShaoshanLiu/orgs","repos_url":"https://api.github.com/users/ShaoshanLiu/repos","events_url":"https://api.github.com/users/ShaoshanLiu/events{/privacy}","received_events_url":"https://api.github.com/users/ShaoshanLiu/received_events","type":"User","site_admin":false},"body":"well, it saves a little bit of memory when mRecache is not used.  But sure, I will change it to final as you suggested. ","created_at":"2014-12-19T01:08:11Z","updated_at":"2014-12-19T01:08:11Z","html_url":"https://github.com/amplab/tachyon/pull/574#discussion_r22084343","pull_request_url":"https://api.github.com/repos/amplab/tachyon/pulls/574","_links":{"self":{"href":"https://api.github.com/repos/amplab/tachyon/pulls/comments/22084343"},"html":{"href":"https://github.com/amplab/tachyon/pull/574#discussion_r22084343"},"pull_request":{"href":"https://api.github.com/repos/amplab/tachyon/pulls/574"}}},"pull_request":{"url":"https://api.github.com/repos/amplab/tachyon/pulls/574","id":26186671,"html_url":"https://github.com/amplab/tachyon/pull/574","diff_url":"https://github.com/amplab/tachyon/pull/574.diff","patch_url":"https://github.com/amplab/tachyon/pull/574.patch","issue_url":"https://api.github.com/repos/amplab/tachyon/issues/574","number":574,"state":"open","locked":false,"title":"FIX ISSUE TACHYON-251: race condition between RemoteBlockInStream close() and doneReCache()","user":{"login":"ShaoshanLiu","id":6655780,"avatar_url":"https://avatars.githubusercontent.com/u/6655780?v=3","gravatar_id":"","url":"https://api.github.com/users/ShaoshanLiu","html_url":"https://github.com/ShaoshanLiu","followers_url":"https://api.github.com/users/ShaoshanLiu/followers","following_url":"https://api.github.com/users/ShaoshanLiu/following{/other_user}","gists_url":"https://api.github.com/users/ShaoshanLiu/gists{/gist_id}","starred_url":"https://api.github.com/users/ShaoshanLiu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ShaoshanLiu/subscriptions","organizations_url":"https://api.github.com/users/ShaoshanLiu/orgs","repos_url":"https://api.github.com/users/ShaoshanLiu/repos","events_url":"https://api.github.com/users/ShaoshanLiu/events{/privacy}","received_events_url":"https://api.github.com/users/ShaoshanLiu/received_events","type":"User","site_admin":false},"body":"https://tachyon.atlassian.net/browse/TACHYON-251?filter=-1\r\n\r\nthere is a race condition in RemoteBlockInStream, between methods close() and DoneReCache().  When close() is called, currently it would cancel BlockOutStream if block out stream is not yet done writing to cache, leading to remote blocks not able to be cached. \r\n\r\nThe root cause of this problem is the lack of synchronization between close() and DoneReCache().  In this change, we added synchronization mechanisms between this two methods such that:\r\n1. when close() is called, it waits for recache to be done with a timeout of 5 seconds\r\n2. if recache is not done when timeout expires, it would fall back to the original logic to cancel BlockOutStream.\r\n3. otherwise, we make sure that remote blocks are properly cached.\r\n\r\n\r\nPerformance:\r\n\r\nBefore Change:\r\nQuery Time taken: 160.786 seconds\r\nQuery Time taken: 131.347 seconds\r\n\r\n\r\nAfter Change:\r\nQuery Time taken: 5.9 seconds\r\nQuery Time taken: 5.891 seconds\r\n\r\n\r\nTest Done Locally:\r\nit has passed all unit tests locally with \"mvn install\"\r\n","created_at":"2014-12-16T22:45:03Z","updated_at":"2014-12-19T01:08:11Z","closed_at":null,"merged_at":null,"merge_commit_sha":"8f941c37c9f47953fd93d66849eb069cc7e1628e","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/amplab/tachyon/pulls/574/commits","review_comments_url":"https://api.github.com/repos/amplab/tachyon/pulls/574/comments","review_comment_url":"https://api.github.com/repos/amplab/tachyon/pulls/comments/{number}","comments_url":"https://api.github.com/repos/amplab/tachyon/issues/574/comments","statuses_url":"https://api.github.com/repos/amplab/tachyon/statuses/b51ae064256e5e4fec16e48d6bada331ea555e82","head":{"label":"ShaoshanLiu:master","ref":"master","sha":"b51ae064256e5e4fec16e48d6bada331ea555e82","user":{"login":"ShaoshanLiu","id":6655780,"avatar_url":"https://avatars.githubusercontent.com/u/6655780?v=3","gravatar_id":"","url":"https://api.github.com/users/ShaoshanLiu","html_url":"https://github.com/ShaoshanLiu","followers_url":"https://api.github.com/users/ShaoshanLiu/followers","following_url":"https://api.github.com/users/ShaoshanLiu/following{/other_user}","gists_url":"https://api.github.com/users/ShaoshanLiu/gists{/gist_id}","starred_url":"https://api.github.com/users/ShaoshanLiu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ShaoshanLiu/subscriptions","organizations_url":"https://api.github.com/users/ShaoshanLiu/orgs","repos_url":"https://api.github.com/users/ShaoshanLiu/repos","events_url":"https://api.github.com/users/ShaoshanLiu/events{/privacy}","received_events_url":"https://api.github.com/users/ShaoshanLiu/received_events","type":"User","site_admin":false},"repo":{"id":26831858,"name":"tachyon","full_name":"ShaoshanLiu/tachyon","owner":{"login":"ShaoshanLiu","id":6655780,"avatar_url":"https://avatars.githubusercontent.com/u/6655780?v=3","gravatar_id":"","url":"https://api.github.com/users/ShaoshanLiu","html_url":"https://github.com/ShaoshanLiu","followers_url":"https://api.github.com/users/ShaoshanLiu/followers","following_url":"https://api.github.com/users/ShaoshanLiu/following{/other_user}","gists_url":"https://api.github.com/users/ShaoshanLiu/gists{/gist_id}","starred_url":"https://api.github.com/users/ShaoshanLiu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ShaoshanLiu/subscriptions","organizations_url":"https://api.github.com/users/ShaoshanLiu/orgs","repos_url":"https://api.github.com/users/ShaoshanLiu/repos","events_url":"https://api.github.com/users/ShaoshanLiu/events{/privacy}","received_events_url":"https://api.github.com/users/ShaoshanLiu/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/ShaoshanLiu/tachyon","description":" A Reliable Memory Centric Distributed Storage System","fork":true,"url":"https://api.github.com/repos/ShaoshanLiu/tachyon","forks_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/forks","keys_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/teams","hooks_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/hooks","issue_events_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/issues/events{/number}","events_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/events","assignees_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/assignees{/user}","branches_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/branches{/branch}","tags_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/tags","blobs_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/git/refs{/sha}","trees_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/statuses/{sha}","languages_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/languages","stargazers_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/stargazers","contributors_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/contributors","subscribers_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/subscribers","subscription_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/subscription","commits_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/commits{/sha}","git_commits_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/git/commits{/sha}","comments_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/comments{/number}","issue_comment_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/issues/comments/{number}","contents_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/contents/{+path}","compare_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/merges","archive_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/downloads","issues_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/issues{/number}","pulls_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/pulls{/number}","milestones_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/milestones{/number}","notifications_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/labels{/name}","releases_url":"https://api.github.com/repos/ShaoshanLiu/tachyon/releases{/id}","created_at":"2014-11-18T21:54:37Z","updated_at":"2014-12-17T19:05:02Z","pushed_at":"2014-12-17T19:05:01Z","git_url":"git://github.com/ShaoshanLiu/tachyon.git","ssh_url":"git@github.com:ShaoshanLiu/tachyon.git","clone_url":"https://github.com/ShaoshanLiu/tachyon.git","svn_url":"https://github.com/ShaoshanLiu/tachyon","homepage":"http://tachyon-project.org","size":8819,"stargazers_count":0,"watchers_count":0,"language":"Java","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":1,"mirror_url":null,"open_issues_count":0,"forks":1,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"amplab:master","ref":"master","sha":"ea7d2492030215ff2edb1c9a9ade1c3d2bd5d55e","user":{"login":"amplab","id":1438454,"avatar_url":"https://avatars.githubusercontent.com/u/1438454?v=3","gravatar_id":"","url":"https://api.github.com/users/amplab","html_url":"https://github.com/amplab","followers_url":"https://api.github.com/users/amplab/followers","following_url":"https://api.github.com/users/amplab/following{/other_user}","gists_url":"https://api.github.com/users/amplab/gists{/gist_id}","starred_url":"https://api.github.com/users/amplab/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/amplab/subscriptions","organizations_url":"https://api.github.com/users/amplab/orgs","repos_url":"https://api.github.com/users/amplab/repos","events_url":"https://api.github.com/users/amplab/events{/privacy}","received_events_url":"https://api.github.com/users/amplab/received_events","type":"Organization","site_admin":false},"repo":{"id":7276954,"name":"tachyon","full_name":"amplab/tachyon","owner":{"login":"amplab","id":1438454,"avatar_url":"https://avatars.githubusercontent.com/u/1438454?v=3","gravatar_id":"","url":"https://api.github.com/users/amplab","html_url":"https://github.com/amplab","followers_url":"https://api.github.com/users/amplab/followers","following_url":"https://api.github.com/users/amplab/following{/other_user}","gists_url":"https://api.github.com/users/amplab/gists{/gist_id}","starred_url":"https://api.github.com/users/amplab/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/amplab/subscriptions","organizations_url":"https://api.github.com/users/amplab/orgs","repos_url":"https://api.github.com/users/amplab/repos","events_url":"https://api.github.com/users/amplab/events{/privacy}","received_events_url":"https://api.github.com/users/amplab/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/amplab/tachyon","description":" A Reliable Memory Centric Distributed Storage System","fork":false,"url":"https://api.github.com/repos/amplab/tachyon","forks_url":"https://api.github.com/repos/amplab/tachyon/forks","keys_url":"https://api.github.com/repos/amplab/tachyon/keys{/key_id}","collaborators_url":"https://api.github.com/repos/amplab/tachyon/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/amplab/tachyon/teams","hooks_url":"https://api.github.com/repos/amplab/tachyon/hooks","issue_events_url":"https://api.github.com/repos/amplab/tachyon/issues/events{/number}","events_url":"https://api.github.com/repos/amplab/tachyon/events","assignees_url":"https://api.github.com/repos/amplab/tachyon/assignees{/user}","branches_url":"https://api.github.com/repos/amplab/tachyon/branches{/branch}","tags_url":"https://api.github.com/repos/amplab/tachyon/tags","blobs_url":"https://api.github.com/repos/amplab/tachyon/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/amplab/tachyon/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/amplab/tachyon/git/refs{/sha}","trees_url":"https://api.github.com/repos/amplab/tachyon/git/trees{/sha}","statuses_url":"https://api.github.com/repos/amplab/tachyon/statuses/{sha}","languages_url":"https://api.github.com/repos/amplab/tachyon/languages","stargazers_url":"https://api.github.com/repos/amplab/tachyon/stargazers","contributors_url":"https://api.github.com/repos/amplab/tachyon/contributors","subscribers_url":"https://api.github.com/repos/amplab/tachyon/subscribers","subscription_url":"https://api.github.com/repos/amplab/tachyon/subscription","commits_url":"https://api.github.com/repos/amplab/tachyon/commits{/sha}","git_commits_url":"https://api.github.com/repos/amplab/tachyon/git/commits{/sha}","comments_url":"https://api.github.com/repos/amplab/tachyon/comments{/number}","issue_comment_url":"https://api.github.com/repos/amplab/tachyon/issues/comments/{number}","contents_url":"https://api.github.com/repos/amplab/tachyon/contents/{+path}","compare_url":"https://api.github.com/repos/amplab/tachyon/compare/{base}...{head}","merges_url":"https://api.github.com/repos/amplab/tachyon/merges","archive_url":"https://api.github.com/repos/amplab/tachyon/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/amplab/tachyon/downloads","issues_url":"https://api.github.com/repos/amplab/tachyon/issues{/number}","pulls_url":"https://api.github.com/repos/amplab/tachyon/pulls{/number}","milestones_url":"https://api.github.com/repos/amplab/tachyon/milestones{/number}","notifications_url":"https://api.github.com/repos/amplab/tachyon/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/amplab/tachyon/labels{/name}","releases_url":"https://api.github.com/repos/amplab/tachyon/releases{/id}","created_at":"2012-12-21T17:43:46Z","updated_at":"2014-12-18T21:34:56Z","pushed_at":"2014-12-18T21:34:55Z","git_url":"git://github.com/amplab/tachyon.git","ssh_url":"git@github.com:amplab/tachyon.git","clone_url":"https://github.com/amplab/tachyon.git","svn_url":"https://github.com/amplab/tachyon","homepage":"http://tachyon-project.org","size":25507,"stargazers_count":1099,"watchers_count":1099,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":362,"mirror_url":null,"open_issues_count":23,"forks":362,"open_issues":23,"watchers":1099,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/amplab/tachyon/pulls/574"},"html":{"href":"https://github.com/amplab/tachyon/pull/574"},"issue":{"href":"https://api.github.com/repos/amplab/tachyon/issues/574"},"comments":{"href":"https://api.github.com/repos/amplab/tachyon/issues/574/comments"},"review_comments":{"href":"https://api.github.com/repos/amplab/tachyon/pulls/574/comments"},"review_comment":{"href":"https://api.github.com/repos/amplab/tachyon/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/amplab/tachyon/pulls/574/commits"},"statuses":{"href":"https://api.github.com/repos/amplab/tachyon/statuses/b51ae064256e5e4fec16e48d6bada331ea555e82"}}}}