Generic libcephfs Java wrappers

Generic libcephfs Java wrappers.

This is the first of a patch series that can eventually replace the current Java wrappers that are Hadoop specific. This group implements enough of the libcephfs API to support Hadoop functionality, includes Debian packaging, autoconf magics that look for default Java installations, and some basic JUnit tests.

The patches are available in the git repository at:

git://github.com/noahdesu/ceph.git wip/java-cephfs

Noah Watkins (3):
java: add Java and C++ source files
java: setup autotools to build cephfs-java
debian: add libcephfs-java package

 configure.ac                                       |   74 ++
 debian/.gitignore                                  |    1 +
 debian/control                                     |    8 +-
 debian/libceph1-java.install                       |    2 +
 debian/rules                                       |    1 +
 src/Makefile.am                                    |   15 +-
 src/java/.gitignore                                |    4 +
 src/java/Makefile.am                               |   49 +
 src/java/README                                    |   43 +
 src/java/build.xml                                 |   67 ++
 .../java/net/newdream/ceph/fs/CephConstants.java   |   38 +
 .../java/net/newdream/ceph/fs/CephDirectory.java   |   84 ++
 .../java/net/newdream/ceph/fs/CephException.java   |   21 +
 .../ceph/fs/CephInvalidStateException.java         |   11 +
 src/java/java/net/newdream/ceph/fs/CephMount.java  |  360 +++++++
 .../net/newdream/ceph/fs/CephNativeLoader.java     |   16 +
 src/java/java/net/newdream/ceph/fs/CephProxy.java  |  217 ++++
 src/java/java/net/newdream/ceph/fs/CephStat.java   |   15 +
 .../java/net/newdream/ceph/fs/CephStatVFS.java     |   14 +
 src/java/java/net/newdream/ceph/fs/CephStruct.java |   33 +
 src/java/native/libcephfs_jni.cc                   | 1037 ++++++++++++++++++++
 src/java/test/CephMountCreateTest.java             |   76 ++
 src/java/test/CephMountTest.java                   |   72 ++
 23 files changed, 2256 insertions(+), 2 deletions(-)
 create mode 100644 debian/libceph1-java.install
 create mode 100644 src/java/.gitignore
 create mode 100644 src/java/Makefile.am
 create mode 100644 src/java/README
 create mode 100644 src/java/build.xml
 create mode 100644 src/java/java/net/newdream/ceph/fs/CephConstants.java
 create mode 100644 src/java/java/net/newdream/ceph/fs/CephDirectory.java
 create mode 100644 src/java/java/net/newdream/ceph/fs/CephException.java
 create mode 100644 src/java/java/net/newdream/ceph/fs/CephInvalidStateException.java
 create mode 100644 src/java/java/net/newdream/ceph/fs/CephMount.java
 create mode 100644 src/java/java/net/newdream/ceph/fs/CephNativeLoader.java
 create mode 100644 src/java/java/net/newdream/ceph/fs/CephProxy.java
 create mode 100644 src/java/java/net/newdream/ceph/fs/CephStat.java
 create mode 100644 src/java/java/net/newdream/ceph/fs/CephStatVFS.java
 create mode 100644 src/java/java/net/newdream/ceph/fs/CephStruct.java
 create mode 100644 src/java/native/libcephfs_jni.cc
 create mode 100644 src/java/test/CephMountCreateTest.java
 create mode 100644 src/java/test/CephMountTest.java

Signature · 5 lines

1.7.5.4
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hey Noah-

This looks great! I'd love to merge this for v0.44, but need your
Signed-off-by: on the patches. Also, if you want to switch to com.ceph while you're at it, that'd be good too.

I have a trivial makefile fix in wip-java you might want to look at.

Has any of the hadoop side work been done to use this yet? Is there anything we can do help get that in a state where we can send it upstream?

Thanks!
sage

Quoted 64 lines · Noah Watkins

Generic libcephfs Java wrappers.

This is the first of a patch series that can eventually replace the current Java wrappers that are Hadoop specific. This group implements enough of the libcephfs API to support Hadoop functionality, includes Debian packaging, autoconf magics that look for default Java installations, and some basic JUnit tests.

The patches are available in the git repository at:

git://github.com/noahdesu/ceph.git wip/java-cephfs

