You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@mesos.apache.org by Till Toenshoff <to...@me.com> on 2014/03/09 20:00:19 UTC

Review Request 18946: Moved JNI code to separate library

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/
-----------------------------------------------------------

Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Vinod Kone.


Bugs: MESOS-855
    https://issues.apache.org/jira/browse/MESOS-855


Repository: mesos-git


Description
-------

Introduced a new environment variable (MESOS_NATIVE_JAVA_LIBRARY). That 
variable points towards libmesos_java. libmesos_java contains the JNI-
specific code (formally part of libmesos) and dynamically links against 
libmesos.

A typical java-based framework relies on mesos.jar to do the loading 
but may use some extra logic in its startup to make sure 
MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
to be adapted to use the new environment variable instead of the old 
one.


Diffs
-----

  bin/mesos-slave-flags.sh.in dc73aef 
  src/Makefile.am 61d832b 
  src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 

Diff: https://reviews.apache.org/r/18946/diff/


Testing
-------

make check and functional testing with external, java based frameworks


Thanks,

Till Toenshoff

Re: Review Request 18946: Moved JNI code to separate library

Posted by Till Toenshoff <to...@me.com>.


> On March 9, 2014, 9:34 p.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [18946]
> > 
> > Failed command: make -j3 check GTEST_FILTER='' >/dev/null
> > 
> > Error:
> >  ev.c:1531:31: warning: 'ev_default_loop_ptr' initialized and declared 'extern' [enabled by default]
> > ev.c: In function 'evpipe_write':
> > ev.c:2160:17: warning: ignoring return value of 'write', declared with attribute warn_unused_result [-Wunused-result]
> > ev.c:2172:17: warning: ignoring return value of 'write', declared with attribute warn_unused_result [-Wunused-result]
> > ev.c: In function 'pipecb':
> > ev.c:2193:16: warning: ignoring return value of 'read', declared with attribute warn_unused_result [-Wunused-result]
> > ev.c:2207:16: warning: ignoring return value of 'read', declared with attribute warn_unused_result [-Wunused-result]
> > In file included from /usr/include/c++/4.6/ext/hash_set:61:0,
> >                  from src/glog/stl_logging.h:54,
> >                  from src/stl_logging_unittest.cc:34:
> > /usr/include/c++/4.6/backward/backward_warning.h:33:2: warning: #warning This file includes at least one deprecated or antiquated header which may be removed without further notice at a future date. Please use a non-deprecated interface with equivalent functionality instead. For a listing of replacement headers and interfaces, consult the file backward_warning.h. To disable this warning use -Wno-deprecated. [-Wcpp]
> > In file included from src/utilities.h:73:0,
> >                  from src/googletest.h:38,
> >                  from src/stl_logging_unittest.cc:48:
> > src/base/mutex.h:137:0: warning: "_XOPEN_SOURCE" redefined [enabled by default]
> > /usr/include/features.h:166:0: note: this is the location of the previous definition
> > warning: no files found matching 'Makefile' under directory 'docs'
> > warning: no files found matching 'indexsidebar.html' under directory 'docs'
> > zip_safe flag not set; analyzing archive contents...
> > /usr/bin/ld: cannot find -lmesos
> > collect2: ld returned 1 exit status
> > make[2]: *** [libmesos_java.la] Error 1
> > make[2]: *** Waiting for unfinished jobs....
> > make[1]: *** [check] Error 2
> > make: *** [check-recursive] Error 1
> >

Whoops, failed to see that my initial version only worked because I had libmesos installed previously. Fix upcoming...


- Till


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/#review36617
-----------------------------------------------------------


