Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-9541

Add hdfsStreamBuilder API to libhdfs to support defaultBlockSizes greater than 2 GB

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.20.1
    • Fix Version/s: 2.9.0, 3.0.0-alpha1
    • Component/s: libhdfs
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      We should have a new API in libhdfs which will support creating files with a default block size that is more than 31 bits in size. We should also make this a builder API so that it is easy to add more options later.

      1. HDFS-9541.001.patch
        8 kB
        Colin P. McCabe
      2. HDFS-9541.002.patch
        9 kB
        Colin P. McCabe

        Issue Links

          Activity

          Hide
          cmccabe Colin P. McCabe added a comment -

          This patch is compatible since it just adds new APIs rather than changing or removing existing ones. For new APIs, we want to explicitly spell out the type sizes so that they are the same on each platform. This patch adds a builder API which we can use to add more options to "open" later if we want.

          Show
          cmccabe Colin P. McCabe added a comment - This patch is compatible since it just adds new APIs rather than changing or removing existing ones. For new APIs, we want to explicitly spell out the type sizes so that they are the same on each platform. This patch adds a builder API which we can use to add more options to "open" later if we want.
          Hide
          zhz Zhe Zhang added a comment -

          Thanks Colin. I think having a builder method is a good idea. Patch LGTM overall. A few minors:

          1. Should it be sizeof(struct hdfsStreamBuilder) + path_len? Since bld->path already has 1 byte allocated to store the \0.
            bld = malloc(sizeof(struct hdfsStreamBuilder) + path_len + 1);
            
          2. Should be s/hdfsStreamBuilderFree/hdfsStreamBuilderBuild/
                 * It is normally not necessary to call this function since
                 * hdfsStreamBuilderFree frees the builder.
            
          3. Might be worthwhile adding another hdfsOpenFile that actually uses int64_t as defaultBlockSize?
          Show
          zhz Zhe Zhang added a comment - Thanks Colin. I think having a builder method is a good idea. Patch LGTM overall. A few minors: Should it be sizeof(struct hdfsStreamBuilder) + path_len ? Since bld->path already has 1 byte allocated to store the \0 . bld = malloc(sizeof(struct hdfsStreamBuilder) + path_len + 1); Should be s/hdfsStreamBuilderFree/hdfsStreamBuilderBuild/ * It is normally not necessary to call this function since * hdfsStreamBuilderFree frees the builder. Might be worthwhile adding another hdfsOpenFile that actually uses int64_t as defaultBlockSize ?
          Hide
          cmccabe Colin P. McCabe added a comment -

          Thanks for the review, Zhe Zhang

          Should it be sizeof(struct hdfsStreamBuilder) + path_len? Since bld->path already has 1 byte allocated to store the \0.

          OK

          Should be s/hdfsStreamBuilderFree/hdfsStreamBuilderBuild/ [in the comment]

          Good catch

          Might be worthwhile adding another hdfsOpenFile that actually uses int64_t as defaultBlockSize?

          I think we should avoid having lots of different "open" functions. The builder lets people set just the options they want, in a more readable way.

          Show
          cmccabe Colin P. McCabe added a comment - Thanks for the review, Zhe Zhang Should it be sizeof(struct hdfsStreamBuilder) + path_len? Since bld->path already has 1 byte allocated to store the \0. OK Should be s/hdfsStreamBuilderFree/hdfsStreamBuilderBuild/ [in the comment] Good catch Might be worthwhile adding another hdfsOpenFile that actually uses int64_t as defaultBlockSize? I think we should avoid having lots of different "open" functions. The builder lets people set just the options they want, in a more readable way.
          Hide
          zhz Zhe Zhang added a comment -

          Thanks Colin for the update. +1 on the v02 patch.

          Show
          zhz Zhe Zhang added a comment - Thanks Colin for the update. +1 on the v02 patch.
          Hide
          zhz Zhe Zhang added a comment -

          Committed to trunk and branch-2.

          Show
          zhz Zhe Zhang added a comment - Committed to trunk and branch-2.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 mvninstall 8m 27s trunk passed
          +1 compile 0m 14s trunk passed with JDK v1.8.0_66
          +1 compile 0m 15s trunk passed with JDK v1.7.0_91
          +1 mvnsite 0m 15s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 mvninstall 0m 10s the patch passed
          +1 compile 0m 12s the patch passed with JDK v1.8.0_66
          +1 cc 0m 12s the patch passed
          +1 javac 0m 12s the patch passed
          +1 compile 0m 12s the patch passed with JDK v1.7.0_91
          +1 cc 0m 12s the patch passed
          +1 javac 0m 12s the patch passed
          +1 mvnsite 0m 13s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 unit 0m 33s hadoop-hdfs-native-client in the patch passed with JDK v1.8.0_66.
          +1 unit 0m 33s hadoop-hdfs-native-client in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 20s Patch does not generate ASF License warnings.
          13m 36s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12784038/HDFS-9541.002.patch
          JIRA Issue HDFS-9541
          Optional Tests asflicense compile cc mvnsite javac unit
          uname Linux c682bb5f4624 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / d0d7c22
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14249/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-native-client U: hadoop-hdfs-project/hadoop-hdfs-native-client
          Max memory used 77MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14249/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 8m 27s trunk passed +1 compile 0m 14s trunk passed with JDK v1.8.0_66 +1 compile 0m 15s trunk passed with JDK v1.7.0_91 +1 mvnsite 0m 15s trunk passed +1 mvneclipse 0m 12s trunk passed +1 mvninstall 0m 10s the patch passed +1 compile 0m 12s the patch passed with JDK v1.8.0_66 +1 cc 0m 12s the patch passed +1 javac 0m 12s the patch passed +1 compile 0m 12s the patch passed with JDK v1.7.0_91 +1 cc 0m 12s the patch passed +1 javac 0m 12s the patch passed +1 mvnsite 0m 13s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 unit 0m 33s hadoop-hdfs-native-client in the patch passed with JDK v1.8.0_66. +1 unit 0m 33s hadoop-hdfs-native-client in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 20s Patch does not generate ASF License warnings. 13m 36s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12784038/HDFS-9541.002.patch JIRA Issue HDFS-9541 Optional Tests asflicense compile cc mvnsite javac unit uname Linux c682bb5f4624 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / d0d7c22 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14249/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-native-client U: hadoop-hdfs-project/hadoop-hdfs-native-client Max memory used 77MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14249/console This message was automatically generated.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #9187 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9187/)
          HDFS-9541. Add hdfsStreamBuilder API to libhdfs to support (zhz: rev cf8af7bb459b21babaad2d972330a3b4c6bb222d)

          • hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/hdfs.c
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/include/hdfs/hdfs.h
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9187 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9187/ ) HDFS-9541 . Add hdfsStreamBuilder API to libhdfs to support (zhz: rev cf8af7bb459b21babaad2d972330a3b4c6bb222d) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/hdfs.c hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/include/hdfs/hdfs.h
          Hide
          cmccabe Colin P. McCabe added a comment -

          Thanks, Zhe Zhang.

          Show
          cmccabe Colin P. McCabe added a comment - Thanks, Zhe Zhang .

            People

            • Assignee:
              cmccabe Colin P. McCabe
              Reporter:
              cmccabe Colin P. McCabe
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development