Noah Watkins (3):
java: add Java and C++ source files
java: setup autotools to build cephfs-java
debian: add libcephfs-java package

configure.ac                                       |   74 ++
debian/.gitignore                                  |    1 +
debian/control                                     |    8 +-
debian/libceph1-java.install                       |    2 +
debian/rules                                       |    1 +
src/Makefile.am                                    |   15 +-
src/java/.gitignore                                |    4 +
src/java/Makefile.am                               |   49 +
src/java/README                                    |   43 +
src/java/build.xml                                 |   67 ++
.../java/net/newdream/ceph/fs/CephConstants.java   |   38 +
.../java/net/newdream/ceph/fs/CephDirectory.java   |   84 ++
.../java/net/newdream/ceph/fs/CephException.java   |   21 +
.../ceph/fs/CephInvalidStateException.java         |   11 +
src/java/java/net/newdream/ceph/fs/CephMount.java  |  360 +++++++
.../net/newdream/ceph/fs/CephNativeLoader.java     |   16 +
src/java/java/net/newdream/ceph/fs/CephProxy.java  |  217 ++++
src/java/java/net/newdream/ceph/fs/CephStat.java   |   15 +
.../java/net/newdream/ceph/fs/CephStatVFS.java     |   14 +
src/java/java/net/newdream/ceph/fs/CephStruct.java |   33 +
src/java/native/libcephfs_jni.cc                   | 1037 ++++++++++++++++++++
src/java/test/CephMountCreateTest.java             |   76 ++
src/java/test/CephMountTest.java                   |   72 ++
23 files changed, 2256 insertions(+), 2 deletions(-)

create mode 100644 debian/libceph1-java.install
create mode 100644 src/java/.gitignore
create mode 100644 src/java/Makefile.am
create mode 100644 src/java/README
create mode 100644 src/java/build.xml
create mode 100644 src/java/java/net/newdream/ceph/fs/CephConstants.java create mode 100644 src/java/java/net/newdream/ceph/fs/CephDirectory.java create mode 100644 src/java/java/net/newdream/ceph/fs/CephException.java create mode 100644 src/java/java/net/newdream/ceph/fs/CephInvalidStateException.java create mode 100644 src/java/java/net/newdream/ceph/fs/CephMount.java create mode 100644 src/java/java/net/newdream/ceph/fs/CephNativeLoader.java create mode 100644 src/java/java/net/newdream/ceph/fs/CephProxy.java create mode 100644 src/java/java/net/newdream/ceph/fs/CephStat.java create mode 100644 src/java/java/net/newdream/ceph/fs/CephStatVFS.java create mode 100644 src/java/java/net/newdream/ceph/fs/CephStruct.java create mode 100644 src/java/native/libcephfs_jni.cc
create mode 100644 src/java/test/CephMountCreateTest.java
create mode 100644 src/java/test/CephMountTest.java

Signature · 5 lines

1.7.5.4
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

I've kept an eye on how much things are deviating from the current implementation and its pretty minimal, so I think producing a new version to use the generic libcephfs interface will happen quickly.

As for producing an upstream patch, I think the two biggest concerns are:

1. Reporting hostname/racks rather than IPs for object location.
2. Address (or at least document) the time synchronization issue.

These are largely orthogonal to getting the code ready for upstream, but may be a "political" road block? I can put together the details of these two issues in a separate thread…

CephMount::conf_get requires a buf_size. But we freely adjust that if we need more space, so is there any point to it? I can't see callers really having a better idea of size than we do.

(Oh, now I see there's another one that doesn't take buf_size. Do we have a use case for the with-size option, though?) Similarly with getdnames

We don't implement CephMount::mkdir, just mkdirs? I *can* see use cases for not wanting recursive creates…

The ordering is a little funny for some functions — we see close() before open(), for instance. Maybe we should reorder these?

set_default_file_replication doesn't actually do anything internally — I thought that was documented somewhere. Even if it's not, it certainly should be here (and elsewhere, patches welcome! ;). (It passes the value to the Client, and the Client passes it to the MDS, but the MDS never looks at it). I like the states and require_state() stuff. It's making me notice that you need a little more documentation on stuff like closedir and what happens to the CephDirectory under success and error returns. :)