On March 9, 2014, 7 p.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18946/
> -----------------------------------------------------------
> 
> (Updated March 9, 2014, 7 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-855
>     https://issues.apache.org/jira/browse/MESOS-855
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduced a new environment variable (MESOS_NATIVE_JAVA_LIBRARY). That 
> variable points towards libmesos_java. libmesos_java contains the JNI-
> specific code (formally part of libmesos) and dynamically links against 
> libmesos.
> 
> A typical java-based framework relies on mesos.jar to do the loading 
> but may use some extra logic in its startup to make sure 
> MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
> to be adapted to use the new environment variable instead of the old 
> one.
> 
> 
> Diffs
> -----
> 
>   bin/mesos-slave-flags.sh.in dc73aef 
>   src/Makefile.am 61d832b 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
> 
> Diff: https://reviews.apache.org/r/18946/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing with external, java based frameworks
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 18946: Moved JNI code to separate library

Posted by Mesos ReviewBot <de...@mesos.apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/#review36617
-----------------------------------------------------------


Bad patch!

Reviews applied: [18946]

Failed command: make -j3 check GTEST_FILTER='' >/dev/null

Error:
 ev.c:1531:31: warning: 'ev_default_loop_ptr' initialized and declared 'extern' [enabled by default]
ev.c: In function 'evpipe_write':
ev.c:2160:17: warning: ignoring return value of 'write', declared with attribute warn_unused_result [-Wunused-result]
ev.c:2172:17: warning: ignoring return value of 'write', declared with attribute warn_unused_result [-Wunused-result]
ev.c: In function 'pipecb':
ev.c:2193:16: warning: ignoring return value of 'read', declared with attribute warn_unused_result [-Wunused-result]
ev.c:2207:16: warning: ignoring return value of 'read', declared with attribute warn_unused_result [-Wunused-result]
In file included from /usr/include/c++/4.6/ext/hash_set:61:0,
                 from src/glog/stl_logging.h:54,
                 from src/stl_logging_unittest.cc:34:
/usr/include/c++/4.6/backward/backward_warning.h:33:2: warning: #warning This file includes at least one deprecated or antiquated header which may be removed without further notice at a future date. Please use a non-deprecated interface with equivalent functionality instead. For a listing of replacement headers and interfaces, consult the file backward_warning.h. To disable this warning use -Wno-deprecated. [-Wcpp]
In file included from src/utilities.h:73:0,
                 from src/googletest.h:38,
                 from src/stl_logging_unittest.cc:48:
src/base/mutex.h:137:0: warning: "_XOPEN_SOURCE" redefined [enabled by default]
/usr/include/features.h:166:0: note: this is the location of the previous definition
warning: no files found matching 'Makefile' under directory 'docs'
warning: no files found matching 'indexsidebar.html' under directory 'docs'
zip_safe flag not set; analyzing archive contents...
/usr/bin/ld: cannot find -lmesos
collect2: ld returned 1 exit status
make[2]: *** [libmesos_java.la] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [check] Error 2
make: *** [check-recursive] Error 1


- Mesos ReviewBot


On March 9, 2014, 7 p.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18946/
> -----------------------------------------------------------
> 
> (Updated March 9, 2014, 7 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-855
>     https://issues.apache.org/jira/browse/MESOS-855
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduced a new environment variable (MESOS_NATIVE_JAVA_LIBRARY). That 
> variable points towards libmesos_java. libmesos_java contains the JNI-
> specific code (formally part of libmesos) and dynamically links against 
> libmesos.
> 
> A typical java-based framework relies on mesos.jar to do the loading 
> but may use some extra logic in its startup to make sure 
> MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
> to be adapted to use the new environment variable instead of the old 
> one.
> 
> 
> Diffs
> -----
> 
>   bin/mesos-slave-flags.sh.in dc73aef 
>   src/Makefile.am 61d832b 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
> 
> Diff: https://reviews.apache.org/r/18946/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing with external, java based frameworks
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 18946: Moved JNI code to separate library

Posted by Niklas Nielsen <ni...@qni.dk>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/#review36668
-----------------------------------------------------------


As you mentioned in the JIRA; this will have consequences for Java frameworks. Would it make sense to introduce two new environment variables and maintain the old one for backward compatibility? 


src/Makefile.am
<https://reviews.apache.org/r/18946/#comment67711>

    Fix spaces/tabs :)


- Niklas Nielsen


On March 9, 2014, 8:27 p.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18946/
> -----------------------------------------------------------
> 
> (Updated March 9, 2014, 8:27 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-855
>     https://issues.apache.org/jira/browse/MESOS-855
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduced a new environment variable (MESOS_NATIVE_JAVA_LIBRARY). That 
> variable points towards libmesos_java. libmesos_java contains the JNI-
> specific code (formally part of libmesos) and dynamically links against 
> libmesos.
> 
> A typical java-based framework relies on mesos.jar to do the loading 
> but may use some extra logic in its startup to make sure 
> MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
> to be adapted to use the new environment variable instead of the old 
> one.
> 
> 
> Diffs
> -----
> 
>   bin/mesos-slave-flags.sh.in dc73aef 
>   src/Makefile.am 384b312 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
> 
> Diff: https://reviews.apache.org/r/18946/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing with external, java based frameworks
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 18946: Moved JNI code to separate library

Posted by Mesos ReviewBot <de...@mesos.apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/#review36626
-----------------------------------------------------------


Patch looks great!

Reviews applied: [18946]

All tests passed.

- Mesos ReviewBot


On March 10, 2014, 3:27 a.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18946/
> -----------------------------------------------------------
> 
> (Updated March 10, 2014, 3:27 a.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-855
>     https://issues.apache.org/jira/browse/MESOS-855
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduced a new environment variable (MESOS_NATIVE_JAVA_LIBRARY). That 
> variable points towards libmesos_java. libmesos_java contains the JNI-
> specific code (formally part of libmesos) and dynamically links against 
> libmesos.
> 
> A typical java-based framework relies on mesos.jar to do the loading 
> but may use some extra logic in its startup to make sure 
> MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
> to be adapted to use the new environment variable instead of the old 
> one.
> 
> 
> Diffs
> -----
> 
>   bin/mesos-slave-flags.sh.in dc73aef 
>   src/Makefile.am 384b312 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
> 
> Diff: https://reviews.apache.org/r/18946/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing with external, java based frameworks
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 18946: Moved JNI code to separate library

Posted by Mesos ReviewBot <de...@mesos.apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/#review36779
-----------------------------------------------------------


Patch looks great!

Reviews applied: [18946]

All tests passed.

- Mesos ReviewBot


On March 11, 2014, 12:48 p.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18946/
> -----------------------------------------------------------
> 
> (Updated March 11, 2014, 12:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, Timothy St. Clair, and Vinod Kone.
> 
> 
> Bugs: MESOS-855
>     https://issues.apache.org/jira/browse/MESOS-855
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduced a new environment variable, MESOS_NATIVE_JAVA_LIBRARY. That 
> variable points towards libmesos_java. libmesos_java contains the JNI-
> specific code (formally part of libmesos) and dynamically links against 
> libmesos.
> 
> A typical java-based framework relies on mesos.jar to do the loading 
> but may use some extra logic in its startup to make sure 
> MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
> to be adapted to use the new environment variable instead of the old 
> one.
> 
> Note: 
> This patch does not remove the MESOS_NATIVE_LIRBRAY variable. It is 
> kept for frameworks that are not JVM based but need to bind to the 
> self-contained Mesos library. MESOS_NATIVE_LIRBRAY points towards 
> libmesos.
> MESOS_NATIVE_JAVA_LIBRARY however is the new variant to be used for 
> JVM based frameworks.
> 
> 
> Diffs
> -----
> 
>   bin/mesos-slave-flags.sh.in dc73aef 
>   src/Makefile.am 384b312 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
>   src/slave/containerizer/containerizer.cpp d0a1023 
>   src/tests/environment.cpp 5c92fcd 
> 
> Diff: https://reviews.apache.org/r/18946/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing with external, java based frameworks
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 18946: Moved JNI code to separate library

Posted by Niklas Nielsen <ni...@qni.dk>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/#review36782
-----------------------------------------------------------

Ship it!


Ship It!

- Niklas Nielsen


On March 11, 2014, 5:48 a.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18946/
> -----------------------------------------------------------
> 
> (Updated March 11, 2014, 5:48 a.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, Timothy St. Clair, and Vinod Kone.
> 
> 
> Bugs: MESOS-855
>     https://issues.apache.org/jira/browse/MESOS-855
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduced a new environment variable, MESOS_NATIVE_JAVA_LIBRARY. That 
> variable points towards libmesos_java. libmesos_java contains the JNI-
> specific code (formally part of libmesos) and dynamically links against 
> libmesos.
> 
> A typical java-based framework relies on mesos.jar to do the loading 
> but may use some extra logic in its startup to make sure 
> MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
> to be adapted to use the new environment variable instead of the old 
> one.
> 
> Note: 
> This patch does not remove the MESOS_NATIVE_LIRBRAY variable. It is 
> kept for frameworks that are not JVM based but need to bind to the 
> self-contained Mesos library. MESOS_NATIVE_LIRBRAY points towards 
> libmesos.
> MESOS_NATIVE_JAVA_LIBRARY however is the new variant to be used for 
> JVM based frameworks.
> 
> 
> Diffs
> -----
> 
>   bin/mesos-slave-flags.sh.in dc73aef 
>   src/Makefile.am 384b312 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
>   src/slave/containerizer/containerizer.cpp d0a1023 
>   src/tests/environment.cpp 5c92fcd 
> 
> Diff: https://reviews.apache.org/r/18946/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing with external, java based frameworks
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 18946: Moved JNI code to separate library

Posted by Till Toenshoff <to...@me.com>.


> On March 11, 2014, 4:50 p.m., Vinod Kone wrote:
> > src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in, line 69
> > <https://reviews.apache.org/r/18946/diff/3/?file=516196#file516196line69>
> >
> >     Log a warning here saying this is deprecate d in favor of MESOS_NATIVE_JAVA_LIBRARY?

Great idea, added that.


- Till


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/#review36800
-----------------------------------------------------------


On March 12, 2014, 12:56 a.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18946/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 12:56 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-855
>     https://issues.apache.org/jira/browse/MESOS-855
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduced a new environment variable, MESOS_NATIVE_JAVA_LIBRARY. That 
> variable points towards libmesos_java. libmesos_java contains the JNI-
> specific code (formally part of libmesos) and dynamically links against 
> libmesos.
> 
> A typical java-based framework relies on mesos.jar to do the loading 
> but may use some extra logic in its startup to make sure 
> MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
> to be adapted to use the new environment variable instead of the old 
> one.
> 
> Note: 
> This patch does not remove the MESOS_NATIVE_LIRBRAY variable. It is 
> kept for frameworks that are not JVM based but need to bind to the 
> self-contained Mesos library. MESOS_NATIVE_LIRBRAY points towards 
> libmesos.
> MESOS_NATIVE_JAVA_LIBRARY however is the new variant to be used for 
> JVM based frameworks.
> 
> 
> Diffs
> -----
> 
>   bin/mesos-slave-flags.sh.in dc73aef 
>   src/Makefile.am 384b312 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
>   src/slave/containerizer/containerizer.cpp d0a1023 
>   src/tests/environment.cpp 5c92fcd 
> 
> Diff: https://reviews.apache.org/r/18946/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing with external, java based frameworks
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 18946: Moved JNI code to separate library

Posted by Vinod Kone <vi...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/#review36800
-----------------------------------------------------------

Ship it!



src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in
<https://reviews.apache.org/r/18946/#comment67991>

    Log a warning here saying this is deprecate d in favor of MESOS_NATIVE_JAVA_LIBRARY?


- Vinod Kone


On March 11, 2014, 12:48 p.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18946/
> -----------------------------------------------------------
> 
> (Updated March 11, 2014, 12:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, Timothy St. Clair, and Vinod Kone.
> 
> 
> Bugs: MESOS-855
>     https://issues.apache.org/jira/browse/MESOS-855
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduced a new environment variable, MESOS_NATIVE_JAVA_LIBRARY. That 
> variable points towards libmesos_java. libmesos_java contains the JNI-
> specific code (formally part of libmesos) and dynamically links against 
> libmesos.
> 
> A typical java-based framework relies on mesos.jar to do the loading 
> but may use some extra logic in its startup to make sure 
> MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
> to be adapted to use the new environment variable instead of the old 
> one.
> 
> Note: 
> This patch does not remove the MESOS_NATIVE_LIRBRAY variable. It is 
> kept for frameworks that are not JVM based but need to bind to the 
> self-contained Mesos library. MESOS_NATIVE_LIRBRAY points towards 
> libmesos.
> MESOS_NATIVE_JAVA_LIBRARY however is the new variant to be used for 
> JVM based frameworks.
> 
> 
> Diffs
> -----
> 
>   bin/mesos-slave-flags.sh.in dc73aef 
>   src/Makefile.am 384b312 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
>   src/slave/containerizer/containerizer.cpp d0a1023 
>   src/tests/environment.cpp 5c92fcd 
> 
> Diff: https://reviews.apache.org/r/18946/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing with external, java based frameworks
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 18946: Moved JNI code to separate library

Posted by Till Toenshoff <to...@me.com>.


> On March 12, 2014, 2:50 a.m., Vinod Kone wrote:
> > src/Makefile.am, lines 389-390
> > <https://reviews.apache.org/r/18946/diff/4/?file=516864#file516864line389>
> >
> >     Ben Mahler just reminded me that this change is going to necessitate a deploy order dependency.
> >     
> >     In particular, if a JVM scheduler is using an old jar and new libmesos it won't have access to the JNI code correct?
> >     
> >     We should figure out a way to not impose this constraint if possible. How about not stripping JNI code from libmesos in this release cycle but strip it in a later release after deprecation.

That is correct, it wont have access. Doing a smooth transition with an initial phase of deprecation appears to be a good approach. How about this then, we leave this RR as is but keep it unsubmitted for now. I will propose another variant that does only deprecate the environment variable but does not split the libs?


- Till


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/#review36897
-----------------------------------------------------------


On March 12, 2014, 12:56 a.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18946/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 12:56 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-855
>     https://issues.apache.org/jira/browse/MESOS-855
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduced a new environment variable, MESOS_NATIVE_JAVA_LIBRARY. That 
> variable points towards libmesos_java. libmesos_java contains the JNI-
> specific code (formally part of libmesos) and dynamically links against 
> libmesos.
> 
> A typical java-based framework relies on mesos.jar to do the loading 
> but may use some extra logic in its startup to make sure 
> MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
> to be adapted to use the new environment variable instead of the old 
> one.
> 
> Note: 
> This patch does not remove the MESOS_NATIVE_LIRBRAY variable. It is 
> kept for frameworks that are not JVM based but need to bind to the 
> self-contained Mesos library. MESOS_NATIVE_LIRBRAY points towards 
> libmesos.
> MESOS_NATIVE_JAVA_LIBRARY however is the new variant to be used for 
> JVM based frameworks.
> 
> 
> Diffs
> -----
> 
>   bin/mesos-slave-flags.sh.in dc73aef 
>   src/Makefile.am 384b312 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
>   src/slave/containerizer/containerizer.cpp d0a1023 
>   src/tests/environment.cpp 5c92fcd 
> 
> Diff: https://reviews.apache.org/r/18946/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing with external, java based frameworks
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 18946: Moved JNI code to separate library

Posted by Vinod Kone <vi...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/#review36897
-----------------------------------------------------------



src/Makefile.am
<https://reviews.apache.org/r/18946/#comment68091>

    Ben Mahler just reminded me that this change is going to necessitate a deploy order dependency.
    
    In particular, if a JVM scheduler is using an old jar and new libmesos it won't have access to the JNI code correct?
    
    We should figure out a way to not impose this constraint if possible. How about not stripping JNI code from libmesos in this release cycle but strip it in a later release after deprecation.


- Vinod Kone


On March 12, 2014, 12:56 a.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18946/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 12:56 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-855
>     https://issues.apache.org/jira/browse/MESOS-855
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduced a new environment variable, MESOS_NATIVE_JAVA_LIBRARY. That 
> variable points towards libmesos_java. libmesos_java contains the JNI-
> specific code (formally part of libmesos) and dynamically links against 
> libmesos.
> 
> A typical java-based framework relies on mesos.jar to do the loading 
> but may use some extra logic in its startup to make sure 
> MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
> to be adapted to use the new environment variable instead of the old 
> one.
> 
> Note: 
> This patch does not remove the MESOS_NATIVE_LIRBRAY variable. It is 
> kept for frameworks that are not JVM based but need to bind to the 
> self-contained Mesos library. MESOS_NATIVE_LIRBRAY points towards 
> libmesos.
> MESOS_NATIVE_JAVA_LIBRARY however is the new variant to be used for 
> JVM based frameworks.
> 
> 
> Diffs
> -----
> 
>   bin/mesos-slave-flags.sh.in dc73aef 
>   src/Makefile.am 384b312 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
>   src/slave/containerizer/containerizer.cpp d0a1023 
>   src/tests/environment.cpp 5c92fcd 
> 
> Diff: https://reviews.apache.org/r/18946/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing with external, java based frameworks
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 18946: Moved JNI code to separate library

Posted by Mesos ReviewBot <de...@mesos.apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/#review36894
-----------------------------------------------------------


Patch looks great!

Reviews applied: [18946]

All tests passed.

- Mesos ReviewBot


On March 12, 2014, 12:56 a.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18946/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 12:56 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-855
>     https://issues.apache.org/jira/browse/MESOS-855
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduced a new environment variable, MESOS_NATIVE_JAVA_LIBRARY. That 
> variable points towards libmesos_java. libmesos_java contains the JNI-
> specific code (formally part of libmesos) and dynamically links against 
> libmesos.
> 
> A typical java-based framework relies on mesos.jar to do the loading 
> but may use some extra logic in its startup to make sure 
> MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
> to be adapted to use the new environment variable instead of the old 
> one.
> 
> Note: 
> This patch does not remove the MESOS_NATIVE_LIRBRAY variable. It is 
> kept for frameworks that are not JVM based but need to bind to the 
> self-contained Mesos library. MESOS_NATIVE_LIRBRAY points towards 
> libmesos.
> MESOS_NATIVE_JAVA_LIBRARY however is the new variant to be used for 
> JVM based frameworks.
> 
> 
> Diffs
> -----
> 
>   bin/mesos-slave-flags.sh.in dc73aef 
>   src/Makefile.am 384b312 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
>   src/slave/containerizer/containerizer.cpp d0a1023 
>   src/tests/environment.cpp 5c92fcd 
> 
> Diff: https://reviews.apache.org/r/18946/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing with external, java based frameworks
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 18946: Moved JNI code to separate library

Posted by Till Toenshoff <to...@me.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/
-----------------------------------------------------------

(Updated March 12, 2014, 12:56 a.m.)


Review request for mesos and Vinod Kone.


Changes
-------

Added deprecation warning.


Bugs: MESOS-855
    https://issues.apache.org/jira/browse/MESOS-855


Repository: mesos-git


Description
-------

Introduced a new environment variable, MESOS_NATIVE_JAVA_LIBRARY. That 
variable points towards libmesos_java. libmesos_java contains the JNI-
specific code (formally part of libmesos) and dynamically links against 
libmesos.

A typical java-based framework relies on mesos.jar to do the loading 
but may use some extra logic in its startup to make sure 
MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
to be adapted to use the new environment variable instead of the old 
one.

Note: 
This patch does not remove the MESOS_NATIVE_LIRBRAY variable. It is 
kept for frameworks that are not JVM based but need to bind to the 
self-contained Mesos library. MESOS_NATIVE_LIRBRAY points towards 
libmesos.
MESOS_NATIVE_JAVA_LIBRARY however is the new variant to be used for 
JVM based frameworks.


Diffs (updated)
-----

  bin/mesos-slave-flags.sh.in dc73aef 
  src/Makefile.am 384b312 
  src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
  src/slave/containerizer/containerizer.cpp d0a1023 
  src/tests/environment.cpp 5c92fcd 

Diff: https://reviews.apache.org/r/18946/diff/


Testing
-------

make check and functional testing with external, java based frameworks


Thanks,

Till Toenshoff

Re: Review Request 18946: Moved JNI code to separate library

Posted by Till Toenshoff <to...@me.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/
-----------------------------------------------------------

(Updated March 11, 2014, 4:59 p.m.)


Review request for mesos and Vinod Kone.


Changes
-------

assigning this to myself for shepherding -- Vinod


Bugs: MESOS-855
    https://issues.apache.org/jira/browse/MESOS-855


Repository: mesos-git


Description
-------

Introduced a new environment variable, MESOS_NATIVE_JAVA_LIBRARY. That 
variable points towards libmesos_java. libmesos_java contains the JNI-
specific code (formally part of libmesos) and dynamically links against 
libmesos.

A typical java-based framework relies on mesos.jar to do the loading 
but may use some extra logic in its startup to make sure 
MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
to be adapted to use the new environment variable instead of the old 
one.

Note: 
This patch does not remove the MESOS_NATIVE_LIRBRAY variable. It is 
kept for frameworks that are not JVM based but need to bind to the 
self-contained Mesos library. MESOS_NATIVE_LIRBRAY points towards 
libmesos.
MESOS_NATIVE_JAVA_LIBRARY however is the new variant to be used for 
JVM based frameworks.


Diffs
-----

  bin/mesos-slave-flags.sh.in dc73aef 
  src/Makefile.am 384b312 
  src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
  src/slave/containerizer/containerizer.cpp d0a1023 
  src/tests/environment.cpp 5c92fcd 

Diff: https://reviews.apache.org/r/18946/diff/


Testing
-------

make check and functional testing with external, java based frameworks


Thanks,

Till Toenshoff

Re: Review Request 18946: Moved JNI code to separate library

Posted by "Timothy St. Clair" <ts...@redhat.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/#review36785
-----------------------------------------------------------

Ship it!


Ship It!

- Timothy St. Clair


On March 11, 2014, 12:48 p.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18946/
> -----------------------------------------------------------
> 
> (Updated March 11, 2014, 12:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, Timothy St. Clair, and Vinod Kone.
> 
> 
> Bugs: MESOS-855
>     https://issues.apache.org/jira/browse/MESOS-855
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduced a new environment variable, MESOS_NATIVE_JAVA_LIBRARY. That 
> variable points towards libmesos_java. libmesos_java contains the JNI-
> specific code (formally part of libmesos) and dynamically links against 
> libmesos.
> 
> A typical java-based framework relies on mesos.jar to do the loading 
> but may use some extra logic in its startup to make sure 
> MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
> to be adapted to use the new environment variable instead of the old 
> one.
> 
> Note: 
> This patch does not remove the MESOS_NATIVE_LIRBRAY variable. It is 
> kept for frameworks that are not JVM based but need to bind to the 
> self-contained Mesos library. MESOS_NATIVE_LIRBRAY points towards 
> libmesos.
> MESOS_NATIVE_JAVA_LIBRARY however is the new variant to be used for 
> JVM based frameworks.
> 
> 
> Diffs
> -----
> 
>   bin/mesos-slave-flags.sh.in dc73aef 
>   src/Makefile.am 384b312 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
>   src/slave/containerizer/containerizer.cpp d0a1023 
>   src/tests/environment.cpp 5c92fcd 
> 
> Diff: https://reviews.apache.org/r/18946/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing with external, java based frameworks
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 18946: Moved JNI code to separate library

Posted by Till Toenshoff <to...@me.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/
-----------------------------------------------------------

(Updated March 11, 2014, 12:48 p.m.)

Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, Timothy St. Clair, and Vinod Kone.

Bugs: MESOS-855
https://issues.apache.org/jira/browse/MESOS-855

Repository: mesos-git

Description (updated)
-------

Introduced a new environment variable, MESOS_NATIVE_JAVA_LIBRARY. That
variable points towards libmesos_java. libmesos_java contains the JNI-
specific code (formally part of libmesos) and dynamically links against
libmesos.

A typical java-based framework relies on mesos.jar to do the loading
but may use some extra logic in its startup to make sure
MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need
to be adapted to use the new environment variable instead of the old
one.

Note:
This patch does not remove the MESOS_NATIVE_LIRBRAY variable. It is
kept for frameworks that are not JVM based but need to bind to the
self-contained Mesos library. MESOS_NATIVE_LIRBRAY points towards
libmesos.
MESOS_NATIVE_JAVA_LIBRARY however is the new variant to be used for
JVM based frameworks.

Diffs
-----

bin/mesos-slave-flags.sh.in dc73aef
src/Makefile.am 384b312
src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2
src/slave/containerizer/containerizer.cpp d0a1023
src/tests/environment.cpp 5c92fcd

Diff: https://reviews.apache.org/r/18946/diff/

Testing
-------

make check and functional testing with external, java based frameworks

Thanks,

Till Toenshoff

Re: Review Request 18946: Moved JNI code to separate library

Posted by Till Toenshoff <to...@me.com>.

(Updated March 11, 2014, 12:42 p.m.)

Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, Timothy St. Clair, and Vinod Kone.

Changes
-------

Enhanced description.

Bugs: MESOS-855
https://issues.apache.org/jira/browse/MESOS-855

Repository: mesos-git

Description (updated)
-------

Note:
This patch does not remove the MESOS_NATIVE_LIRBRAY variable. It is
kept for frameworks that are not JVM based but need to bind to the
self-contained Mesos library and. MESOS_NATIVE_LIRBRAY points towards
libmesos.
MESOS_NATIVE_JAVA_LIBRARY however is the new variant to be used for
JVM based frameworks.

Diffs
-----

Diff: https://reviews.apache.org/r/18946/diff/

Testing
-------

make check and functional testing with external, java based frameworks

Thanks,

Till Toenshoff

Re: Review Request 18946: Moved JNI code to separate library

Posted by Till Toenshoff <to...@me.com>.

(Updated March 11, 2014, 12:32 p.m.)

Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, Timothy St. Clair, and Vinod Kone.

Changes
-------

Addressed comments:
- JNI library now is versioned and marked as shared while building
- fixed spaces

Added omitted patches to:
- containerizer
- tests-environment

Bugs: MESOS-855
https://issues.apache.org/jira/browse/MESOS-855

Repository: mesos-git

Description
-------

Introduced a new environment variable (MESOS_NATIVE_JAVA_LIBRARY). That
variable points towards libmesos_java. libmesos_java contains the JNI-
specific code (formally part of libmesos) and dynamically links against
libmesos.

Diffs (updated)
-----

Diff: https://reviews.apache.org/r/18946/diff/

Testing
-------

make check and functional testing with external, java based frameworks

Thanks,

Till Toenshoff

Re: Review Request 18946: Moved JNI code to separate library

Posted by Till Toenshoff <to...@me.com>.


> On March 10, 2014, 9:06 p.m., Timothy St. Clair wrote:
> > So I looked over the patch, and it appears there will be a new install target but it doesn't appear to be shared or versioned?  
> > 
> > e.g. (no) -version-info 0:0:0 -release $(PACKAGE_VERSION) -shared 
> > 
> > Is there a reason?

Clear omission, thanks for pointing it out - fixed.


- Till


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/#review36705
-----------------------------------------------------------


On March 11, 2014, 12:32 p.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18946/
> -----------------------------------------------------------
> 
> (Updated March 11, 2014, 12:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, Timothy St. Clair, and Vinod Kone.
> 
> 
> Bugs: MESOS-855
>     https://issues.apache.org/jira/browse/MESOS-855
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduced a new environment variable (MESOS_NATIVE_JAVA_LIBRARY). That 
> variable points towards libmesos_java. libmesos_java contains the JNI-
> specific code (formally part of libmesos) and dynamically links against 
> libmesos.
> 
> A typical java-based framework relies on mesos.jar to do the loading 
> but may use some extra logic in its startup to make sure 
> MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
> to be adapted to use the new environment variable instead of the old 
> one.
> 
> 
> Diffs
> -----
> 
>   bin/mesos-slave-flags.sh.in dc73aef 
>   src/Makefile.am 384b312 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
>   src/slave/containerizer/containerizer.cpp d0a1023 
>   src/tests/environment.cpp 5c92fcd 
> 
> Diff: https://reviews.apache.org/r/18946/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing with external, java based frameworks
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 18946: Moved JNI code to separate library

Posted by "Timothy St. Clair" <ts...@redhat.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/#review36705
-----------------------------------------------------------


So I looked over the patch, and it appears there will be a new install target but it doesn't appear to be shared or versioned?  

e.g. (no) -version-info 0:0:0 -release $(PACKAGE_VERSION) -shared 

Is there a reason?

- Timothy St. Clair


On March 10, 2014, 3:27 a.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18946/
> -----------------------------------------------------------
> 
> (Updated March 10, 2014, 3:27 a.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-855
>     https://issues.apache.org/jira/browse/MESOS-855
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduced a new environment variable (MESOS_NATIVE_JAVA_LIBRARY). That 
> variable points towards libmesos_java. libmesos_java contains the JNI-
> specific code (formally part of libmesos) and dynamically links against 
> libmesos.
> 
> A typical java-based framework relies on mesos.jar to do the loading 
> but may use some extra logic in its startup to make sure 
> MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
> to be adapted to use the new environment variable instead of the old 
> one.
> 
> 
> Diffs
> -----
> 
>   bin/mesos-slave-flags.sh.in dc73aef 
>   src/Makefile.am 384b312 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 
> 
> Diff: https://reviews.apache.org/r/18946/diff/
> 
> 
> Testing
> -------
> 
> make check and functional testing with external, java based frameworks
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 18946: Moved JNI code to separate library

Posted by Till Toenshoff <to...@me.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18946/
-----------------------------------------------------------

(Updated March 10, 2014, 3:27 a.m.)


Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Vinod Kone.


Changes
-------

Fixed build error when libmesos is not previously installed.


Bugs: MESOS-855
    https://issues.apache.org/jira/browse/MESOS-855


Repository: mesos-git


Description
-------

Introduced a new environment variable (MESOS_NATIVE_JAVA_LIBRARY). That 
variable points towards libmesos_java. libmesos_java contains the JNI-
specific code (formally part of libmesos) and dynamically links against 
libmesos.

A typical java-based framework relies on mesos.jar to do the loading 
but may use some extra logic in its startup to make sure 
MESOS_NATIVE[_JAVA]_LIBRARY is set/valid. That extra-logic would need 
to be adapted to use the new environment variable instead of the old 
one.


Diffs (updated)
-----

  bin/mesos-slave-flags.sh.in dc73aef 
  src/Makefile.am 384b312 
  src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 231d1e2 

Diff: https://reviews.apache.org/r/18946/diff/


Testing
-------

make check and functional testing with external, java based frameworks


Thanks,

Till Toenshoff