GitHub PullRequestReviewCommentEvent at 2014-04-04 17:44:48
Repository: tachyon
Actor: haoyuan
Payload: {"comment":{"url":"https://api.github.com/repos/amplab/tachyon/pulls/comments/11305106","id":11305106,"diff_hunk":"@@ -363,52 +367,69 @@ int _createFile(boolean recursive, String path, boolean directory, long blockSiz\n     LOG.debug(\"createFile\" + CommonUtils.parametersToString(path));\n \n     String[] pathNames = CommonUtils.getPathComponents(path);\n+    String name = pathNames[pathNames.length - 1];\n \n-    synchronized (mRoot) {\n-      Inode inode = getInode(pathNames);\n-      if (inode != null) {\n-        LOG.info(\"FileAlreadyExistException: File \" + path + \" already exist.\");\n-        throw new FileAlreadyExistException(\"Path \" + path + \" already exist.\");\n-      }\n-\n-      String name = pathNames[pathNames.length - 1];\n-      String folderPath = null;\n-      if (path.length() - name.length() == 1) {\n-        folderPath = path.substring(0, path.length() - name.length());\n-      } else {\n-        folderPath = path.substring(0, path.length() - name.length() - 1);\n-      }\n-      inode = getInode(folderPath);\n-      if (inode == null) {\n-        int succeed = 0;\n-        if (recursive) {\n-          succeed = createFile(true, folderPath, true, blockSizeByte);\n-        }\n-        if (!recursive || succeed <= 0) {\n-          LOG.info(\"InvalidPathException: File \" + path + \" creation failed. Folder \" + folderPath\n-              + \" does not exist.\");\n-          throw new InvalidPathException(\"InvalidPathException: File \" + path + \" creation \"\n-              + \"failed. Folder \" + folderPath + \" does not exist.\");\n+    String[] folderPath = new String[pathNames.length - 1];\n+    System.arraycopy(pathNames, 0, folderPath, 0, folderPath.length);\n+    InodeLocks inodeLocks = getInodeWithLocks(folderPath, true);\n+    try {\n+      // pathInd is the index into pathNames where we start filling in the path from the inode.\n+      int pathInd = folderPath.length;\n+      if (inodeLocks.getNonexistentInd() >= 0) {\n+        // Then the path component at errorInd k doesn't exist. If it's not recursive, we throw an\n+        // exception here. Otherwise we add the remaining path components to the list of components\n+        // to create.\n+        if (!recursive) {\n+          final String msg =\n+              \"File \" + path + \" creation failed. Component \" + inodeLocks.getNonexistentInd()\n+                  + \"(\" + folderPath[inodeLocks.getNonexistentInd()] + \") does not exist\";\n+          LOG.info(\"InvalidPathException: \" + msg);\n+          throw new InvalidPathException(msg);\n         } else {\n-          inode = mInodes.get(succeed);\n+          // The lock at errorInd-1 was the last lock taken, but it was only readLocked. Since we'll\n+          // be modifying inodeLocks.getInode(), we need to upgrade the lock to a write lock. Since\n+          // another write lock could have been taken and created the file we want during the\n+          // upgrade, the file we are trying to create may already exist. In that case, we'll just\n+          // throw a FileAlreadyExistsException.\n+          upgradeLock(inodeLocks.getLocks()[inodeLocks.getNonexistentInd() - 1]);\n+          inodeLocks.setIsWrite(true);\n+          // We will start filling in the path from inodeLocks.getNonexistentInd()\n+          pathInd = inodeLocks.getNonexistentInd();\n         }\n-      } else if (inode.isFile()) {\n-        LOG.info(\"InvalidPathException: File \" + path + \" creation failed. \" + folderPath\n-            + \" is a file.\");\n-        throw new InvalidPathException(\"File \" + path + \" creation failed. \" + folderPath\n-            + \" is a file\");\n       }\n \n-      Inode ret = null;\n+      InodeFolder cur = (InodeFolder) inodeLocks.getInode();","path":"main/src/main/java/tachyon/master/MasterInfo.java","position":107,"original_position":107,"commit_id":"82fae7ad586ec791753d6a4081285bb43ffa3e14","original_commit_id":"82fae7ad586ec791753d6a4081285bb43ffa3e14","user":{"login":"haoyuan","id":1200956,"avatar_url":"https://avatars.githubusercontent.com/u/1200956?","gravatar_id":"e0ad06a984399d0a01dcdc622fb45d39","url":"https://api.github.com/users/haoyuan","html_url":"https://github.com/haoyuan","followers_url":"https://api.github.com/users/haoyuan/followers","following_url":"https://api.github.com/users/haoyuan/following{/other_user}","gists_url":"https://api.github.com/users/haoyuan/gists{/gist_id}","starred_url":"https://api.github.com/users/haoyuan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/haoyuan/subscriptions","organizations_url":"https://api.github.com/users/haoyuan/orgs","repos_url":"https://api.github.com/users/haoyuan/repos","events_url":"https://api.github.com/users/haoyuan/events{/privacy}","received_events_url":"https://api.github.com/users/haoyuan/received_events","type":"User","site_admin":false},"body":"Does the code guarantee that the inodeLocks.getInode() returns an InodeFolder? Is it possible to be an InodeFile?","created_at":"2014-04-04T17:44:48Z","updated_at":"2014-04-04T17:44:48Z","html_url":"https://github.com/amplab/tachyon/pull/154#discussion_r11305106","pull_request_url":"https://api.github.com/repos/amplab/tachyon/pulls/154","_links":{"self":{"href":"https://api.github.com/repos/amplab/tachyon/pulls/comments/11305106"},"html":{"href":"https://github.com/amplab/tachyon/pull/154#discussion_r11305106"},"pull_request":{"href":"https://api.github.com/repos/amplab/tachyon/pulls/154"}}}}