The CHECK_ARG_* stuff could use some docs. I see now that CHECK_ARG_BOUNDS is expecting a full comparison, so it's probably just my lack of preprocessor idiom knowledge, but it weirded me out. And CHECK_ARG_COND appears to be unused. do we need mount() when we can just run mount(null)? Similarly, I don't think we need an if-else in CephMountCreateTest::setupMount. :)

Quoted 8 lines

I've kept an eye on how much things are deviating from the current implementation and its pretty minimal, so I think producing a new version to use the generic libcephfs interface will happen quickly.

As for producing an upstream patch, I think the two biggest concerns are:

1. Reporting hostname/racks rather than IPs for object location.
2. Address (or at least document) the time synchronization issue.

Quoted 1 line

These are largely orthogonal to getting the code ready for upstream, but may be a "political" road block? I can put together the details of these two issues in a separate thread…

Please do! I know I've seen most of these issues come up, but I'm still a bit fuzzy on the time sync issue.

Thanks!
-Greg

Quoted 1 line · Greg Farnum

CephMount::conf_get requires a buf_size. But we freely adjust that if we need more space, so is there any point to it? I can't see callers really having a better idea of size than we do.

Good point. I can't think of a reason to publicly expose the buffer length param in CephMount to users, but CephProxy is a private interface. On a related a related note:

int ceph_conf_get(struct ceph_mount_info *cmount, const char *option, char *buf, size_t len);

shouldn't the 'buf' parameter be char ** if libcephfs is going to be doing a re-alloc, or is there some voodoo behind the scenes?

Quoted 2 lines

(Oh, now I see there's another one that doesn't take buf_size. Do we have a use case for the with-size option, though?) Similarly with getdnames

Yes, the default buf_size interfaces are meant to be friendly, while the explicit buf_size interfaces are primarily meant for the unit tests so that the buffer expansion logic can be tested. It would be safe to leave it public, but maybe it is better for it to be protected and used explicitly by the unit tests.

Quoted 1 line

We don't implement CephMount::mkdir, just mkdirs? I *can* see use cases for not wanting recursive creates…

I was going for the minimum to support Hadoop. I'll stick mkdirs on the short list of API expansion TODOs.

Quoted 1 line

I like the states and require_state() stuff. It's making me notice that you need a little more documentation on stuff like closedir and what happens to the CephDirectory under success and error returns. :)

The state checking is really helpful to catch set faults when blindly casting Java long to void* :) I wonder if there is a way to push the state checking even further down in libcephfs? Currently all errors are communicated through a generic CephException, but making things more specific as we go along will be nice.

Quoted 9 lines · Noah Watkins

Quoted 1 line · Greg Farnum

CephMount::conf_get requires a buf_size. But we freely adjust that if we need more space, so is there any point to it? I can't see callers really having a better idea of size than we do.

Good point. I can't think of a reason to publicly expose the buffer length param in CephMount to users, but CephProxy is a private interface. On a related a related note:

int ceph_conf_get(struct ceph_mount_info *cmount, const char *option, char *buf, size_t len);

shouldn't the 'buf' parameter be char ** if libcephfs is going to be doing a re-alloc, or is there some voodoo behind the scenes?

libcephfs doesn't do a re-alloc there; I was talking about the Java stuff when I said it freely adjusts. :)

Quoted 3 lines

Quoted 1 line

I like the states and require_state() stuff. It's making me notice that you need a little more documentation on stuff like closedir and what happens to the CephDirectory under success and error returns. :)

The state checking is really helpful to catch set faults when blindly casting Java long to void* :) I wonder if there is a way to push the state checking even further down in libcephfs? Currently all errors are communicated through a generic CephException, but making things more specific as we go along will be nice.

I'm not sure how we'd push it farther down in a way that's any cleaner, did you have some thoughts or just a wish to not do it in Java?

Oh, and I forgot something important last time: there's no licensing associated with any of these files right now. :) I think we talked previously about making it Apache, but whatever it is should be clearly noted!

-Greg

Quoted 11 lines · Greg Farnum

Quoted 9 lines · Noah Watkins

Quoted 1 line · Greg Farnum

