You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2014/08/14 03:19:13 UTC

Review Request 24687: Added support for disabling glog initialization.

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

Review request for mesos, Benjamin Hindman and Ben Mahler.


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


Repository: mesos-git


Description
-------

C++ frameworks can now explicitly disable glog initialization if they choose to do so.

By default now the executor driver also enables glog initialization. It didn't use to before, but I think that's a bug.


Diffs
-----

  include/mesos/executor.hpp ed2330f1677b9aa56ef43bae5dec0b3acfca0e1c 
  include/mesos/scheduler.hpp 802727270ae8afb2d772bdb0170a2378dd611ee4 
  src/exec/exec.cpp 15d41eb303c81a1ae958adc76a105c11d7ef72ef 
  src/java/src/org/apache/mesos/MesosExecutorDriver.java 910548c0b5137294f67b8a21b8c77fc2d8dbd5e3 
  src/java/src/org/apache/mesos/MesosSchedulerDriver.java afdbbbc0e6deddcf620517b7ecc4ab7947ae91b6 
  src/logging/flags.hpp d30a7069c07af5b98a7f26e4158e839cbf424506 
  src/sched/sched.cpp cbc52916c551b324aab7c5ddb51b2f7679cae88b 
  src/scheduler/scheduler.cpp 498d6aa421c96768d4be0ccff38d148e992949fc 

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


Testing
-------

make check

# ./src/long-lived-framework 127.0.0.1:5050
I0814 01:13:32.889717 56878 sched.cpp:139] Version: 0.20.0
I0814 01:13:32.896556 56975 sched.cpp:235] New master detected at master@127.0.0.1:5050
I0814 01:13:32.896836 56975 sched.cpp:243] No credentials provided. Attempting to register without authentication

# MESOS_LOGGING_INITIALIZE=0 ./src/long-lived-framework 127.0.0.1:5050
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0814 01:14:18.517209 60286 sched.cpp:139] Version: 0.20.0
I0814 01:14:18.524113 60322 sched.cpp:235] New master detected at master@127.0.0.1:5050
I0814 01:14:18.524395 60322 sched.cpp:243] No credentials provided. Attempting to register without authentication


# MESOS_LOGGING_INITIALIZE=1 ./src/long-lived-framework 127.0.0.1:5050
I0814 01:14:25.621443 60969 sched.cpp:139] Version: 0.20.0
I0814 01:14:25.628525 60986 sched.cpp:235] New master detected at master@127.0.0.1:5050
I0814 01:14:25.628746 60986 sched.cpp:243] No credentials provided. Attempting to register without authentication


Thanks,

Vinod Kone


Re: Review Request 24687: Added support for disabling glog initialization.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24687/
-----------------------------------------------------------

(Updated Aug. 15, 2014, 5:42 p.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


Changes
-------

benm's comments.


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


Repository: mesos-git


Description
-------

C++ frameworks can now explicitly disable glog initialization if they choose to do so.

By default now the executor driver also enables glog initialization. It didn't use to before, but I think that's a bug.


Diffs (updated)
-----

  include/mesos/executor.hpp ed2330f1677b9aa56ef43bae5dec0b3acfca0e1c 
  include/mesos/scheduler.hpp 802727270ae8afb2d772bdb0170a2378dd611ee4 
  src/exec/exec.cpp 15d41eb303c81a1ae958adc76a105c11d7ef72ef 
  src/java/src/org/apache/mesos/MesosExecutorDriver.java 910548c0b5137294f67b8a21b8c77fc2d8dbd5e3 
  src/java/src/org/apache/mesos/MesosSchedulerDriver.java afdbbbc0e6deddcf620517b7ecc4ab7947ae91b6 
  src/logging/flags.hpp d30a7069c07af5b98a7f26e4158e839cbf424506 
  src/sched/sched.cpp cbc52916c551b324aab7c5ddb51b2f7679cae88b 
  src/scheduler/scheduler.cpp 498d6aa421c96768d4be0ccff38d148e992949fc 

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


Testing (updated)
-------

make check

# ./src/long-lived-framework 127.0.0.1:5050
I0814 01:13:32.889717 56878 sched.cpp:139] Version: 0.20.0
I0814 01:13:32.896556 56975 sched.cpp:235] New master detected at master@127.0.0.1:5050
I0814 01:13:32.896836 56975 sched.cpp:243] No credentials provided. Attempting to register without authentication

# MESOS_LOGGING_INITIALIZE=0 ./src/long-lived-framework 127.0.0.1:5050
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0814 01:14:18.517209 60286 sched.cpp:139] Version: 0.20.0
I0814 01:14:18.524113 60322 sched.cpp:235] New master detected at master@127.0.0.1:5050
I0814 01:14:18.524395 60322 sched.cpp:243] No credentials provided. Attempting to register without authentication


