Skip to content

Commit

Permalink
Use InodePath for more methods in InodeTree
Browse files Browse the repository at this point in the history
  • Loading branch information
@gpang
gpang committed May 15, 2016
1 parent fee7e68 commit d3e6764
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 87 deletions.
30 changes: 11 additions & 19 deletions core/server/src/main/java/alluxio/master/file/FileSystemMaster.java
View file
Expand Up @@ -663,16 +663,12 @@ long createFileAndJournal(InodePath inodePath, CreateFileOptions options)
InodeTree.CreatePathResult createResult = createFileInternal(inodePath, options); InodeTree.CreatePathResult createResult = createFileInternal(inodePath, options);
// TODO(gpang): let create path take InodePath (and update it with correct locks). // TODO(gpang): let create path take InodePath (and update it with correct locks).
mInodeTree.ensureFullInodePath(inodePath, InodeTree.LockMode.WRITE); mInodeTree.ensureFullInodePath(inodePath, InodeTree.LockMode.WRITE);
try { List<Inode<?>> created = createResult.getCreated();
List<Inode<?>> created = createResult.getCreated();


writeJournalEntry(mDirectoryIdGenerator.toJournalEntry()); writeJournalEntry(mDirectoryIdGenerator.toJournalEntry());
journalCreatePathResult(createResult); journalCreatePathResult(createResult);
flushJournal(); flushJournal();
return created.get(created.size() - 1).getId(); return created.get(created.size() - 1).getId();
} finally {
createResult.unlock();
}
} }


/** /**
Expand Down Expand Up @@ -1178,15 +1174,11 @@ InodeTree.CreatePathResult createDirectoryAndJournal(InodePath inodePath,
InodeTree.CreatePathResult createResult = createDirectoryInternal(inodePath, options); InodeTree.CreatePathResult createResult = createDirectoryInternal(inodePath, options);
// TODO(gpang): let create path take InodePath (and update it with correct locks). // TODO(gpang): let create path take InodePath (and update it with correct locks).
mInodeTree.ensureFullInodePath(inodePath, InodeTree.LockMode.WRITE); mInodeTree.ensureFullInodePath(inodePath, InodeTree.LockMode.WRITE);
try { writeJournalEntry(mDirectoryIdGenerator.toJournalEntry());
writeJournalEntry(mDirectoryIdGenerator.toJournalEntry()); journalCreatePathResult(createResult);
journalCreatePathResult(createResult); flushJournal();
flushJournal(); MasterContext.getMasterSource().incDirectoriesCreated(1);
MasterContext.getMasterSource().incDirectoriesCreated(1); return createResult;
return createResult;
} finally {
createResult.unlock();
}
} }


/** /**
Expand Down Expand Up @@ -2194,7 +2186,7 @@ void setAttributeInternal(InodePath inodePath, long opTimeMs, SetAttributeOption
throws FileDoesNotExistException { throws FileDoesNotExistException {
Inode<?> inode = inodePath.getInode(); Inode<?> inode = inodePath.getInode();
if (options.getPinned() != null) { if (options.getPinned() != null) {
mInodeTree.setPinned(inode, options.getPinned(), opTimeMs); mInodeTree.setPinned(inodePath, options.getPinned(), opTimeMs);
inode.setLastModificationTimeMs(opTimeMs); inode.setLastModificationTimeMs(opTimeMs);
} }
if (options.getTtl() != null) { if (options.getTtl() != null) {
Expand Down
56 changes: 20 additions & 36 deletions core/server/src/main/java/alluxio/master/file/meta/InodeTree.java
View file
Expand Up @@ -182,22 +182,6 @@ public boolean inodePathExists(AlluxioURI path) {
} }
} }


/**
* @param path the path to get the inode for
* @return the inode with the given path
* @throws InvalidPathException if the path is invalid
* @throws FileDoesNotExistException if the path does not exist
*/
public Inode<?> getInodeByPath(AlluxioURI path)
throws InvalidPathException, FileDoesNotExistException {
TraversalResult traversalResult =
traverseToInode(PathUtils.getPathComponents(path.getPath()), LockMode.READ);
if (!traversalResult.isFound()) {
throw new FileDoesNotExistException(ExceptionMessage.PATH_DOES_NOT_EXIST.getMessage(path));
}
return traversalResult.getInode();
}