CephMount::conf_get requires a buf_size. But we freely adjust that if we need more space, so is there any point to it? I can't see callers really having a better idea of size than we do.

Good point. I can't think of a reason to publicly expose the buffer length param in CephMount to users, but CephProxy is a private interface. On a related a related note:

int ceph_conf_get(struct ceph_mount_info *cmount, const char *option, char *buf, size_t len);

shouldn't the 'buf' parameter be char ** if libcephfs is going to be doing a re-alloc, or is there some voodoo behind the scenes?

libcephfs doesn't do a re-alloc there; I was talking about the Java stuff when I said it freely adjusts. :)

The documentation seems to indicate that it does do a malloc. Maybe it is just out of date?

/* Returns a configuration value as a string.
* If len is positive, that is the maximum number of bytes we'll write into the
* buffer. If len == -1, we'll call malloc() and set *buf.
* Returns 0 on success, error code otherwise. Returns ENAMETOOLONG if the
* buffer is too short. */

Quoted 5 lines

/* Returns a configuration value as a string.
* If len is positive, that is the maximum number of bytes we'll write into the
* buffer. If len == -1, we'll call malloc() and set *buf.
* Returns 0 on success, error code otherwise. Returns ENAMETOOLONG if the
* buffer is too short. */

Ah. Two things from that:
1) "If len == -1". Which it isn't, here.
2) There is no voodoo, that just isn't going to work. Either it was created incorrectly in reference to md_config_t::get_val, or else code got moved around without updating that documentation. :/ Options including char ** (as you said), char *& (ewww), or…not doing that?

Anybody have thoughts on the right solution?

Quoted 6 lines · Greg Farnum

Quoted 5 lines

/* Returns a configuration value as a string.
* If len is positive, that is the maximum number of bytes we'll write into the
* buffer. If len == -1, we'll call malloc() and set *buf.
* Returns 0 on success, error code otherwise. Returns ENAMETOOLONG if the
* buffer is too short. */

Quoted 5 lines

Ah. Two things from that:
1) "If len == -1". Which it isn't, here.
2) There is no voodoo, that just isn't going to work. Either it was created incorrectly in reference to md_config_t::get_val, or else code got moved around without updating that documentation. :/ Options including char ** (as you said), char *& (ewww), or…not doing that?

Anybody have thoughts on the right solution?

I am partial to APIs that put the burden on the caller to free/expand the buffer in response to -ENAMETOOLONG. /2cents

Quoted 20 lines · Noah Watkins

Quoted 16 lines · Greg Farnum

Quoted 7 lines

/* Returns a configuration value as a string.
* If len is positive, that is the maximum number of bytes we'll write into the
* buffer. If len == -1, we'll call malloc() and set *buf.
* Returns 0 on success, error code otherwise. Returns ENAMETOOLONG if the
* buffer is too short. */

Ah. Two things from that:
1) "If len == -1". Which it isn't, here.
2) There is no voodoo, that just isn't going to work. Either it was created incorrectly in reference to md_config_t::get_val, or else code got moved around without updating that documentation. :/ Options including char ** (as you said), char *& (ewww), or?not doing that?

Anybody have thoughts on the right solution?

Quoted 2 lines

I am partial to APIs that put the burden on the caller to free/expand the buffer in response to -ENAMETOOLONG. /2cents

That sounds good to me. That comment exactly describes the behavior of md_config_t::get_val(). It would also be cleaner if len didn't have a special meaning and md_config_t::get_val() used strings instead of char*. It already uses strings internally, and only the librados C++ api actually uses len=-1.

As for producing an upstream patch, I think the two biggest concerns are:

1. Reporting hostname/racks rather than IPs for object location.
2. Address (or at least document) the time synchronization issue.

Quoted 4 lines · Noah Watkins

As for producing an upstream patch, I think the two biggest concerns are:

1. Reporting hostname/racks rather than IPs for object location.
2. Address (or at least document) the time synchronization issue.

Regarding (1), my last comment in this ticket (http://tracker.newdream.net/issues/1666) seems to indicate that a stat occurs on a client and the mtime in the cached inode is returned, but the client is expecting the mtime in the updated inode in the OSD.

Is there an easy way to invalidate the the inodes in a client to at least verify the problem?

I'm still not sure what a graceful solution to the problem is... :(