# MESOS_LOGGING_INITIALIZE=1 ./src/long-lived-framework 127.0.0.1:5050
I0814 01:14:25.621443 60969 sched.cpp:139] Version: 0.20.0
I0814 01:14:25.628525 60986 sched.cpp:235] New master detected at master@127.0.0.1:5050
I0814 01:14:25.628746 60986 sched.cpp:243] No credentials provided. Attempting to register without authentication


Thanks,

Vinod Kone


Re: Review Request 24687: Added support for disabling glog initialization.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24687/#review50578
-----------------------------------------------------------


Patch looks great!

Reviews applied: [24687]

All tests passed.

- Mesos ReviewBot


On Aug. 14, 2014, 1:19 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24687/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2014, 1:19 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-1629
>     https://issues.apache.org/jira/browse/MESOS-1629
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> C++ frameworks can now explicitly disable glog initialization if they choose to do so.
> 
> By default now the executor driver also enables glog initialization. It didn't use to before, but I think that's a bug.
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp ed2330f1677b9aa56ef43bae5dec0b3acfca0e1c 
>   include/mesos/scheduler.hpp 802727270ae8afb2d772bdb0170a2378dd611ee4 
>   src/exec/exec.cpp 15d41eb303c81a1ae958adc76a105c11d7ef72ef 
>   src/java/src/org/apache/mesos/MesosExecutorDriver.java 910548c0b5137294f67b8a21b8c77fc2d8dbd5e3 
>   src/java/src/org/apache/mesos/MesosSchedulerDriver.java afdbbbc0e6deddcf620517b7ecc4ab7947ae91b6 
>   src/logging/flags.hpp d30a7069c07af5b98a7f26e4158e839cbf424506 
>   src/sched/sched.cpp cbc52916c551b324aab7c5ddb51b2f7679cae88b 
>   src/scheduler/scheduler.cpp 498d6aa421c96768d4be0ccff38d148e992949fc 
> 
> Diff: https://reviews.apache.org/r/24687/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> # ./src/long-lived-framework 127.0.0.1:5050
> I0814 01:13:32.889717 56878 sched.cpp:139] Version: 0.20.0
> I0814 01:13:32.896556 56975 sched.cpp:235] New master detected at master@127.0.0.1:5050
> I0814 01:13:32.896836 56975 sched.cpp:243] No credentials provided. Attempting to register without authentication
> 
> # MESOS_LOGGING_INITIALIZE=0 ./src/long-lived-framework 127.0.0.1:5050
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0814 01:14:18.517209 60286 sched.cpp:139] Version: 0.20.0
> I0814 01:14:18.524113 60322 sched.cpp:235] New master detected at master@127.0.0.1:5050
> I0814 01:14:18.524395 60322 sched.cpp:243] No credentials provided. Attempting to register without authentication
> 
> 
> # MESOS_LOGGING_INITIALIZE=1 ./src/long-lived-framework 127.0.0.1:5050
> I0814 01:14:25.621443 60969 sched.cpp:139] Version: 0.20.0
> I0814 01:14:25.628525 60986 sched.cpp:235] New master detected at master@127.0.0.1:5050
> I0814 01:14:25.628746 60986 sched.cpp:243] No credentials provided. Attempting to register without authentication
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 24687: Added support for disabling glog initialization.

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

> On Aug. 15, 2014, 1:07 a.m., Ben Mahler wrote:
> > src/logging/flags.hpp, lines 59-62
> > <https://reviews.apache.org/r/24687/diff/1/?file=660055#file660055line59>
> >
> >     Does 'initialize_logging' read more naturally?
> >     
> >     Maybe we should have a NOTE in this flag description that this is only used for the library entry points (executor / scheduler drivers), for other components (master / slave) it is ignored completely.

changed it to "initialize_driver_logging" to be more precise.


