Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.22.0
    • Fix Version/s: 0.23.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change

      Description

      FileContext should have API to get statistics from underlying file systems.

      1. HADOOP-6432.1.patch
        4 kB
        Jitendra Nath Pandey
      2. HADOOP-6432.2.patch
        9 kB
        Jitendra Nath Pandey
      3. HADOOP-6432.3.patch
        11 kB
        Jitendra Nath Pandey
      4. HADOOP-6432.4.patch
        12 kB
        Jitendra Nath Pandey
      5. HADOOP-6432-trunk.5.patch
        12 kB
        Jitendra Nath Pandey
      6. HADOOP-6432-trunk.6.patch
        15 kB
        Jitendra Nath Pandey
      7. HADOOP-6432-trunk.8.patch
        13 kB
        Jitendra Nath Pandey

        Activity

        Hide
        Suresh Srinivas added a comment -
        1. Can you please add a brief description of what the patch does
        2. FileContext.java - remove Options.Rename import
        3. All FileContext.java public methods added should have javadoc
        4. FileContext.getStatistics() - add more description to the method that only scheme and authority is used for looking up the file system.
        5. FileContext.getAllStatistics() - should not throw URISyntaxException.
        6. AbstractFileSystem.java - IdentifyHashMap import is unused
        7. AbstractFileSystem.STATISTICS_TABLE declaration exceeds 80 columns.
        8. AbstractFileSystem.getStatistics() - use getUri() method get the URI with scheme and authority.
        9. AbstractFileSystem and FileContext printStatistics() prints the statistics using System.out.println. Not sure where this is used and if we need it.
        10. AbstractFileSystem.getAllStatistics - initialize statsMap size to that of STATISTICS_TABLE. Also it is a good idea to define copy constructor in Statistics class.
        11. Is it a good idea to have FcStatisticsTest that includes common logic and have a subclass for TestLocalFcStatistics, TestHdfsFcStatistics etc? Could the base test use FileContextTestHelper and avoid using /tmp as directory to create tests. Also code could be reused by adding a method checkStatistics(int expectedBytesRead, int expectedBytesWritten, Statistics s)
        Show
        Suresh Srinivas added a comment - Can you please add a brief description of what the patch does FileContext.java - remove Options.Rename import All FileContext.java public methods added should have javadoc FileContext.getStatistics() - add more description to the method that only scheme and authority is used for looking up the file system. FileContext.getAllStatistics() - should not throw URISyntaxException. AbstractFileSystem.java - IdentifyHashMap import is unused AbstractFileSystem.STATISTICS_TABLE declaration exceeds 80 columns. AbstractFileSystem.getStatistics() - use getUri() method get the URI with scheme and authority. AbstractFileSystem and FileContext printStatistics() prints the statistics using System.out.println. Not sure where this is used and if we need it. AbstractFileSystem.getAllStatistics - initialize statsMap size to that of STATISTICS_TABLE. Also it is a good idea to define copy constructor in Statistics class. Is it a good idea to have FcStatisticsTest that includes common logic and have a subclass for TestLocalFcStatistics, TestHdfsFcStatistics etc? Could the base test use FileContextTestHelper and avoid using /tmp as directory to create tests. Also code could be reused by adding a method checkStatistics(int expectedBytesRead, int expectedBytesWritten, Statistics s)
        Hide
        Jitendra Nath Pandey added a comment -

        A few points regarding the patch:
        1. The Statistics are statically stored inside AbstractFileSystem in a map that is keyed on URI. Only scheme and authority part of the URI are used.
        2. API are added to FileContext to get the statistics.
        3. FileContext.getAllStatistics returns a map of URI and statistics, but the URI key contains only scheme and authority.

        Show
        Jitendra Nath Pandey added a comment - A few points regarding the patch: 1. The Statistics are statically stored inside AbstractFileSystem in a map that is keyed on URI. Only scheme and authority part of the URI are used. 2. API are added to FileContext to get the statistics. 3. FileContext.getAllStatistics returns a map of URI and statistics, but the URI key contains only scheme and authority.
        Hide
        Jitendra Nath Pandey added a comment -

        HADOOP-6432.3.patch submitted for hudson tests.

        Show
        Jitendra Nath Pandey added a comment - HADOOP-6432 .3.patch submitted for hudson tests.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12428353/HADOOP-6432.3.patch
        against trunk revision 891511.

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

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

        -1 javadoc. The javadoc tool appears to have generated 1 warning messages.

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

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

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

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

        Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/218/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/218/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/218/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/218/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/12428353/HADOOP-6432.3.patch against trunk revision 891511. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. -1 javadoc. The javadoc tool appears to have generated 1 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/218/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/218/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/218/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/218/console This message is automatically generated.
        Hide
        Jitendra Nath Pandey added a comment -

        New patch fixing the javadoc warning.

        Show
        Jitendra Nath Pandey added a comment - New patch fixing the javadoc warning.
        Hide
        Jitendra Nath Pandey added a comment -

        HADOOP-6432.4.patch submitted for hudson tests.

        Show
        Jitendra Nath Pandey added a comment - HADOOP-6432 .4.patch submitted for hudson tests.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12428368/HADOOP-6432.4.patch
        against trunk revision 891511.

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

        +1 tests included. The patch appears to include 7 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 findbugs. The patch does not introduce any new Findbugs warnings.

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

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

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

        Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/219/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/219/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/219/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/219/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/12428368/HADOOP-6432.4.patch against trunk revision 891511. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 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 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/219/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/219/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/219/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/219/console This message is automatically generated.
        Hide
        Jitendra Nath Pandey added a comment -

        New patch.

        Show
        Jitendra Nath Pandey added a comment - New patch.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12469871/HADOOP-6432-trunk.5.patch
        against trunk revision 1064919.

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

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

        -1 javadoc. The javadoc tool appears to have generated 1 warning messages.

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

        +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 core unit tests.

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

        +1 system test framework. The patch passed system test framework compile.

        Test results: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/212//testReport/
        Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/212//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/212//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/12469871/HADOOP-6432-trunk.5.patch against trunk revision 1064919. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 5 new or modified tests. -1 javadoc. The javadoc tool appears to have generated 1 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +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 core unit tests. +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/212//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/212//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/212//console This message is automatically generated.
        Hide
        Suresh Srinivas added a comment -
        1. AbstractFileSystem.java - remove unused import IdentityHashMap
        2. AbstractFileSystem#getStatistics() - can you make only the inner part that accesses HashMap synchronized, excluding validation of URI.
        3. minor: remove empty line changes before AbstractFileSystem#getStatistics() method.
        4. STATISTICS_TABLE declaration goes beyond 80 columns
        5. FileContext#rename() can you change the link in javadoc from Rename#OVERWRITE to Options.Rename#OVERWRITE to fix a javadoc warning
        6. FileContext.java - the newly added methods need not be synchronized as it synchronization is handled by AbstractFileSystem.
        7. FileContext#getAllStatistics() in javadoc unnecessarily indicates URISyntaxException is thrown.
        8. FileContext#clearStatistics() - javadoc should indicate this method clears statistics for all the file systems.
        9. FileContext#getStatistics() - for @param uri, have a fullstop after "the uri to lookup the statistics".
        10. FileContext#printStatistics() - why does this method throw IOException? Remove it both from javadoc and declaration.
        11. Complete the @param argument for new copy constructor in Statistics.
        12. FCStatisticsBaseTest - "Base class to test File Context Statistics." instead of File Context add link to FileContext.
        13. FCStatisticsBaseTest - why do you need exact URI from getSchemeAuthorityUri()? The API handles any URI by picking scheme and authority right?
        14. TestLocalFsFCStatistics - please add comment on why you are doing blockSize + 12? Also please change the class javadoc to indicate that this is testing stats for LocalFs.
        Show
        Suresh Srinivas added a comment - AbstractFileSystem.java - remove unused import IdentityHashMap AbstractFileSystem#getStatistics() - can you make only the inner part that accesses HashMap synchronized, excluding validation of URI. minor: remove empty line changes before AbstractFileSystem#getStatistics() method. STATISTICS_TABLE declaration goes beyond 80 columns FileContext#rename() can you change the link in javadoc from Rename#OVERWRITE to Options.Rename#OVERWRITE to fix a javadoc warning FileContext.java - the newly added methods need not be synchronized as it synchronization is handled by AbstractFileSystem. FileContext#getAllStatistics() in javadoc unnecessarily indicates URISyntaxException is thrown. FileContext#clearStatistics() - javadoc should indicate this method clears statistics for all the file systems. FileContext#getStatistics() - for @param uri, have a fullstop after "the uri to lookup the statistics". FileContext#printStatistics() - why does this method throw IOException? Remove it both from javadoc and declaration. Complete the @param argument for new copy constructor in Statistics. FCStatisticsBaseTest - "Base class to test File Context Statistics." instead of File Context add link to FileContext. FCStatisticsBaseTest - why do you need exact URI from getSchemeAuthorityUri()? The API handles any URI by picking scheme and authority right? TestLocalFsFCStatistics - please add comment on why you are doing blockSize + 12? Also please change the class javadoc to indicate that this is testing stats for LocalFs.
        Hide
        Jitendra Nath Pandey added a comment -

        Uploaded a new patch addressing the comments, submitted for Hudson tests.

        Show
        Jitendra Nath Pandey added a comment - Uploaded a new patch addressing the comments, submitted for Hudson tests.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12469962/HADOOP-6432-trunk.6.patch
        against trunk revision 1065959.

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

        +1 tests included. The patch appears to include 5 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 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 core unit tests.

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

        +1 system test framework. The patch passed system test framework compile.

        Test results: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/214//testReport/
        Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/214//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/214//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/12469962/HADOOP-6432-trunk.6.patch against trunk revision 1065959. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 5 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 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 core unit tests. +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/214//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/214//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/214//console This message is automatically generated.
        Hide
        Jitendra Nath Pandey added a comment -

        javadoc for rename method in FileContext was reformatted in previous patch.

        Show
        Jitendra Nath Pandey added a comment - javadoc for rename method in FileContext was reformatted in previous patch.
        Hide
        Suresh Srinivas added a comment -

        +1 for the patch.

        Show
        Suresh Srinivas added a comment - +1 for the patch.
        Hide
        Jitendra Nath Pandey added a comment -

        The only change in the last patch and the previous one was in javadoc. I ran javadoc manually to verify.

        Show
        Jitendra Nath Pandey added a comment - The only change in the last patch and the previous one was in javadoc. I ran javadoc manually to verify.
        Hide
        Jitendra Nath Pandey added a comment -

        I just committed this.

        Show
        Jitendra Nath Pandey added a comment - I just committed this.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #494 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/494/)
        HADOOP-6432. Add Statistics support in FileContext. Contributed by jitendra.

        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #494 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/494/ ) HADOOP-6432 . Add Statistics support in FileContext. Contributed by jitendra.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk #594 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/594/)
        HADOOP-6432. Add Statistics support in FileContext. Contributed by jitendra.

        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk #594 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/594/ ) HADOOP-6432 . Add Statistics support in FileContext. Contributed by jitendra.

          People

          • Assignee:
            Jitendra Nath Pandey
            Reporter:
            Jitendra Nath Pandey
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development