Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-alpha
    • Fix Version/s: 2.0.0-alpha
    • Component/s: hdfs-client
    • Labels:
      None

      Description

      Currently one can set the quota of a file or directory from the command line, but if a user wants to set it programmatically, they need to use DistributedFileSystem, which is annotated InterfaceAudience.Private.

      1. HDFS-3000.patch
        9 kB
        Aaron T. Myers
      2. HDFS-3000.patch
        8 kB
        Aaron T. Myers
      3. HDFS-3000.patch
        8 kB
        Aaron T. Myers
      4. HDFS-3000.patch
        8 kB
        Aaron T. Myers
      5. HDFS-3000.patch
        8 kB
        Aaron T. Myers

        Activity

        Hide
        Aaron T. Myers added a comment -

        I've given this some thought and come to the conclusion that we should just add setQuota(Path p, long nsQuota, long dsQuota) to the public interface of o.a.h.fs.FileSystem. Doing so shouldn't be incompatible, as we're just adding a net new method.

        The concept of a quota is already somewhat exposed in o.a.h.fs.FileSystem, since getContentSummary(Path) is included in FileSystem, which returns an object containing quota information. FileSystem implementations which don't support quotas return "-1" for the quota fields. We could either add a no-op default implementation of setQuota(...) to FileSystem, or add a default implementation that throws an UnsupportedOperationException, as is done for o.a.h.fs.FileSystem#listCorruptFileBlocks.

        If people are OK with this proposal, I'll whip up a patch real quick.

        Show
        Aaron T. Myers added a comment - I've given this some thought and come to the conclusion that we should just add setQuota(Path p, long nsQuota, long dsQuota) to the public interface of o.a.h.fs.FileSystem. Doing so shouldn't be incompatible, as we're just adding a net new method. The concept of a quota is already somewhat exposed in o.a.h.fs.FileSystem, since getContentSummary(Path) is included in FileSystem, which returns an object containing quota information. FileSystem implementations which don't support quotas return "-1" for the quota fields. We could either add a no-op default implementation of setQuota(...) to FileSystem, or add a default implementation that throws an UnsupportedOperationException, as is done for o.a.h.fs.FileSystem#listCorruptFileBlocks . If people are OK with this proposal, I'll whip up a patch real quick.
        Hide
        Eli Collins added a comment -

        On one hand quotas are not HDFS-specific, so fine moving them out of DistributedFileSystem. The quota interface isn't entirely generic, eg I'm not sure other file systems would combine the namespace and diskspace values in one API, but I suspect it's reasonably generic enough to be useful by another FS.

        On the other FileSystem, nor FileContext which would also need this, doesn't currently have an administrative-level interface (which quotas are). Would it be better instead to have a new public class for administrative-level APIs that is public / maintained? Eg in the same way we have haadmin and dfshaadmin, have both a generic and hdfs-specific dfs admin classes, and make the new one generic and public?

        Show
        Eli Collins added a comment - On one hand quotas are not HDFS-specific, so fine moving them out of DistributedFileSystem. The quota interface isn't entirely generic, eg I'm not sure other file systems would combine the namespace and diskspace values in one API, but I suspect it's reasonably generic enough to be useful by another FS. On the other FileSystem, nor FileContext which would also need this, doesn't currently have an administrative-level interface (which quotas are). Would it be better instead to have a new public class for administrative-level APIs that is public / maintained? Eg in the same way we have haadmin and dfshaadmin, have both a generic and hdfs-specific dfs admin classes, and make the new one generic and public?
        Hide
        Colin Patrick McCabe added a comment -

        Random thought: Does it make sense to add more functions to the FileSystem API? I thought it was deprecated in favor of FileContext.

        Show
        Colin Patrick McCabe added a comment - Random thought: Does it make sense to add more functions to the FileSystem API? I thought it was deprecated in favor of FileContext.
        Hide
        Aaron T. Myers added a comment -

        On one hand quotas are not HDFS-specific, so fine moving them out of DistributedFileSystem. The quota interface isn't entirely generic, eg I'm not sure other file systems would combine the namespace and diskspace values in one API, but I suspect it's reasonably generic enough to be useful by another FS.

        This was my thinking as well.

        On the other FileSystem, nor FileContext which would also need this, doesn't currently have an administrative-level interface (which quotas are). Would it be better instead to have a new public class for administrative-level APIs that is public / maintained? Eg in the same way we have haadmin and dfshaadmin, have both a generic and hdfs-specific dfs admin classes, and make the new one generic and public?

        I agree with you that HDFS could stand to have some public administrative API, for example to get/set safe mode, but I don't think setQuotas should belong in that API for a few reasons:

        1. I don't think it's necessarily an "administrative command" per se. At least, I don't see why there's a meaningful distinction between setting quotas and, say, chown.
        2. There'd still be the issue of the asymmetry between having to get quotas via FileSystem, but set them via some other interface, which seems goofy to me.

        Does this reasoning make sense?

        You make a good point about FileContext. I'll add that to the patch.

        Show
        Aaron T. Myers added a comment - On one hand quotas are not HDFS-specific, so fine moving them out of DistributedFileSystem. The quota interface isn't entirely generic, eg I'm not sure other file systems would combine the namespace and diskspace values in one API, but I suspect it's reasonably generic enough to be useful by another FS. This was my thinking as well. On the other FileSystem, nor FileContext which would also need this, doesn't currently have an administrative-level interface (which quotas are). Would it be better instead to have a new public class for administrative-level APIs that is public / maintained? Eg in the same way we have haadmin and dfshaadmin, have both a generic and hdfs-specific dfs admin classes, and make the new one generic and public? I agree with you that HDFS could stand to have some public administrative API, for example to get/set safe mode, but I don't think setQuotas should belong in that API for a few reasons: I don't think it's necessarily an "administrative command" per se. At least, I don't see why there's a meaningful distinction between setting quotas and, say, chown. There'd still be the issue of the asymmetry between having to get quotas via FileSystem, but set them via some other interface, which seems goofy to me. Does this reasoning make sense? You make a good point about FileContext. I'll add that to the patch.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        I agree with Eli that it is better to add an Admin API then adding them to FileSystem. The namespace quota is quite specific to HDFS.

        > I don't think it's necessarily an "administrative command" per se. At least, I don't see why there's a meaningful distinction between setting quotas and, say, chown.

        Setting quota is pure admin method but users could use chown for changing groups although they cannot change owner.

        Show
        Tsz Wo Nicholas Sze added a comment - I agree with Eli that it is better to add an Admin API then adding them to FileSystem. The namespace quota is quite specific to HDFS. > I don't think it's necessarily an "administrative command" per se. At least, I don't see why there's a meaningful distinction between setting quotas and, say, chown. Setting quota is pure admin method but users could use chown for changing groups although they cannot change owner.
        Hide
        Aaron T. Myers added a comment -

        The namespace quota is quite specific to HDFS.

        Is it? I don't have any specific counterexamples, but I'd be surprised if other file systems didn't have a similar concept, given that INodes are usually a finite resource.

        Setting quota is pure admin method but users could use chown for changing groups although they cannot change owner.

        I suppose that distinction is reasonable. The distinction, then, is "a requirement of superuser privileges in all cases."

        But, that still leaves open the issue of the asymmetry between getting/setting quotas. Does that not concern you at all, Nicholas?

        Show
        Aaron T. Myers added a comment - The namespace quota is quite specific to HDFS. Is it? I don't have any specific counterexamples, but I'd be surprised if other file systems didn't have a similar concept, given that INodes are usually a finite resource. Setting quota is pure admin method but users could use chown for changing groups although they cannot change owner. I suppose that distinction is reasonable. The distinction, then, is "a requirement of superuser privileges in all cases." But, that still leaves open the issue of the asymmetry between getting/setting quotas. Does that not concern you at all, Nicholas?
        Hide
        Aaron T. Myers added a comment -

        Random thought: Does it make sense to add more functions to the FileSystem API? I thought it was deprecated in favor of FileContext.

        I think it does, as the vast majority of users are still on FileSystem. Of course, this discussion might be moot if we end up not adding anything to FileSystem at all, per Nicholas's suggestion. So let me get back to you on that one, Colin, once we come to a conclusion on the administrative API issue.

        Show
        Aaron T. Myers added a comment - Random thought: Does it make sense to add more functions to the FileSystem API? I thought it was deprecated in favor of FileContext. I think it does, as the vast majority of users are still on FileSystem. Of course, this discussion might be moot if we end up not adding anything to FileSystem at all, per Nicholas's suggestion. So let me get back to you on that one, Colin, once we come to a conclusion on the administrative API issue.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        > Is it? I don't have any specific counterexamples, ...

        If you cannot easily find an counterexample, does it qualify for "quite specific"?

        > ... Does that not concern you at all, Nicholas?

        No. There are many values that users can read but not update, e.g. hostname, other network configurations, home directory, etc.

        Show
        Tsz Wo Nicholas Sze added a comment - > Is it? I don't have any specific counterexamples, ... If you cannot easily find an counterexample, does it qualify for "quite specific"? > ... Does that not concern you at all, Nicholas? No. There are many values that users can read but not update, e.g. hostname, other network configurations, home directory, etc.
        Hide
        Eli Collins added a comment -

        ATM has a point wrt the asymmetry, does it make sense to have setOwner and setQuota live in separate classes? I thought FileSystem/FileContext we're purely non-admin, but that's not the case.

        Another idea, we have an interface for administrative methods (setOwner, setQuota, etc) but have the implementation still live in FileSystem/Context.

        Show
        Eli Collins added a comment - ATM has a point wrt the asymmetry, does it make sense to have setOwner and setQuota live in separate classes? I thought FileSystem/FileContext we're purely non-admin, but that's not the case. Another idea, we have an interface for administrative methods (setOwner, setQuota, etc) but have the implementation still live in FileSystem/Context.
        Hide
        Aaron T. Myers added a comment -

        If you cannot easily find an counterexample, does it qualify for "quite specific"?

        I didn't say I couldn't find one - I said I didn't know. My not knowing if such things are common doesn't necessarily mean they're not.

        No. There are many values that users can read but not update, e.g. hostname, other network configurations, home directory, etc.

        That seems reasonable to me. A separate administrative interface it is, then. I'll work on a patch for it.

        Show
        Aaron T. Myers added a comment - If you cannot easily find an counterexample, does it qualify for "quite specific"? I didn't say I couldn't find one - I said I didn't know. My not knowing if such things are common doesn't necessarily mean they're not. No. There are many values that users can read but not update, e.g. hostname, other network configurations, home directory, etc. That seems reasonable to me. A separate administrative interface it is, then. I'll work on a patch for it.
        Hide
        Allen Wittenauer added a comment -

        File count quotas are extremely common and hardly unique to HDFS. Easy examples: UFS, ZFS, WAFL, VxFS, ...

        Show
        Allen Wittenauer added a comment - File count quotas are extremely common and hardly unique to HDFS. Easy examples: UFS, ZFS, WAFL, VxFS, ...
        Hide
        Tsz Wo Nicholas Sze added a comment -

        @Allen, Good to know and thanks for the input. I still don't agree that it is "extremely" common.

        Show
        Tsz Wo Nicholas Sze added a comment - @Allen, Good to know and thanks for the input. I still don't agree that it is "extremely" common.
        Hide
        Aaron T. Myers added a comment -

        Here's a patch which adds the HDFS administrative API as discussed. Please review.

        Show
        Aaron T. Myers added a comment - Here's a patch which adds the HDFS administrative API as discussed. Please review.
        Hide
        Aaron T. Myers added a comment -

        Right after posting the patch I noticed some goofy indentation and stale javadoc comments from an earlier version of the patch which declared more exception types. Here's an updated patch. Sorry for the noise.

        Show
        Aaron T. Myers added a comment - Right after posting the patch I noticed some goofy indentation and stale javadoc comments from an earlier version of the patch which declared more exception types. Here's an updated patch. Sorry for the noise.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12520697/HDFS-3000.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hdfs.TestLeaseRecovery2

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2133//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2133//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12520697/HDFS-3000.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hdfs.TestLeaseRecovery2 +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2133//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2133//console This message is automatically generated.
        Hide
        Aaron T. Myers added a comment -

        I'm confident that the test failure is unrelated.

        Show
        Aaron T. Myers added a comment - I'm confident that the test failure is unrelated.
        Hide
        Tsz Wo Nicholas Sze added a comment -
        • How about create DFSClient instead of using DistributedFileSystem?
        • How about create a new package org.apache.hadoop.hdfs.client for the API?
        Show
        Tsz Wo Nicholas Sze added a comment - How about create DFSClient instead of using DistributedFileSystem? How about create a new package org.apache.hadoop.hdfs.client for the API?
        Hide
        Aaron T. Myers added a comment -

        How about create DFSClient instead of using DistributedFileSystem?

        What's the benefit of doing this?

        How about create a new package org.apache.hadoop.hdfs.client for the API?

        That seems like a good idea to me. I'll update the patch once you get back to me on the question above.

        Show
        Aaron T. Myers added a comment - How about create DFSClient instead of using DistributedFileSystem? What's the benefit of doing this? How about create a new package org.apache.hadoop.hdfs.client for the API? That seems like a good idea to me. I'll update the patch once you get back to me on the question above.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        > What's the benefit of doing this?

        DistributedFileSystem.setQuota(..) and other admin methods should be removed. They should be moved to the admin API.

        Show
        Tsz Wo Nicholas Sze added a comment - > What's the benefit of doing this? DistributedFileSystem.setQuota(..) and other admin methods should be removed. They should be moved to the admin API.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12520700/HDFS-3000.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in .

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2135//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2135//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12520700/HDFS-3000.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2135//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2135//console This message is automatically generated.
        Hide
        Aaron T. Myers added a comment -

        Here's an updated patch which moves the class to the o.a.h.hdfs.client package.

        DistributedFileSystem.setQuota(..) and other admin methods should be removed. They should be moved to the admin API.

        I tried to do this, but it's not as easy as it seems since DistributedFileSystem is taking care of a bunch of path handling. The DFSClient just deals in Strings, not Paths, so to use DFSClient directly I'd need to also re-implement or expose DistributedFileSystem#getPathName, DistributedFileSystem#checkPath, DistributedFileSystem#makeAbsolute, and DistributedFeilSystem.workingDir. Given this, are you OK leaving it as is, Nicholas?

        Show
        Aaron T. Myers added a comment - Here's an updated patch which moves the class to the o.a.h.hdfs.client package. DistributedFileSystem.setQuota(..) and other admin methods should be removed. They should be moved to the admin API. I tried to do this, but it's not as easy as it seems since DistributedFileSystem is taking care of a bunch of path handling. The DFSClient just deals in Strings, not Paths, so to use DFSClient directly I'd need to also re-implement or expose DistributedFileSystem#getPathName, DistributedFileSystem#checkPath, DistributedFileSystem#makeAbsolute, and DistributedFeilSystem.workingDir. Given this, are you OK leaving it as is, Nicholas?
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12520771/HDFS-3000.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in .

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2139//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2139//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12520771/HDFS-3000.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2139//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2139//console This message is automatically generated.
        Hide
        Eli Collins added a comment -
        • Mind chiming in on the rationale of the setQuota vs setSpaceQuota naming (ie the former for namespace)?
        • Agree w Nicholas that we should move setQuota from DFS to this admin class, let's do that in a follow on change

        There's an extra "/**" in the setQuota javadoc. Otherwise, +1 looks good

        Show
        Eli Collins added a comment - Mind chiming in on the rationale of the setQuota vs setSpaceQuota naming (ie the former for namespace)? Agree w Nicholas that we should move setQuota from DFS to this admin class, let's do that in a follow on change There's an extra "/**" in the setQuota javadoc. Otherwise, +1 looks good
        Hide
        Aaron T. Myers added a comment -

        Mind chiming in on the rationale of the setQuota vs setSpaceQuota naming (ie the former for namespace)?

        Happy to. I agree they're not the clearest names, but I did it this way to mirror the methods in ContentSummary, which are called getQuota and getSpaceQuota, respectively.

        Agree w Nicholas that we should move setQuota from DFS to this admin class, let's do that in a follow on change

        Makes sense to me. Filed: HDFS-3173

        There's an extra "/**" in the setQuota javadoc. Otherwise, +1 looks good

        Good catch. Fixed.

        Show
        Aaron T. Myers added a comment - Mind chiming in on the rationale of the setQuota vs setSpaceQuota naming (ie the former for namespace)? Happy to. I agree they're not the clearest names, but I did it this way to mirror the methods in ContentSummary, which are called getQuota and getSpaceQuota, respectively. Agree w Nicholas that we should move setQuota from DFS to this admin class, let's do that in a follow on change Makes sense to me. Filed: HDFS-3173 There's an extra "/**" in the setQuota javadoc. Otherwise, +1 looks good Good catch. Fixed.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12520799/HDFS-3000.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in .

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2142//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2142//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12520799/HDFS-3000.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2142//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2142//console This message is automatically generated.
        Hide
        Eli Collins added a comment -

        +1 looks good

        Show
        Eli Collins added a comment - +1 looks good
        Hide
        Aaron T. Myers added a comment -

        Thanks a lot for the reviews, Eli.

        Nicholas, does this seem OK to you? I'd like to get this checked in in the next day or two.

        Thanks a lot.

        Show
        Aaron T. Myers added a comment - Thanks a lot for the reviews, Eli. Nicholas, does this seem OK to you? I'd like to get this checked in in the next day or two. Thanks a lot.
        Hide
        Daryn Sharp added a comment -

        Should the dfsadmin CLI be updated to use the new api?

        Show
        Daryn Sharp added a comment - Should the dfsadmin CLI be updated to use the new api?
        Hide
        Tsz Wo Nicholas Sze added a comment -

        > Nicholas, does this seem OK to you? ...

        Let me check it later today.

        Show
        Tsz Wo Nicholas Sze added a comment - > Nicholas, does this seem OK to you? ... Let me check it later today.
        Hide
        Aaron T. Myers added a comment -

        Should the dfsadmin CLI be updated to use the new api?

        No reason it couldn't, but I also don't see any reason to do so in this JIRA. It will need to be done to implement HDFS-3173, so maybe just do so over there?

        Show
        Aaron T. Myers added a comment - Should the dfsadmin CLI be updated to use the new api? No reason it couldn't, but I also don't see any reason to do so in this JIRA. It will need to be done to implement HDFS-3173 , so maybe just do so over there?
        Hide
        Tsz Wo Nicholas Sze added a comment -

        > I tried to do this, but it's not as easy as it seems since DistributedFileSystem is taking care of a bunch of path handling. ... are you OK leaving it as is, Nicholas?

        Sure. If it is too hard, let's leave it for now and make the improvement later.

        +1 patch looks good. Some minor comments which could be fixed here or later.

        • "quota (count of files and directories)" => "namespace quota (count of name objects including files, directories and symlinks)"
        • "space quota (size of files and directories) for a directory." => "disk space quota (size of files) for a directory. Note that directories and symlinks do not occupy disk space.
        • You may want to put the following in one line.
           public HdfsAdmin(URI uri, Configuration conf)
                throws IOException {
          
        • Remove "throws IOException" from TestHdfsAdmin.shutDownCluster().
        Show
        Tsz Wo Nicholas Sze added a comment - > I tried to do this, but it's not as easy as it seems since DistributedFileSystem is taking care of a bunch of path handling. ... are you OK leaving it as is, Nicholas? Sure. If it is too hard, let's leave it for now and make the improvement later. +1 patch looks good. Some minor comments which could be fixed here or later. "quota (count of files and directories)" => "namespace quota (count of name objects including files, directories and symlinks)" "space quota (size of files and directories) for a directory." => "disk space quota (size of files) for a directory. Note that directories and symlinks do not occupy disk space. You may want to put the following in one line. public HdfsAdmin(URI uri, Configuration conf) throws IOException { Remove "throws IOException" from TestHdfsAdmin.shutDownCluster().
        Hide
        Daryn Sharp added a comment -

        +1 Although I'd suggest maybe adding a ctor that takes a filesystem instance. It user may want to use a custom configured filesystem, or avoid creation of another fs instance if the fs cache is disabled.

        Show
        Daryn Sharp added a comment - +1 Although I'd suggest maybe adding a ctor that takes a filesystem instance. It user may want to use a custom configured filesystem, or avoid creation of another fs instance if the fs cache is disabled.
        Hide
        Aaron T. Myers added a comment -

        Thanks a lot for the reviews, Daryn and Nicholas. Here's an updated patch which incorporates all of Nicholas's feedback.

        I'd suggest maybe adding a ctor that takes a filesystem instance. It user may want to use a custom configured filesystem, or avoid creation of another fs instance if the fs cache is disabled.

        I didn't do this since in HDFS-3173 we will presumably be removing the DistributedFileSystem instance variable from the HdfsAdmin class, per Nicholas's suggestion above.

        I'm going to go ahead and commit this since the changes since the last patch were all cosmetic or to javadocs.

        Show
        Aaron T. Myers added a comment - Thanks a lot for the reviews, Daryn and Nicholas. Here's an updated patch which incorporates all of Nicholas's feedback. I'd suggest maybe adding a ctor that takes a filesystem instance. It user may want to use a custom configured filesystem, or avoid creation of another fs instance if the fs cache is disabled. I didn't do this since in HDFS-3173 we will presumably be removing the DistributedFileSystem instance variable from the HdfsAdmin class, per Nicholas's suggestion above. I'm going to go ahead and commit this since the changes since the last patch were all cosmetic or to javadocs.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #2062 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2062/)
        HDFS-3000. Add a public API for setting quotas. Contributed by Aaron T. Myers. (Revision 1309227)

        Result = SUCCESS
        atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309227
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client/HdfsAdmin.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHdfsAdmin.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2062 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2062/ ) HDFS-3000 . Add a public API for setting quotas. Contributed by Aaron T. Myers. (Revision 1309227) Result = SUCCESS atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309227 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client/HdfsAdmin.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHdfsAdmin.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #1987 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1987/)
        HDFS-3000. Add a public API for setting quotas. Contributed by Aaron T. Myers. (Revision 1309227)

        Result = SUCCESS
        atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309227
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client/HdfsAdmin.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHdfsAdmin.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #1987 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1987/ ) HDFS-3000 . Add a public API for setting quotas. Contributed by Aaron T. Myers. (Revision 1309227) Result = SUCCESS atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309227 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client/HdfsAdmin.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHdfsAdmin.java
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12521273/HDFS-3000.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in .

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2180//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2180//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12521273/HDFS-3000.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2180//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2180//console This message is automatically generated.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #2000 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2000/)
        HDFS-3000. Add a public API for setting quotas. Contributed by Aaron T. Myers. (Revision 1309227)

        Result = SUCCESS
        atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309227
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client/HdfsAdmin.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHdfsAdmin.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2000 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2000/ ) HDFS-3000 . Add a public API for setting quotas. Contributed by Aaron T. Myers. (Revision 1309227) Result = SUCCESS atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309227 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client/HdfsAdmin.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHdfsAdmin.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1005 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1005/)
        HDFS-3000. Add a public API for setting quotas. Contributed by Aaron T. Myers. (Revision 1309227)

        Result = FAILURE
        atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309227
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client/HdfsAdmin.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHdfsAdmin.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1005 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1005/ ) HDFS-3000 . Add a public API for setting quotas. Contributed by Aaron T. Myers. (Revision 1309227) Result = FAILURE atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309227 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client/HdfsAdmin.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHdfsAdmin.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1040 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1040/)
        HDFS-3000. Add a public API for setting quotas. Contributed by Aaron T. Myers. (Revision 1309227)

        Result = FAILURE
        atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309227
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client/HdfsAdmin.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHdfsAdmin.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1040 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1040/ ) HDFS-3000 . Add a public API for setting quotas. Contributed by Aaron T. Myers. (Revision 1309227) Result = FAILURE atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309227 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/client/HdfsAdmin.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHdfsAdmin.java
        Hide
        Aaron T. Myers added a comment -

        I've just committed this to trunk and branch-2.

        Thanks a lot for the reviews.

        Show
        Aaron T. Myers added a comment - I've just committed this to trunk and branch-2. Thanks a lot for the reviews.

          People

          • Assignee:
            Aaron T. Myers
            Reporter:
            Aaron T. Myers
          • Votes:
            0 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development