> On Aug. 15, 2014, 1:07 a.m., Ben Mahler wrote:
> > include/mesos/executor.hpp, line 234
> > <https://reviews.apache.org/r/24687/diff/1/?file=660050#file660050line234>
> >
> >     Would a better example here and below be GLOG_v (even though it's not a mesos flag)? I think by default we don't print anything, right?

updated the comment.


- Vinod


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


On Aug. 15, 2014, 5:42 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24687/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2014, 5:42 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-1629
>     https://issues.apache.org/jira/browse/MESOS-1629
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> C++ frameworks can now explicitly disable glog initialization if they choose to do so.
> 
> By default now the executor driver also enables glog initialization. It didn't use to before, but I think that's a bug.
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp ed2330f1677b9aa56ef43bae5dec0b3acfca0e1c 
>   include/mesos/scheduler.hpp 802727270ae8afb2d772bdb0170a2378dd611ee4 
>   src/exec/exec.cpp 15d41eb303c81a1ae958adc76a105c11d7ef72ef 
>   src/java/src/org/apache/mesos/MesosExecutorDriver.java 910548c0b5137294f67b8a21b8c77fc2d8dbd5e3 
>   src/java/src/org/apache/mesos/MesosSchedulerDriver.java afdbbbc0e6deddcf620517b7ecc4ab7947ae91b6 
>   src/logging/flags.hpp d30a7069c07af5b98a7f26e4158e839cbf424506 
>   src/sched/sched.cpp cbc52916c551b324aab7c5ddb51b2f7679cae88b 
>   src/scheduler/scheduler.cpp 498d6aa421c96768d4be0ccff38d148e992949fc 
> 
> Diff: https://reviews.apache.org/r/24687/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> # ./src/long-lived-framework 127.0.0.1:5050
> I0814 01:13:32.889717 56878 sched.cpp:139] Version: 0.20.0
> I0814 01:13:32.896556 56975 sched.cpp:235] New master detected at master@127.0.0.1:5050
> I0814 01:13:32.896836 56975 sched.cpp:243] No credentials provided. Attempting to register without authentication
> 
> # MESOS_LOGGING_INITIALIZE=0 ./src/long-lived-framework 127.0.0.1:5050
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0814 01:14:18.517209 60286 sched.cpp:139] Version: 0.20.0
> I0814 01:14:18.524113 60322 sched.cpp:235] New master detected at master@127.0.0.1:5050
> I0814 01:14:18.524395 60322 sched.cpp:243] No credentials provided. Attempting to register without authentication
> 
> 
> # MESOS_LOGGING_INITIALIZE=1 ./src/long-lived-framework 127.0.0.1:5050
> I0814 01:14:25.621443 60969 sched.cpp:139] Version: 0.20.0
> I0814 01:14:25.628525 60986 sched.cpp:235] New master detected at master@127.0.0.1:5050
> I0814 01:14:25.628746 60986 sched.cpp:243] No credentials provided. Attempting to register without authentication
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 24687: Added support for disabling glog initialization.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24687/#review50678
-----------------------------------------------------------

Ship it!



include/mesos/executor.hpp
<https://reviews.apache.org/r/24687/#comment88540>

    Would a better example here and below be GLOG_v (even though it's not a mesos flag)? I think by default we don't print anything, right?



src/exec/exec.cpp
<https://reviews.apache.org/r/24687/#comment88541>

    Curious why you didn't use executor->error() here akin to what's done inside sched.cpp?



src/logging/flags.hpp
<https://reviews.apache.org/r/24687/#comment88545>

    Does 'initialize_logging' read more naturally?
    
    Maybe we should have a NOTE in this flag description that this is only used for the library entry points (executor / scheduler drivers), for other components (master / slave) it is ignored completely.


- Ben Mahler


On Aug. 14, 2014, 1:19 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24687/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2014, 1:19 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-1629
>     https://issues.apache.org/jira/browse/MESOS-1629
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> C++ frameworks can now explicitly disable glog initialization if they choose to do so.
> 
> By default now the executor driver also enables glog initialization. It didn't use to before, but I think that's a bug.
> 
> 
> Diffs
> -----
> 
>   include/mesos/executor.hpp ed2330f1677b9aa56ef43bae5dec0b3acfca0e1c 
>   include/mesos/scheduler.hpp 802727270ae8afb2d772bdb0170a2378dd611ee4 
>   src/exec/exec.cpp 15d41eb303c81a1ae958adc76a105c11d7ef72ef 
>   src/java/src/org/apache/mesos/MesosExecutorDriver.java 910548c0b5137294f67b8a21b8c77fc2d8dbd5e3 
>   src/java/src/org/apache/mesos/MesosSchedulerDriver.java afdbbbc0e6deddcf620517b7ecc4ab7947ae91b6 
>   src/logging/flags.hpp d30a7069c07af5b98a7f26e4158e839cbf424506 
>   src/sched/sched.cpp cbc52916c551b324aab7c5ddb51b2f7679cae88b 
>   src/scheduler/scheduler.cpp 498d6aa421c96768d4be0ccff38d148e992949fc 
> 
> Diff: https://reviews.apache.org/r/24687/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> # ./src/long-lived-framework 127.0.0.1:5050
> I0814 01:13:32.889717 56878 sched.cpp:139] Version: 0.20.0
> I0814 01:13:32.896556 56975 sched.cpp:235] New master detected at master@127.0.0.1:5050
> I0814 01:13:32.896836 56975 sched.cpp:243] No credentials provided. Attempting to register without authentication
> 
> # MESOS_LOGGING_INITIALIZE=0 ./src/long-lived-framework 127.0.0.1:5050
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0814 01:14:18.517209 60286 sched.cpp:139] Version: 0.20.0
> I0814 01:14:18.524113 60322 sched.cpp:235] New master detected at master@127.0.0.1:5050
> I0814 01:14:18.524395 60322 sched.cpp:243] No credentials provided. Attempting to register without authentication
> 
> 
> # MESOS_LOGGING_INITIALIZE=1 ./src/long-lived-framework 127.0.0.1:5050
> I0814 01:14:25.621443 60969 sched.cpp:139] Version: 0.20.0
> I0814 01:14:25.628525 60986 sched.cpp:235] New master detected at master@127.0.0.1:5050
> I0814 01:14:25.628746 60986 sched.cpp:243] No credentials provided. Attempting to register without authentication
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>