public InodePath lockInodePath(AlluxioURI path, LockMode lockMode) throws InvalidPathException { public InodePath lockInodePath(AlluxioURI path, LockMode lockMode) throws InvalidPathException {
TraversalResult traversalResult = TraversalResult traversalResult =
traverseToInode(PathUtils.getPathComponents(path.getPath()), lockMode); traverseToInode(PathUtils.getPathComponents(path.getPath()), lockMode);
Expand Down Expand Up @@ -485,7 +469,7 @@ public CreatePathResult createPath(InodePath inodePath, CreatePathOptions<?> opt
} }


LOG.debug("createFile: File Created: {} parent: ", lastInode, currentInodeDirectory); LOG.debug("createFile: File Created: {} parent: ", lastInode, currentInodeDirectory);
return new CreatePathResult(modifiedInodes, createdInodes, existingNonPersisted, lockGroup); return new CreatePathResult(modifiedInodes, createdInodes, existingNonPersisted);
} }


/** /**
Expand Down Expand Up @@ -558,11 +542,14 @@ public void deleteInode(Inode<?> inode) throws FileDoesNotExistException {
* Sets the pinned state of an inode. If the inode is a directory, the pinned state will be set * Sets the pinned state of an inode. If the inode is a directory, the pinned state will be set
* recursively. * recursively.
* *
* @param inode the {@link Inode} to set the pinned state for * @param inodePath the {@link InodePath} to set the pinned state for
* @param pinned the pinned state to set for the inode (and possible descendants) * @param pinned the pinned state to set for the inode (and possible descendants)
* @param opTimeMs the operation time * @param opTimeMs the operation time
* @throws FileDoesNotExistException if inode does not exist
*/ */
public void setPinned(Inode<?> inode, boolean pinned, long opTimeMs) { public void setPinned(InodePath inodePath, boolean pinned, long opTimeMs)
throws FileDoesNotExistException {
Inode<?> inode = inodePath.getInode();
inode.setPinned(pinned); inode.setPinned(pinned);
inode.setLastModificationTimeMs(opTimeMs); inode.setLastModificationTimeMs(opTimeMs);


Expand All @@ -575,8 +562,15 @@ public void setPinned(Inode<?> inode, boolean pinned, long opTimeMs) {
} else { } else {
assert inode instanceof InodeDirectory; assert inode instanceof InodeDirectory;
// inode is a directory. Set the pinned state for all children. // inode is a directory. Set the pinned state for all children.
TempInodePathWithDescendant tempInodePath = new TempInodePathWithDescendant(inodePath);
for (Inode<?> child : ((InodeDirectory) inode).getChildren()) { for (Inode<?> child : ((InodeDirectory) inode).getChildren()) {
setPinned(child, pinned, opTimeMs); child.lockWrite();
try {
tempInodePath.setDescendant(child, getPath(child));
setPinned(tempInodePath, pinned, opTimeMs);
} finally {
child.unlockWrite();
}
} }
} }
} }
Expand All @@ -585,11 +579,12 @@ public void setPinned(Inode<?> inode, boolean pinned, long opTimeMs) {
* Sets the pinned state of an inode. If the inode is a directory, the pinned state will be set * Sets the pinned state of an inode. If the inode is a directory, the pinned state will be set
* recursively. * recursively.
* *
* @param inode the {@link Inode} to set the pinned state for * @param inodePath the {@link InodePath} to set the pinned state for
* @param pinned the pinned state to set for the inode (and possible descendants) * @param pinned the pinned state to set for the inode (and possible descendants)
* @throws FileDoesNotExistException if inode does not exist
*/ */
public void setPinned(Inode<?> inode, boolean pinned) { public void setPinned(InodePath inodePath, boolean pinned) throws FileDoesNotExistException {
setPinned(inode, pinned, System.currentTimeMillis()); setPinned(inodePath, pinned, System.currentTimeMillis());
} }


/** /**
Expand Down Expand Up @@ -709,6 +704,7 @@ private TraversalResult traverseToInode(String[] pathComponents, LockMode lockMo
List<Inode<?>> nonPersistedInodes = Lists.newArrayList(); List<Inode<?>> nonPersistedInodes = Lists.newArrayList();
List<Inode<?>> inodes = Lists.newArrayList(); List<Inode<?>> inodes = Lists.newArrayList();
InodeLockGroup lockGroup = new InodeLockGroup(); InodeLockGroup lockGroup = new InodeLockGroup();
// TODO(gpang): close lockGroup if any exception happens.


if (pathComponents == null) { if (pathComponents == null) {
throw new InvalidPathException("passed-in pathComponents is null"); throw new InvalidPathException("passed-in pathComponents is null");
Expand Down Expand Up @@ -896,22 +892,17 @@ public static final class CreatePathResult {
private final List<Inode<?>> mModified; private final List<Inode<?>> mModified;
private final List<Inode<?>> mCreated; private final List<Inode<?>> mCreated;
private final List<Inode<?>> mPersisted; private final List<Inode<?>> mPersisted;
private final InodeLockGroup mLockGroup;


/** /**
* Constructs the results of modified and created inodes when creating a path. * Constructs the results of modified and created inodes when creating a path.
* *
* @param modified a list of modified inodes * @param modified a list of modified inodes
* @param created a list of created inodes * @param created a list of created inodes
* @param lockGroup the {@link InodeLockGroup} managing locks for the inodes read, modified,
* and created
*/ */
CreatePathResult(List<Inode<?>> modified, List<Inode<?>> created, List<Inode<?>> persisted, CreatePathResult(List<Inode<?>> modified, List<Inode<?>> created, List<Inode<?>> persisted) {
InodeLockGroup lockGroup) {
mModified = Preconditions.checkNotNull(modified); mModified = Preconditions.checkNotNull(modified);
mCreated = Preconditions.checkNotNull(created); mCreated = Preconditions.checkNotNull(created);
mPersisted = Preconditions.checkNotNull(persisted); mPersisted = Preconditions.checkNotNull(persisted);
mLockGroup = lockGroup;
} }


/** /**
Expand All @@ -934,12 +925,5 @@ public List<Inode<?>> getCreated() {
public List<Inode<?>> getPersisted() { public List<Inode<?>> getPersisted() {
return mPersisted; return mPersisted;
} }

/**
* Unlocks all the locked inodes in this result.
*/
public void unlock() {
mLockGroup.unlock();
}
} }
} }
62 changes: 30 additions & 32 deletions core/server/src/test/java/alluxio/master/file/meta/InodeTreeTest.java
View file
Expand Up @@ -114,11 +114,11 @@ public static void beforeClass() throws Exception {
*/ */
@Test @Test
public void initializeRootTwiceTest() throws Exception { public void initializeRootTwiceTest() throws Exception {
Inode<?> root = mTree.getInodeByPath(new AlluxioURI("/")); Inode<?> root = getInodeByPath(mTree, new AlluxioURI("/"));
// initializeRoot call does nothing // initializeRoot call does nothing
mTree.initializeRoot(TEST_PERMISSION_STATUS); mTree.initializeRoot(TEST_PERMISSION_STATUS);
Assert.assertEquals(TEST_PERMISSION_STATUS.getUserName(), root.getUserName()); Assert.assertEquals(TEST_PERMISSION_STATUS.getUserName(), root.getUserName());
Inode<?> newRoot = mTree.getInodeByPath(new AlluxioURI("/")); Inode<?> newRoot = getInodeByPath(mTree, new AlluxioURI("/"));
Assert.assertEquals(root, newRoot); Assert.assertEquals(root, newRoot);
} }


Expand All @@ -133,7 +133,7 @@ public void createDirectoryTest() throws Exception {
// create directory // create directory
mTree.createPath(TEST_URI, sDirectoryOptions); mTree.createPath(TEST_URI, sDirectoryOptions);
Assert.assertTrue(mTree.inodePathExists(TEST_URI)); Assert.assertTrue(mTree.inodePathExists(TEST_URI));
Inode<?> test = mTree.getInodeByPath(TEST_URI); Inode<?> test = getInodeByPath(mTree, TEST_URI);
Assert.assertEquals(TEST_PATH, test.getName()); Assert.assertEquals(TEST_PATH, test.getName());
Assert.assertTrue(test.isDirectory()); Assert.assertTrue(test.isDirectory());
Assert.assertEquals("user1", test.getUserName()); Assert.assertEquals("user1", test.getUserName());
Expand All @@ -143,7 +143,7 @@ public void createDirectoryTest() throws Exception {
// create nested directory // create nested directory
mTree.createPath(NESTED_URI, sNestedDirectoryOptions); mTree.createPath(NESTED_URI, sNestedDirectoryOptions);
Assert.assertTrue(mTree.inodePathExists(NESTED_URI)); Assert.assertTrue(mTree.inodePathExists(NESTED_URI));
Inode<?> nested = mTree.getInodeByPath(NESTED_URI); Inode<?> nested = getInodeByPath(mTree, NESTED_URI);
Assert.assertEquals(TEST_PATH, nested.getName()); Assert.assertEquals(TEST_PATH, nested.getName());
Assert.assertEquals(2, nested.getParentId()); Assert.assertEquals(2, nested.getParentId());
Assert.assertTrue(test.isDirectory()); Assert.assertTrue(test.isDirectory());
Expand Down Expand Up @@ -180,13 +180,12 @@ public void createExistingDirectoryTest() throws Exception {
@Test @Test
public void createFileUnderPinnedDirectoryTest() throws Exception { public void createFileUnderPinnedDirectoryTest() throws Exception {
// create nested directory // create nested directory
InodeTree.CreatePathResult createResult = mTree.createPath(NESTED_URI, sNestedDirectoryOptions); mTree.createPath(NESTED_URI, sNestedDirectoryOptions);
List<Inode<?>> created = createResult.getCreated();
Inode<?> nested = created.get(created.size() - 1);
createResult.unlock();


// pin nested folder // pin nested folder
mTree.setPinned(nested, true); try (InodePath inodePath = mTree.lockFullInodePath(NESTED_URI, InodeTree.LockMode.WRITE)) {
mTree.setPinned(inodePath, true);
}


// create nested file under pinned folder // create nested file under pinned folder
mTree.createPath(NESTED_FILE_URI, sNestedFileOptions); mTree.createPath(NESTED_FILE_URI, sNestedFileOptions);
Expand All @@ -205,7 +204,7 @@ public void createFileUnderPinnedDirectoryTest() throws Exception {
public void createFileTest() throws Exception { public void createFileTest() throws Exception {
// created nested file // created nested file
mTree.createPath(NESTED_FILE_URI, sNestedFileOptions); mTree.createPath(NESTED_FILE_URI, sNestedFileOptions);
Inode<?> nestedFile = mTree.getInodeByPath(NESTED_FILE_URI); Inode<?> nestedFile = getInodeByPath(mTree, NESTED_FILE_URI);
Assert.assertEquals("file", nestedFile.getName()); Assert.assertEquals("file", nestedFile.getName());
Assert.assertEquals(2, nestedFile.getParentId()); Assert.assertEquals(2, nestedFile.getParentId());
Assert.assertTrue(nestedFile.isFile()); Assert.assertTrue(nestedFile.isFile());
Expand All @@ -227,11 +226,9 @@ public void createPathTest() throws Exception {
CommonUtils.sleepMs(10); CommonUtils.sleepMs(10);


// create nested directory // create nested directory
InodeTree.CreatePathResult createResult = InodeTree.CreatePathResult createResult = mTree.createPath(NESTED_URI, sNestedDirectoryOptions);
mTree.createPath(NESTED_URI, sNestedDirectoryOptions);
List<Inode<?>> modified = createResult.getModified(); List<Inode<?>> modified = createResult.getModified();
List<Inode<?>> created = createResult.getCreated(); List<Inode<?>> created = createResult.getCreated();
createResult.unlock();


// 1 modified directory // 1 modified directory
Assert.assertEquals(1, modified.size()); Assert.assertEquals(1, modified.size());
Expand Down Expand Up @@ -261,7 +258,6 @@ public void createPathTest() throws Exception {
createResult = mTree.createPath(NESTED_FILE_URI, options); createResult = mTree.createPath(NESTED_FILE_URI, options);
modified = createResult.getModified(); modified = createResult.getModified();
created = createResult.getCreated(); created = createResult.getCreated();
createResult.unlock();


// test directory was modified // test directory was modified
Assert.assertEquals(1, modified.size()); Assert.assertEquals(1, modified.size());
Expand Down Expand Up @@ -363,7 +359,7 @@ public void inodeIdExistsTest() throws Exception {
Assert.assertFalse(mTree.inodeIdExists(1)); Assert.assertFalse(mTree.inodeIdExists(1));


mTree.createPath(TEST_URI, sFileOptions); mTree.createPath(TEST_URI, sFileOptions);
Inode<?> inode = mTree.getInodeByPath(TEST_URI); Inode<?> inode = getInodeByPath(mTree, TEST_URI);
Assert.assertTrue(mTree.inodeIdExists(inode.getId())); Assert.assertTrue(mTree.inodeIdExists(inode.getId()));


mTree.deleteInode(inode); mTree.deleteInode(inode);
Expand All @@ -380,7 +376,7 @@ public void inodePathExistsTest() throws Exception {
mTree.createPath(TEST_URI, sFileOptions); mTree.createPath(TEST_URI, sFileOptions);
Assert.assertTrue(mTree.inodePathExists(TEST_URI)); Assert.assertTrue(mTree.inodePathExists(TEST_URI));


mTree.deleteInode(mTree.getInodeByPath(TEST_URI)); mTree.deleteInode(getInodeByPath(mTree, TEST_URI));
Assert.assertFalse(mTree.inodePathExists(TEST_URI)); Assert.assertFalse(mTree.inodePathExists(TEST_URI));
} }


Expand All @@ -395,7 +391,7 @@ public void getInodeByNonexistingPathTest() throws Exception {
mThrown.expectMessage("Path /test does not exist"); mThrown.expectMessage("Path /test does not exist");


Assert.assertFalse(mTree.inodePathExists(TEST_URI)); Assert.assertFalse(mTree.inodePathExists(TEST_URI));
mTree.getInodeByPath(TEST_URI); getInodeByPath(mTree, TEST_URI);
} }


/** /**
Expand All @@ -410,7 +406,7 @@ public void getInodeByNonexistingNestedPathTest() throws Exception {


mTree.createPath(NESTED_URI, sNestedDirectoryOptions); mTree.createPath(NESTED_URI, sNestedDirectoryOptions);
Assert.assertFalse(mTree.inodePathExists(NESTED_FILE_URI)); Assert.assertFalse(mTree.inodePathExists(NESTED_FILE_URI));
mTree.getInodeByPath(NESTED_FILE_URI); getInodeByPath(mTree, NESTED_FILE_URI);
} }


/** /**
Expand Down Expand Up @@ -454,13 +450,11 @@ public void getPathTest() throws Exception {
// test one level // test one level
InodeTree.CreatePathResult createResult = mTree.createPath(TEST_URI, sDirectoryOptions); InodeTree.CreatePathResult createResult = mTree.createPath(TEST_URI, sDirectoryOptions);
List<Inode<?>> created = createResult.getCreated(); List<Inode<?>> created = createResult.getCreated();
createResult.unlock();
Assert.assertEquals(new AlluxioURI("/test"), mTree.getPath(created.get(created.size() - 1))); Assert.assertEquals(new AlluxioURI("/test"), mTree.getPath(created.get(created.size() - 1)));


// test nesting // test nesting
createResult = mTree.createPath(NESTED_URI, sNestedDirectoryOptions); createResult = mTree.createPath(NESTED_URI, sNestedDirectoryOptions);
created = createResult.getCreated(); created = createResult.getCreated();
createResult.unlock();
Assert.assertEquals(new AlluxioURI("/nested/test"), Assert.assertEquals(new AlluxioURI("/nested/test"),
mTree.getPath(created.get(created.size() - 1))); mTree.getPath(created.get(created.size() - 1)));
} }
Expand Down Expand Up @@ -496,7 +490,6 @@ public void deleteInodeTest() throws Exception {
InodeTree.CreatePathResult createResult = InodeTree.CreatePathResult createResult =
mTree.createPath(NESTED_URI, sNestedDirectoryOptions); mTree.createPath(NESTED_URI, sNestedDirectoryOptions);
List<Inode<?>> created = createResult.getCreated(); List<Inode<?>> created = createResult.getCreated();
createResult.unlock();


// all inodes under root // all inodes under root
try (InodePath inodePath = mTree.lockFullInodePath(0, InodeTree.LockMode.WRITE)) { try (InodePath inodePath = mTree.lockFullInodePath(0, InodeTree.LockMode.WRITE)) {
Expand Down Expand Up @@ -528,29 +521,29 @@ public void deleteNonexistingInodeTest() throws Exception {
} }


/** /**
* Tests the {@link InodeTree#setPinned(Inode, boolean)} method. * Tests the {@link InodeTree#setPinned(InodePath, boolean)} method.
* *
* @throws Exception if creating the path fails * @throws Exception if creating the path fails
*/ */
@Test @Test
public void setPinnedTest() throws Exception { public void setPinnedTest() throws Exception {
InodeTree.CreatePathResult createResult = mTree.createPath(NESTED_URI, sNestedDirectoryOptions);
mTree.createPath(NESTED_URI, sNestedDirectoryOptions);
List<Inode<?>> created = createResult.getCreated();
Inode<?> nested = created.get(created.size() - 1);
createResult.unlock();
mTree.createPath(NESTED_FILE_URI, sNestedFileOptions); mTree.createPath(NESTED_FILE_URI, sNestedFileOptions);


// no inodes pinned // no inodes pinned
Assert.assertEquals(0, mTree.getPinIdSet().size()); Assert.assertEquals(0, mTree.getPinIdSet().size());


// pin nested folder // pin nested folder
mTree.setPinned(nested, true); try (InodePath inodePath = mTree.lockFullInodePath(NESTED_URI, InodeTree.LockMode.WRITE)) {
mTree.setPinned(inodePath, true);
}
// nested file pinned // nested file pinned
Assert.assertEquals(1, mTree.getPinIdSet().size()); Assert.assertEquals(1, mTree.getPinIdSet().size());


// unpin nested folder // unpin nested folder
mTree.setPinned(nested, false); try (InodePath inodePath = mTree.lockFullInodePath(NESTED_URI, InodeTree.LockMode.WRITE)) {
mTree.setPinned(inodePath, false);
}
Assert.assertEquals(0, mTree.getPinIdSet().size()); Assert.assertEquals(0, mTree.getPinIdSet().size());
} }


Expand Down Expand Up @@ -627,7 +620,6 @@ public void getInodePathById() throws Exception {
} }


InodeTree.CreatePathResult createResult = mTree.createPath(NESTED_FILE_URI, sNestedFileOptions); InodeTree.CreatePathResult createResult = mTree.createPath(NESTED_FILE_URI, sNestedFileOptions);
createResult.unlock();


for (Inode<?> inode : createResult.getCreated()) { for (Inode<?> inode : createResult.getCreated()) {
long id = inode.getId(); long id = inode.getId();
Expand All @@ -645,8 +637,7 @@ public void getInodePathByPath() throws Exception {
} }


// Create a nested file. // Create a nested file.
InodeTree.CreatePathResult createResult = mTree.createPath(NESTED_FILE_URI, sNestedFileOptions); mTree.createPath(NESTED_FILE_URI, sNestedFileOptions);
createResult.unlock();


AlluxioURI uri = new AlluxioURI("/nested"); AlluxioURI uri = new AlluxioURI("/nested");
try (InodePath inodePath = mTree.lockFullInodePath(uri, InodeTree.LockMode.READ)) { try (InodePath inodePath = mTree.lockFullInodePath(uri, InodeTree.LockMode.READ)) {
Expand All @@ -664,6 +655,13 @@ public void getInodePathByPath() throws Exception {
} }
} }


// Helper to get an inode by path. The inode is unlocked before returning.
private static Inode<?> getInodeByPath(InodeTree root, AlluxioURI path) throws Exception {
try (InodePath inodePath = root.lockFullInodePath(path, InodeTree.LockMode.READ)) {
return inodePath.getInode();
}
}

// helper for verifying that correct objects were journaled to the output stream // helper for verifying that correct objects were journaled to the output stream
private static void verifyJournal(InodeTree root, List<Inode<?>> journaled) throws Exception { private static void verifyJournal(InodeTree root, List<Inode<?>> journaled) throws Exception {
JournalOutputStream mockOutputStream = Mockito.mock(JournalOutputStream.class); JournalOutputStream mockOutputStream = Mockito.mock(JournalOutputStream.class);
Expand Down

0 comments on commit d3e6764

Please sign in to comment.