[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16410372#comment-16410372 ] Hudson commented on HDFS-8766: -- SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #13869 (See [https://builds.apache.org/job/Hadoop-trunk-Commit/13869/]) HDFS-8766. Implement a libhdfs(3) compatible API. Contributed by James (james.clampffer: rev dbd1d0c75bce3b7885fbf6332a1d5c673d36395f) * (add) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/bindings/c/hdfs_cpp.h * (add) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/bindings/c/CMakeLists.txt * (add) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/bindings/c/hdfs.cc * (add) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/bindings/CMakeLists.txt * (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/CMakeLists.txt * (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/CMakeLists.txt * (add) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/bindings/c/hdfs_cpp.cc > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer >Priority: Major > Fix For: HDFS-8707 > > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch, > HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch, > HDFS-8766.HDFS-8707.009.patch, HDFS-8766.HDFS-8707.010.patch, > HDFS-8766.HDFS-8707.011.patch, HDFS-8766.HDFS-8707.012.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14978366#comment-14978366 ] Bob Hansen commented on HDFS-8766: -- Yow, [~wheat9]! That's a good catch on the promise/future bug. The implementation of promise/future is a total failure of the Principle of Least Astonishment. Thank you for linking to the reference. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch, > HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch, > HDFS-8766.HDFS-8707.009.patch, HDFS-8766.HDFS-8707.010.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14978378#comment-14978378 ] James Clampffer commented on HDFS-8766: --- Thanks for the review Haohui! Good catch on the "using namespace" in the header, that must have ended up there while I was splitting the files apart. I'll fix that, the comment issue, and go back to using shared_ptrs to the promises to fix the race condition similar to your fix on the old github branch. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch, > HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch, > HDFS-8766.HDFS-8707.009.patch, HDFS-8766.HDFS-8707.010.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14979012#comment-14979012 ] James Clampffer commented on HDFS-8766: --- "Please wrap the statement with braces though it is a single line. There are several places like the above code needs to be fixed." It looks like this is something the google coding style explicitly allows: "Short conditional statements may be written on one line if this enhances readability. You may use this only when the line is brief and the statement does not use the else clause.". If we are going to be very strict about those rules everywhere else I'd like to keep them the same here. I don't feel strongly about it either way but any exceptions or additional restrictions need to be easily available for everyone to see. Even without any extra stuff we at least need a README that says "all code must follow google's standards". Currently the only way people can find out that this project sticks to the google style guide is by reading all of the comments and even they will only catch the additional rules that have been pointed out. This will save everyone a significant amount of time because we'll have fewer code reviews fail due to style issues. If you have time today or tomorrow would you mind dropping a README into /libhdfspp file with your coding standards? I'd be happy to add one but I don't know what else I'd be missing. I'll file a jira for adding a file with coding standards to the source tree. Let me know what you prefer with the simple if..cond..something and I'll conform to that. I'll get the debug helpers out of the header. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch, > HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch, > HDFS-8766.HDFS-8707.009.patch, HDFS-8766.HDFS-8707.010.patch, > HDFS-8766.HDFS-8707.011.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14978816#comment-14978816
]
Haohui Mai commented on HDFS-8766:
--
Looks good to me overall. A few nitpicks:
{code}
+ if (!s.ok()) return -1;
{code}
Please wrap the statement with braces though it is a single line. There are
several places like the above code needs to be fixed.
{code}
+/** error reporting helpers **/
+extern thread_local std::string errstr;
+void ReportError(int errnum, std::string msg);
+
+/**
+ *client supplies buffer and length of buffer
+ *fill it and add trailing null
+ **/
+void getErrorStr(char *buf, size_t len);
{code}
Looks like they can be removed from the header file.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch,
> HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch,
> HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch,
> HDFS-8766.HDFS-8707.009.patch, HDFS-8766.HDFS-8707.010.patch,
> HDFS-8766.HDFS-8707.011.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14978819#comment-14978819
]
Haohui Mai commented on HDFS-8766:
--
Things that be a
{code}
+#include "libhdfspp/hdfs.h"
+ auto callback = [stat, ](const Status , FileSystem *f) {
+fs = f;
+stat->set_value(s);
+ };
{code}
It might make sense to rename the {{libhdfspp/hdfs.h}} to
{{libhdfspp/libhdfspp.h}}. It is also beneficial to change the way how the
{{FileSystem}} works to only make the connection when calling {{Connect()}}.
They can be addressed in follow-up jiras.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch,
> HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch,
> HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch,
> HDFS-8766.HDFS-8707.009.patch, HDFS-8766.HDFS-8707.010.patch,
> HDFS-8766.HDFS-8707.011.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14979418#comment-14979418 ] Haohui Mai commented on HDFS-8766: -- bq. Currently the only way people can find out that this project sticks to the google style guide is by reading all of the comments and even they will only catch the additional rules that have been pointed out. This will save everyone a significant amount of time because we'll have fewer code reviews fail due to style issues. Good point. The current coding style is mostly based on the Google C++ styling guide, with some divergences towards to Hadoop styling guide (which is based on the Sun styling guide) (http://wiki.apache.org/hadoop/CodeReviewChecklist) so that the styling can be keep somewhat consistent with the Java side. It makes sense to have a README to document them. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Fix For: HDFS-8707 > > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch, > HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch, > HDFS-8766.HDFS-8707.009.patch, HDFS-8766.HDFS-8707.010.patch, > HDFS-8766.HDFS-8707.011.patch, HDFS-8766.HDFS-8707.012.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14979415#comment-14979415 ] Haohui Mai commented on HDFS-8766: -- +1. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch, > HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch, > HDFS-8766.HDFS-8707.009.patch, HDFS-8766.HDFS-8707.010.patch, > HDFS-8766.HDFS-8707.011.patch, HDFS-8766.HDFS-8707.012.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14977288#comment-14977288
]
Bob Hansen commented on HDFS-8766:
--
{code}
third_party/asio-1.10.2/include
third_party/gmock-1.7.0
${OPENSSL_INCLUDE_DIR}
+ ../libhdfs/include
{code}
It would be good to make this a named variable that can be overridden, but I
think we should capture that in a "make libhdfs++ build to be configurable
outside of the Hadoop tree" Jira.
I agree with Haohui that the hdfs_cpp code should live in lib/fs, and I also
agree with James that that can be the purview of HDFS-9144.
I can see the templated async-to-sync work being tricky, especially given the
gcc bug. I'll see if I can poke at it. Do you have a reference to that bug
for my own edification?
+1, and we can capture the outstanding work in new Jiras.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch,
> HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch,
> HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch,
> HDFS-8766.HDFS-8707.009.patch, HDFS-8766.HDFS-8707.010.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14977422#comment-14977422 ] James Clampffer commented on HDFS-8766: --- I'm not sure what I'm missing with regards to style stuff: -changed bindings to bindings/c -changed c headers to corresponding c++ headers -changed to google style guide rules for include ordering -stripped out comments on includes -added libhdfs/include to include path, changed includes to hdfs/hdfs.h (in brackets) -split method implementation from c api stubs into different files -changed comments to /** */ style I'll look into the race condition stuff. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch, > HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch, > HDFS-8766.HDFS-8707.009.patch, HDFS-8766.HDFS-8707.010.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14977494#comment-14977494
]
Hadoop QA commented on HDFS-8766:
-
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | pre-patch | 6m 14s | Pre-patch HDFS-8707 compilation
is healthy. |
| {color:green}+1{color} | @author | 0m 0s | The patch does not contain any
@author tags. |
| {color:red}-1{color} | tests included | 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. |
| {color:red}-1{color} | javac | 1m 32s | The patch appears to cause the
build to fail. |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12769119/HDFS-8766.HDFS-8707.010.patch
|
| Optional Tests | javac unit |
| git revision | HDFS-8707 / 6d68759 |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/13236/console |
This message was automatically generated.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch,
> HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch,
> HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch,
> HDFS-8766.HDFS-8707.009.patch, HDFS-8766.HDFS-8707.010.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14977440#comment-14977440 ] James Clampffer commented on HDFS-8766: --- Thanks for the +1! I agree that we should be able to override that path for projects that want to keep headers in a common place. I think I need to learn a few more CMake tricks before I can do it right. I'll open up a "make libhdfs++ build to be configurable outside of the Hadoop tree" once I get to work tomorrow. As for the GCC bug you can check it out here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41933 It says it's been resolved, so I'll have to see when the fix made it out to various linux distros. But if it's breaking on the default GCC used ubuntu 14.04 LTS I don't think it's safe to include in a project that needs to be built on some older compilers. There's some workarounds posted that I'm looking into using to get something simple and portable. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch, > HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch, > HDFS-8766.HDFS-8707.009.patch, HDFS-8766.HDFS-8707.010.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14977386#comment-14977386 ] Haohui Mai commented on HDFS-8766: -- Looks like many of the comments listed in https://issues.apache.org/jira/browse/HDFS-8766?focusedCommentId=14971680=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14971680 remain unaddressed. Can you go through and update the patch? bq. It would be similar to doing "stringstream ss;; return ss.str().c_str()" I don't think so. Please see http://stackoverflow.com/questions/10843304/race-condition-in-pthread-once > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch, > HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch, > HDFS-8766.HDFS-8707.009.patch, HDFS-8766.HDFS-8707.010.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14977458#comment-14977458
]
Haohui Mai commented on HDFS-8766:
--
Sorry looked at the wrong patch. Good work, [~James Clampffer]!
Some minor issues:
{code}
+/*
+ C API implementations
+*/
{code}
It might make more sense to make it into the following:
{code}
+/**
+ * C API implementations
+ **/
{code}
to make it consistent with the rest of the code in the repository (both C++ and
Java)
{code}
+using namespace hdfs;
+/* Seperate the handles used by the C api from the C++ API*/
{code}
It's a bad idea to put {{using namespace}} into the header file.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch,
> HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch,
> HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch,
> HDFS-8766.HDFS-8707.009.patch, HDFS-8766.HDFS-8707.010.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14976459#comment-14976459 ] Bob Hansen commented on HDFS-8766: -- [~James Clampffer] - re: namespaces - I think that the FileHandle and FileSystem will grow to become the primary C++ API, so they should just be in the hdfs namespace. Perhaps they should live in the lib/fs/ directory. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch, > HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch, > HDFS-8766.HDFS-8707.009.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14975188#comment-14975188 ] James Clampffer commented on HDFS-8766: --- Thanks for the input Haohui! I have the bulk of the next patch done incorporating your feedback but it doesn't look like I'll have it done and tested until tomorrow morning. In the meantime I have a couple questions/comments so we can save a round trip of reviewing if you catch this before then: "The comments for the include files are unnecessary. The order of include files should be (1) local headers, (2) headers that are specific to the projects and (3) C++ headers. It avoid unnecessary loading." Just to check: google's coding style says 1) include the header your implementing 2) C/C++ headers 3) project files, do we want to stick with google here? I have no preference I'd just like to make sure we're both on the same page. "There is a race-condition here. You're effectively hitting https://github.com/haohui/libhdfspp/issues/31. You'll need to capture the promise into the closure of the handler. And please wrap them as a template to avoid copying code." a) We should actually be safe in this case. Looking back at the old code I noticed it wasn't really a race condition so much as a dangling reference issue that tended to show up during races. The code that caused this was "return stat.get_future.get()". The promise, stat, was stack allocated so when you call get_future you get back an rvalue that holds a reference to data contained in (or otherwise managed by) the stack allocated object. Because get is being called on rvalue nothing was left to keep the original stat object alive as the scope ended so in some cases stat's destructor would get called, or the stack was unwound and rewound past that address, before the future's get method returned. It would be similar to doing "stringstream ss;; return ss.str().c_str()". In this case it is guaranteed that the promise object stays alive for the duration of the call. b) It turns out that creating one synchronization template is tricky due to a compiler bug in GCC. The only way I can think of doing this correctly with a single function involves using a variadic capture list to create a wrapper callback (to set the future) and then calling the real callback from there. GCC doesn't seem to handle unpacking variadic arguments in capture lists in a lot of versions (mine included). I'm going to put a little more time into finding an alternative implementation but it looks like I may have to create 1 template for each number of arguments; it's better than nothing and portable. If you happen to have some time to think about this and come up with something that only needs one templated call I'd be happy to test it on GCC and use it. Now that the FileHandle and FileSystem method implementations are separated from the C API wrappers I think it makes sense to put that all of the C++ into a namespace to keep things clean. Do you have any preference about namespace nesting/naming conventions here? I'm thinking hdfs::c_bindings should suffice. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch, > HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch, > HDFS-8766.HDFS-8707.009.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14971680#comment-14971680
]
Haohui Mai commented on HDFS-8766:
--
{code}
+add_library(bindings hdfs.cc)
+add_dependencies(bindings fs rpc reader proto common fs rpc reader proto
common)
{code}
It makes more sense to put all the files further down to {{bindings/c}} as
there will be JNI bindings in {{bindings/jni}}.
{code}
+#include "stdint.h"
+#include "errno.h"
{code}
Should be {{cstdint}} and {{cerrno}}
{code}
+#include "stdint.h"
+#include "errno.h"
+#include "../../../libhdfs/include/hdfs/hdfs.h"
+
+#include "libhdfspp/hdfs.h"
+#include "libhdfspp/status.h"
+#include "fs/filesystem.h"
+#include "common/hdfs_public_api.h"
+
+#include //to_string, string
+#include //promise, future
+#include //make_shared
+#include
+#include
+#include
+
{code}
The comments for the include files are unnecessary. The order of include files
should be (1) local headers, (2) headers that are specific to the projects and
(3) C++ headers. It avoid unnecessary loading.
{code}
+#include "../../../libhdfs/include/hdfs/hdfs.h"
{code}
Headers that are external to the projects are supposed to be included via
bracket. It makes sense to add {{libhdfs/include}} in the
{{include_directories}} in {{CMakeLists.txt}} so that you can use {{#include
}} here.
{code}
+int HdfsInternal::AddWorkerThread() {
...
{code}
Let's separate the implementation of {{HdfsInternal}} and the C bindings into
different files to make the tasks HDFS-9144 easier.
{code}
+//-
+// Begin public C API implementations
+//-
{code}
It makes sense to use the {{/** ... **/}} types of comments to be consistent
with the coding styles (which is used by libhdfspp and the Java implementation).
I'll comment on implementation details separately.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch,
> HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch,
> HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch,
> HDFS-8766.HDFS-8707.009.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14971767#comment-14971767
]
Haohui Mai commented on HDFS-8766:
--
{code}
+typedef struct hdfsFile_internal {
{code}
This will tie the C++ APIs that are proposed in HDFS-9144 with the C bindings.
I suggest the following pattern:
{code}
namespace {
class FileHandle { ... };
}
struct hdfsFile_internal {
FileHandle handle;
hdfsFile_internal(FileHandle &) : handle(std::move(handle)) {}
}
FileHandle *unwrap(hdfsFile f) {
return f->handle;
}
hdfsFile wrap(FileHandle &) {
return new hdfsFile_internal(handle);
}
{code}
{code}
+ std::promise stat;
+ std::future future = stat.get_future();
+
+ // wrap async FileSystem::Open with promise to make it a blocking call
+ InputStream *input_stream = nullptr;
+ auto h = [, _stream](const Status , InputStream *is) {
+stat.set_value(s);
+input_stream = is;
+ };
+
+ file_system_->Open(path, h);
+
+ // block until promise is set
+ auto s = future.get();
{code}
There is a race-condition here. You're effectively hitting
https://github.com/haohui/libhdfspp/issues/31. You'll need to capture the
promise into the closure of the handler. And please wrap them as a template to
avoid copying code.
{code}
+ // note: order of members matters a lot here.
+ std::unique_ptr service_;
+ // std::thread needs to join before deletion
+ struct WorkerDeleter {
+void operator()(std::thread *t) {
+ t->join();
+ delete t;
+}
{code}
Instead of relying the orders to ensure correctness, how about extracting them
as functions to explicitly enforce the order?
{code}
+hdfsFile HdfsInternal::OpenFileForRead(const std::string ) {
+bool HdfsInternal::Connect(const char *nn, tPort port, unsigned int threads) {
{code}
It makes more sense to for these methods to return a {{Status}} object (which
contains the information about {{errno}}). The C API can set the errno based on
the information. For example,
{code}
+Status HdfsInternal::OpenFileForRead(const std::string , FileHandle
**handle) {
{code}
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch,
> HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch,
> HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch,
> HDFS-8766.HDFS-8707.009.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14970096#comment-14970096
]
Hadoop QA commented on HDFS-8766:
-
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | pre-patch | 5m 50s | Pre-patch HDFS-8707 compilation
is healthy. |
| {color:green}+1{color} | @author | 0m 0s | The patch does not contain any
@author tags. |
| {color:red}-1{color} | tests included | 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. |
| {color:red}-1{color} | javac | 1m 25s | The patch appears to cause the
build to fail. |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12768160/HDFS-8766.HDFS-8707.009.patch
|
| Optional Tests | javac unit |
| git revision | HDFS-8707 / 6828dd5 |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/13141/console |
This message was automatically generated.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch,
> HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch,
> HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch,
> HDFS-8766.HDFS-8707.009.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14967515#comment-14967515 ] Haohui Mai commented on HDFS-8766: -- Thanks for the comments, Bob. bq. I'm unsure if you see all of them as show-stoppers and some as small nits; it would be nice if you could clarify that when we have the narrow band of Jira to communicate. The earlier comments are fairly mechanical. They usually come from Jenkins if Jenkins is working. We have strong emphasis to minimize code duplication, consistent coding styles to allow other people in the community to participate. I see the above comments as the low bars before the patch is ready to be reviewed. For 1a and 1b, please refer to HDFS-9253. bq. As we discussed, we will be losing a valuable debugging aid that has sussed out issues in integration testing already. I don't think we should ship with this in-place, but I would like to keep it for the short term. Again, let's make a "Before we merge with trunk" Jira to capture that so we can both reap the benefits of it during the high-risk portion of our development and not merge it into trunk. While dev branches are usually fast moving, we have the same level of expectations on quality controls. We also try to maintain a very clear commit history so that it is easy to merge to code back to trunk. That is being said, it's very undesirable to commit macros and debugging codes into the repository. However, the git workflows should allow you to keep these codes locally and to be applied when they are required. We also have strong emphasis keep patches small and independent so that they can be easily reviewed and committed. They will allow faster development in the dev branch. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch, > HDFS-8766.HDFS-8707.007.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14967884#comment-14967884
]
Hadoop QA commented on HDFS-8766:
-
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | pre-patch | 5m 56s | Pre-patch HDFS-8707 compilation
is healthy. |
| {color:green}+1{color} | @author | 0m 0s | The patch does not contain any
@author tags. |
| {color:red}-1{color} | tests included | 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. |
| {color:red}-1{color} | javac | 1m 23s | The patch appears to cause the
build to fail. |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12767859/HDFS-8766.HDFS-8707.008.patch
|
| Optional Tests | javac unit |
| git revision | HDFS-8707 / 6828dd5 |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/13113/console |
This message was automatically generated.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch,
> HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch,
> HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14965443#comment-14965443
]
Hadoop QA commented on HDFS-8766:
-
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | pre-patch | 7m 34s | Pre-patch HDFS-8707 compilation
is healthy. |
| {color:green}+1{color} | @author | 0m 0s | The patch does not contain any
@author tags. |
| {color:red}-1{color} | tests included | 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. |
| {color:red}-1{color} | javac | 2m 3s | The patch appears to cause the
build to fail. |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12767627/HDFS-8766.HDFS-8707.007.patch
|
| Optional Tests | javac unit |
| git revision | HDFS-8707 / ea310d7 |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/13085/console |
This message was automatically generated.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch,
> HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch,
> HDFS-8766.HDFS-8707.007.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14964024#comment-14964024 ] James Clampffer commented on HDFS-8766: --- Thanks Bob and Haohui for the feedback. Bob, I'll make those changes first so we can get that landed, then right after do the following: 1) I'm going to open up another jira for cli utilities in an effort to eventually make a c++ implementation of the hadoop CLI tools. I'll turn this stub into a unix-like cat executable so that it can be used for other things as well. Then move onto stat and other simple fs utils. 2) I can get rid of the INVALIDATE_MEMBER_PTR macro and use unique pointers, but I'd like to add a deleter to the smart pointers that nulls the pointer value on destruction. This sort of debugging tool would be more useful in the pipeline and block reader implementations because those are where most of the dangling pointers and references tend to originate. 3) Opened HDFS-9265 for this. It's more broken than I originally thought which explains all the double deletes that often manifest as calls to pure virtual methods I've been seeing in crashes. Inputstream's state owns a unique_ptr to the reader, and the reader is handing out shared_ptrs of itself to the continuation pipeline. So it's double deleting by design rather than a memory stomp. Fixing the make_shared_from_this usage alone isn't enough. I can take a stab at refactoring and fixing this sometime this week. 4) This is the conventional posix signature, this was intentional because google's guide says follow existing conventions if they are already well known. 5) Agree with Bob, he's already going to be refactoring a lot so I'll keep everything in the same place and let him move that. So the patch will cut out the c_api_test executable (1), add unique_ptrs (2) + get rid of INVALIDATE_MEMBER_PTR, omit (3) because it's in another jira, not change (4) because it falls into "exceptions to naming rules", and not change (5) because it's going to be handled in HDFS-9144. I'll also get rid of hdfs.h and use the one in the tree. Haohui, can you think of anything else I'm going to need to change? I think the current implementation, once rebased, should be sufficient to get on the development branch so Bob can begin HDFS-9144, and then I'll submit another patch with the changes I mentioned above tomorrow or Wednesday. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14963963#comment-14963963 ] Bob Hansen commented on HDFS-8766: -- Haohui - thanks for those explicit and actionable items. I'm unsure if you see all of them as show-stoppers and some as small nits; it would be nice if you could clarify that when we have the narrow band of Jira to communicate. 1a. While copied code is The Devil, I believe strongly that in the current state of the code, we should not put forward declarations into a header file that we don't implement. That will just lead to linker errors later and sad consumers.Perhaps we should explicitly rename it to "hdfspp.h" to disambiguate it. 1b. I think we can make forward progress by landing this code with the integration tests as-is and having a separate Jira to integrate the integration test functionality with the existing tests. Bringing that scope into this bug is a major effort, and deserves its own Jira. 2. As we discussed, we will be losing a valuable debugging aid that has sussed out issues in integration testing already. I don't think we should ship with this in-place, but I would like to keep it for the short term. Again, let's make a "Before we merge with trunk" Jira to capture that so we can both reap the benefits of it during the high-risk portion of our development and not merge it into trunk. 3. This code won't pass integration test without that fix. As a matter of good form, it is good to break changes into atomic work units that stand alone, but as we discussed, we should be flexible during this phase of the development lifecycle so we don't get held up on issues of form. Can we accept that it's an improvement and accept? 4. I'm sure this is a minor nit that we can fix as we go, but I hope we're all on board that this is not a show-stopper. 5. Our plan is to take care of that with HDFS-9144 after this changeset lands. This is good; that will be better. The code does need to be re-factored to land on top of HDFS-9207. James - can you put together a patch that will apply to the current state of the HDFS-8707 branch (and perhaps fix the pread --> Pread renaming while you're there) and submit a patch that we can land? Haohui - can we land this patch with your points #1, 2, and 5 captured in new Jiras? > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14961217#comment-14961217
]
Haohui Mai commented on HDFS-8766:
--
Thanks for updating the patch. Some comments:
1. Remove {[hdfs.h}}, {{c_api_test.cc}} in this patch and reuse the code
existing repo, as HDFS-9207 and HDFS-9253 have landed.
2. Remove {{hdfs_macros.h}} and use {{unique_ptr}}.
3. Separate the bug fixes in
{{hadoop-hdfs-project/hadoop-hdfs-client/src/main/native/libhdfspp/lib/reader/remote_block_reader_impl.h}}
into another jira.
4. Rename {{HdfsInternal::pread}} to {{HdfsInternal::Pread}} to follow the
Google C++ styling guide.
5. Separate the implementation of the C API and the definition of
{{HdfsInternal}} in different files.
Haven't closely looked into the logics yet -- the patch should be much smaller
and cleaner after the above changes. Will do it in the next round.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch,
> HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14959780#comment-14959780
]
Hadoop QA commented on HDFS-8766:
-
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:red}-1{color} | patch | 0m 0s | The patch command could not apply
the patch during dryrun. |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12766903/HDFS-8766.HDFS-8707.006.patch
|
| Optional Tests | javac unit |
| git revision | HDFS-8707 / 4cd3b99 |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/13014/console |
This message was automatically generated.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch,
> HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14958164#comment-14958164 ] Haohui Mai commented on HDFS-8766: -- As far as I can see there are multiple issues with the current patch -- I don't think it's ready yet. I'll give more detailed comment once it's rebased on top of HDFS-9207. Assigning it back to James. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: Haohui Mai > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14955252#comment-14955252 ] Haohui Mai commented on HDFS-8766: -- I'd like to proceed with this patch after HDFS-9207. The reason is that the patch contains both integration tests and the hdfs.h file which duplicates the existing code in libhdfs. It makes sense to go with HDFS-9207 first and minimize this patch. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14953933#comment-14953933
]
Hadoop QA commented on HDFS-8766:
-
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:red}-1{color} | patch | 0m 0s | The patch command could not apply
the patch during dryrun. |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12766186/HDFS-8766.HDFS-8707.005.patch
|
| Optional Tests | javac unit |
| git revision | HDFS-8707 / 1b05389 |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/12940/console |
This message was automatically generated.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch,
> HDFS-8766.HDFS-8707.005.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14941988#comment-14941988 ] Haohui Mai commented on HDFS-8766: -- bq. I added a simple timeout to clear out the bad datanodes after a specified period of time (default to 2 minutes). I think that should be sufficient for the initial API The default timeout is 10 minutes and the timeout should be bounded to every single data node. This functionality requires a specific gmock test. I think it makes sense to separate the integration test to an another jira. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14939954#comment-14939954
]
Hadoop QA commented on HDFS-8766:
-
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | pre-patch | 5m 14s | Pre-patch HDFS-8707 compilation
is healthy. |
| {color:green}+1{color} | @author | 0m 0s | The patch does not contain any
@author tags. |
| {color:red}-1{color} | tests included | 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. |
| {color:red}-1{color} | javac | 1m 22s | The patch appears to cause the
build to fail. |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12764625/HDFS-8766.HDFS-8707.003.patch
|
| Optional Tests | javac unit |
| git revision | HDFS-8707 / 3668778 |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/12761/console |
This message was automatically generated.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14940047#comment-14940047 ] James Clampffer commented on HDFS-8766: --- Thanks for the review Bob. 1) Thanks for pointing that out. I'm not sure how the shared_ptr snuck back in; I'll remove that. 2) The thinking there was to avoid making the user do a cast if they just got some fresh space with malloc or wanted to do something else odd with memory in C. I was on the fence about using char *. I have no strong preference either way. 3) That was just a remnant of when I was doing a quick test/prototype; libhdfs used structs and I took the declaration from there. I just left it that way because there wasn't much to hide yet. Originally I had the exposed C functions touching input_stream_ and file_system_ on hdfs_internal and hdfsFile_internal a lot more so I wanted members to be public by default. The only function that does that now is hdfsFileIsOpenForRead and the implementation for that should be in hdfsFile_internal anyway. I can switch it over to a class and make the members private. And yes, I'll put a note into HDFS-8790 about managed pointers and commit them with that patch. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14940089#comment-14940089
]
Haohui Mai commented on HDFS-8766:
--
Thanks for updating the patch!.
{code}
+ if (!s.ok()) {
+// for now assume this was a down DN, not going to reinvent HDFS-9103
+if (s.code() == Status::kResourceUnavailable) {
+ errno = ECOMM;
+ bad_datanodes_.insert(datanode);
+}
+// otherwise it was a real error so indicate that something's gone wrong
+return -1;
+ }
+
+ return (ssize_t)read_count;
+}
{code}
There are several issues:
(1) It makes sense to refactor the above code into a function.
(2) the errno should be {{EINTR}}.
(3) When a DN added into the dead DN list, it needs to be removed from the list
after a pre-configured time.
(4) This function needs a unit test in gmock.
Nit: Things like class name should be in caml cases instead of underscores. In
order to glue them with hdfs.h, an easy way is to add {{typedef ClassName
hdfs_internal}} in the implementation.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14940090#comment-14940090 ] Haohui Mai commented on HDFS-8766: -- bq. For now I'd really like to avoid shared_ptr and unique_ptr until I move the stress tests from github to this for HDFS-8790. I wouldn't be surprised if some bugs shake out when running large stress tests so I'd like to make things easier to debug using the pointer invalidation macro. I think it makes sense to commit clean codes and separate the debugging code into a separate jira. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14940001#comment-14940001 ] Bob Hansen commented on HDFS-8766: -- Minor nits: * Still have a shared_ptr to a promise in hdfsFile_internal::pread * Is there a reason getErrorStr doesn't take a char *? * Is there a reason hdfs_internal is a struct rather than a class? Can you file a follow-up JIRA or add to HDFS-8790 to convert raw pointers to managed pointers? > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14940462#comment-14940462
]
Hadoop QA commented on HDFS-8766:
-
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | pre-patch | 6m 28s | Pre-patch HDFS-8707 compilation
is healthy. |
| {color:green}+1{color} | @author | 0m 0s | The patch does not contain any
@author tags. |
| {color:red}-1{color} | tests included | 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. |
| {color:red}-1{color} | javac | 1m 35s | The patch appears to cause the
build to fail. |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12764686/HDFS-8766.HDFS-8707.004.patch
|
| Optional Tests | javac unit |
| git revision | HDFS-8707 / 3668778 |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/12765/console |
This message was automatically generated.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch,
> HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14939049#comment-14939049
]
Hadoop QA commented on HDFS-8766:
-
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | pre-patch | 5m 17s | Pre-patch HDFS-8707 compilation
is healthy. |
| {color:green}+1{color} | @author | 0m 0s | The patch does not contain any
@author tags. |
| {color:red}-1{color} | tests included | 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. |
| {color:red}-1{color} | javac | 1m 23s | The patch appears to cause the
build to fail. |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12764511/HDFS-8766.HDFS-8707.002.patch
|
| Optional Tests | javac unit |
| git revision | HDFS-8707 / 3668778 |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/12755/console |
This message was automatically generated.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14936199#comment-14936199
]
Haohui Mai commented on HDFS-8766:
--
Thanks for updating the patch.
{code}
+add_subdirectory(compatibility)
{code}
It should be named as {{binding/c}}.
{code}
+ virtual ~IoServiceImpl(){}
+
{code}
It doesn't seem to be necessary.
{code}
+#include "../../include/compatibility/hdfs.h"
{code}
The include paths are incorrect. It should be "compatibility/hdfs.h". There are
many places like this.
{code}
+struct hdfsFile_internal {
+ hdfsFile_internal(InputStream *is) : inputStream(is){};
...
{code}
The naming is far off for the Google C++ style guide (which is the one this
project is used for). Please refer to
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Naming and
adhere to the naming conventions.
{code}
+ InputStream *inputStream;
{code}
Looks like {{hdfsFile_internal}} owns the object. It should be {{unique_ptr}}.
{code}
+ bool connect(const char *nn, tPort port, unsigned int threads = 1) {
{code}
Please separate the declaration and the implementation of the function.
{code}
if (nullptr == file)
{code}
Another examples that is off the styling guide. Please change it to {{if
(file)}}. There are multiple snippets like that.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14934158#comment-14934158
]
Hadoop QA commented on HDFS-8766:
-
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | pre-patch | 5m 23s | Pre-patch HDFS-8707 compilation
is healthy. |
| {color:green}+1{color} | @author | 0m 0s | The patch does not contain any
@author tags. |
| {color:red}-1{color} | tests included | 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. |
| {color:red}-1{color} | javac | 1m 21s | The patch appears to cause the
build to fail. |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12764097/HDFS-8766.HDFS-8707.001.patch
|
| Optional Tests | javac unit |
| git revision | HDFS-8707 / 3668778 |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/12721/console |
This message was automatically generated.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch,
> HDFS-8766.HDFS-8707.001.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14905406#comment-14905406
]
Haohui Mai commented on HDFS-8766:
--
Thanks for the work.
It looks like that the code needs a lot of clean ups. The code needs to follow
the Google C++ styling guide and be formatted using clang-format.
I think it's important to have several abstractions. There should be C++
classes that correspond FileSystem / FileHandle as Bob mentioned.
{code}
+ ssize_t pread(void *buf, size_t nbyte, off_t offset) {
+auto stat = std::make_shared();
+std::future future(stat->get_future());
+
+//wrap async call with promise/future to make it blocking
+size_t readCount = 0;
+std::string datanode;
+auto h = [stat, , ](const Status , const std::string
, size_t bytes) {
+ stat->set_value(s);
+ readCount = bytes;
+ datanode = dn;
+};
+
+inputStream->PositionRead(buf, nbyte, offset, bad_datanodes, h);
+
+//wait for async to finish
+auto s = future.get();
+
+if(!s.ok()) {
+ //for now assume this was a down DN, not going to reinvent HDFS-9103
+ if(s.code() == Status::kResourceUnavailable) {
+errno = ECOMM;
+bad_datanodes.insert(datanode);
+ }
+ //otherwise it was a real error so indicate that something's gone wrong
+ return -1;
+}
{code}
It should be a generic template to abstract the operations of wrapping async
calls to synchronous calls.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14905218#comment-14905218
]
Hadoop QA commented on HDFS-8766:
-
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | pre-patch | 6m 8s | Pre-patch HDFS-8707 compilation
is healthy. |
| {color:green}+1{color} | @author | 0m 0s | The patch does not contain any
@author tags. |
| {color:red}-1{color} | tests included | 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. |
| {color:red}-1{color} | javac | 1m 34s | The patch appears to cause the
build to fail. |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12761957/HDFS-8766.HDFS-8707.000.patch
|
| Optional Tests | javac unit |
| git revision | HDFS-8707 / 5d912ea |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/12632/console |
This message was automatically generated.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14905488#comment-14905488 ] James Clampffer commented on HDFS-8766: --- Thanks for the input! Re: It looks like that the code needs a lot of clean ups. The code needs to follow the Google C++ styling guide and be formatted using clang-format. Agreed. I didn't know we were using automated tools to check the code style, I'll look into that. What's the biggest motivation for choosing Google's C++ standards on a new project like this? The main thing that bothers me is never using exceptions particularly in the context of constructors, I'd rather let RAII handle cleanup and avoid lots of checks at the call site. I'm not using much RAII at the moment because I'd like to wait until we settle on an object ownership convention and keep it consistent throughout the library. Re: I think it's important to have several abstractions. There should be C++ classes that correspond FileSystem / FileHandle as Bob mentioned. We should probably spend some time talking about what we want out of the different abstractions before too much more work is done, my rough ideas for implementing FileSystem / FileHandle in order of personal preference would be: 1) Turn hdfs_internal into FileSystem and hdfsFile_internal into FileHandle; this is the approach I started taking here. It separates the compatibility layer from changes in the underlying classes and provides simple examples about how to use the lower level async APIs. A nice bonus here is that it gives a good opportunity to hide some of the type traits and templates for people who just want a simple object oriented interface. 2) Take the stuff from compatibility and add it directly to the InputStream/FileSystem objects. Less code to maintain but pushes a lot of code that isn't too useful for C++11 programmers into the main classes. 3) Implement the FileHandle/FileSystem directly on top of the block_reader, rpc_engine, and IoService classes. I'd like to avoid this because it seems like a whole lot of code would be duplicated. It seems like a perfect time to begin working on HDFS-9115 as a group so we are all on the same page. Re: It should be a generic template to abstract the operations of wrapping async calls to synchronous calls. I strongly agree; before I do much more work on this I'm going to write a little macro or templated class to wrap up async calls. > Implement a libhdfs(3) compatible API > - > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client >Reporter: James Clampffer >Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-8766) Implement a libhdfs(3) compatible API
[
https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14905513#comment-14905513
]
Haohui Mai commented on HDFS-8766:
--
bq. Agreed. I didn't know we were using automated tools to check the code
style, I'll look into that. What's the biggest motivation for choosing Google's
C++ standards on a new project like this? The main thing that bothers me is
never using exceptions particularly in the context of constructors, I'd rather
let RAII handle cleanup and avoid lots of checks at the call site. I'm not
using much RAII at the moment because I'd like to wait until we settle on an
object ownership convention and keep it consistent throughout the library.
libhdfs++ is designed to not use C++ exceptions at all. Please do not pull in
any existing code form the legacy libhdfs3 codebase which heavily use c++
exceptions. For resource clean ups, please refer to how the {{FileSystem}}
object is implemented.
> Implement a libhdfs(3) compatible API
> -
>
> Key: HDFS-8766
> URL: https://issues.apache.org/jira/browse/HDFS-8766
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
>Reporter: James Clampffer
>Assignee: James Clampffer
> Attachments: HDFS-8766.HDFS-8707.000.patch
>
>
> Add a synchronous API that is compatible with the hdfs.h header used in
> libhdfs and libhdfs3. This will make it possible for projects using
> libhdfs/libhdfs3 to relink against libhdfspp with minimal changes.
> This also provides a pure C interface that can be linked against projects
> that aren't built in C++11 mode for various reasons but use the same
> compiler. It also allows many other programming languages to access
> libhdfspp through builtin FFI interfaces.
> The libhdfs API is very similar to the posix file API which makes it easier
> for programs built using posix filesystem calls to be modified to access HDFS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
