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.
Issue Links
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
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?
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.
Thanks Colin for the update. +1 on the v02 patch.
Committed to trunk and branch-2.
 -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 | |
| 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.
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
Thanks, Zhe Zhang.
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.