date:20150417

Re: Review Request 33257: Fixed recover tasks only by the intiated containerizer.

2015-04-17 Thread Till Toenshoff



> On April 18, 2015, 2:55 a.m., Till Toenshoff wrote:
> >

One more tiny nit, the summary has a typo.


- Till


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


On April 17, 2015, 7:07 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33257/
> ---
> 
> (Updated April 17, 2015, 7:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Ian Downes, Jie 
> Yu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2601
> https://issues.apache.org/jira/browse/MESOS-2601
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed recover tasks only by the intiated containerizer.
> Currently both mesos and docker containerizer recovers tasks that wasn't 
> started by themselves.
> The proposed fix is to record the intended containerizer in the checkpointed 
> executorInfo, and reuse that information on recover to know if the 
> containerizer should recover or not. We are free to modify the executorInfo 
> since it's not being used to relaunch any task.
> The external containerizer doesn't need to change since it is only recovering 
> containers that are returned by the containers script.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 6893684e6d199a5d69fc8bba8e60c4acaae9c3c9 
>   src/slave/containerizer/docker.cpp f9fb07806e3b7d7d2afc1be3b8756eac23b32dcd 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e4136095fca55637864f495098189ab3ad8d8fe7 
>   src/slave/slave.cpp a0595f93ce4720f5b9926326d01210460ccb0667 
>   src/tests/containerizer_tests.cpp 5991aa628083dac7c5e8bf7ba297f4f9edeec05f 
>   src/tests/docker_containerizer_tests.cpp 
> c772d4c836de18b0e87636cb42200356d24ec73d 
> 
> Diff: https://reviews.apache.org/r/33257/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 33257: Fixed recover tasks only by the intiated containerizer.

2015-04-17 Thread Till Toenshoff


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

Ship it!



src/slave/containerizer/docker.cpp


Not sure what would be correct but I guess it would be right to use a 
lower-case "d" for "docker" in all comments. Right now we have both variants in 
this patch.



src/slave/containerizer/docker.cpp


s/checkpoint/checkpoints/



src/slave/containerizer/docker.cpp


s/docker/the docker/



src/slave/containerizer/docker.cpp


Reference to temporary is going to be a nono :) - see  
https://reviews.apache.org/r/33271



src/slave/containerizer/docker.cpp


Why did you move this up?



src/tests/containerizer_tests.cpp


s/Mesos/mesos/



src/tests/docker_containerizer_tests.cpp


s/docker/the docker/


- Till Toenshoff


On April 17, 2015, 7:07 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33257/
> ---
> 
> (Updated April 17, 2015, 7:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Ian Downes, Jie 
> Yu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2601
> https://issues.apache.org/jira/browse/MESOS-2601
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed recover tasks only by the intiated containerizer.
> Currently both mesos and docker containerizer recovers tasks that wasn't 
> started by themselves.
> The proposed fix is to record the intended containerizer in the checkpointed 
> executorInfo, and reuse that information on recover to know if the 
> containerizer should recover or not. We are free to modify the executorInfo 
> since it's not being used to relaunch any task.
> The external containerizer doesn't need to change since it is only recovering 
> containers that are returned by the containers script.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 6893684e6d199a5d69fc8bba8e60c4acaae9c3c9 
>   src/slave/containerizer/docker.cpp f9fb07806e3b7d7d2afc1be3b8756eac23b32dcd 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e4136095fca55637864f495098189ab3ad8d8fe7 
>   src/slave/slave.cpp a0595f93ce4720f5b9926326d01210460ccb0667 
>   src/tests/containerizer_tests.cpp 5991aa628083dac7c5e8bf7ba297f4f9edeec05f 
>   src/tests/docker_containerizer_tests.cpp 
> c772d4c836de18b0e87636cb42200356d24ec73d 
> 
> Diff: https://reviews.apache.org/r/33257/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 33331: Added file headers section to the C++ style guide.

2015-04-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [0, 1]

All tests passed.

- Mesos ReviewBot


On April 18, 2015, 1:34 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1/
> ---
> 
> (Updated April 18, 2015, 1:34 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   docs/mesos-c++-style-guide.md de1b93e 
> 
> Diff: https://reviews.apache.org/r/1/diff/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 33330: Fixed C++ style guide formatting.

2015-04-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [0]

All tests passed.

- Mesos ReviewBot


On April 18, 2015, 1:32 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/0/
> ---
> 
> (Updated April 18, 2015, 1:32 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Enables syntax highlighting in markdown viewers.
> 
> 
> Diffs
> -
> 
>   docs/mesos-c++-style-guide.md de1b93e 
> 
> Diff: https://reviews.apache.org/r/0/diff/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 33331: Added file headers section to the C++ style guide.

2015-04-17 Thread Till Toenshoff


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

(Updated April 18, 2015, 1:34 a.m.)


Review request for mesos.


Repository: mesos-incubating


Description
---

see summary.


Diffs
-

  docs/mesos-c++-style-guide.md de1b93e 

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


Testing
---

N/A


Thanks,

Till Toenshoff

Review Request 33331: Added file headers section to the C++ style guide.

2015-04-17 Thread Till Toenshoff


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

Review request for mesos.


Repository: mesos-incubating


Description
---

see summary.


Diffs
-

  docs/mesos-c++-style-guide.md de1b93e 

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


Testing
---

N/A


Thanks,

Till Toenshoff

Review Request 33330: Fixed C++ style guide formatting.

2015-04-17 Thread Till Toenshoff


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

Review request for mesos.


Repository: mesos-incubating


Description
---

Enables syntax highlighting in markdown viewers.


Diffs
-

  docs/mesos-c++-style-guide.md de1b93e 

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


Testing
---

N/A


Thanks,

Till Toenshoff

Re: Review Request 33329: Removed unnecessary freeaddrinfo in getIP if getaddrinfo returns error.

2015-04-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [33329]

All tests passed.

- Mesos ReviewBot


On April 18, 2015, 12:35 a.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33329/
> ---
> 
> (Updated April 18, 2015, 12:35 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Evelina Dumitrescu, and Jie Yu.
> 
> 
> Bugs: mesos-2636
> https://issues.apache.org/jira/browse/mesos-2636
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unnecessary freeaddrinfo in getIP if getaddrinfo returns error.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
> 3622165af064a1f0a9d461af0e1d0d88a352d4a8 
> 
> Diff: https://reviews.apache.org/r/33329/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Review Request 33329: Removed unnecessary freeaddrinfo in getIP if getaddrinfo returns error.

2015-04-17 Thread Chi Zhang


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

Review request for mesos, Ben Mahler, Evelina Dumitrescu, and Jie Yu.


Bugs: mesos-2636
https://issues.apache.org/jira/browse/mesos-2636


Repository: mesos


Description
---

Removed unnecessary freeaddrinfo in getIP if getaddrinfo returns error.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
3622165af064a1f0a9d461af0e1d0d88a352d4a8 

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


Testing
---


Thanks,

Chi Zhang

Jenkins build is back to normal : Mesos-Trunk-Ubuntu-Build-In-Src-Set-JAVA_HOME #2610

2015-04-17 Thread Apache Jenkins Server

See

Jenkins build is back to normal : Mesos-Trunk-Ubuntu-Build-Out-Of-Src-Disable-Java-Disable-Python-Disable-Webui #2891

2015-04-17 Thread Apache Jenkins Server

See

Jenkins build is back to normal : Mesos » clang,docker||Hadoop,ubuntu:14.10 #155

2015-04-17 Thread Apache Jenkins Server

See

Re: Review Request 29947: Fixed a race condition in hook tests for remove-executor hook.

2015-04-17 Thread Kapil Arya


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

(Updated April 17, 2015, 7:09 p.m.)


Review request for mesos and Niklas Nielsen.


Changes
---

updated diffs


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


Repository: mesos


Description
---

The task must be killed properly before shutting down the driver. 


Diffs (updated)
-

  src/examples/test_hook_module.cpp 2f2da1c5ef85af06c7f366d38ce5b64f39d0076f 
  src/tests/hook_tests.cpp bb9de25bd2c4601d333a3ca1aec13820c7df7378 

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


Testing
---

make check


Thanks,

Kapil Arya

Re: [jira] [Commented] (MESOS-2144) Segmentation Fault in ExamplesTest.LowLevelSchedulerPthread

2015-04-17 Thread Cody Maloney

Yea, it looks like we're hitting multiple different sources of SIGSEGV
inside the test. The ExamplesTest in general on OS X I have 1+ fail on our
OS X Buildbot almost every commit currently.

On Fri, Apr 17, 2015 at 1:53 PM, Benjamin Mahler 
wrote:

> Cody, your stack trace looks unrelated to the original issue here? It
> seems related to checkpointing in the slave.
>
> On Thu, Apr 16, 2015 at 5:02 PM, Benjamin Mahler <
> benjamin.mah...@gmail.com> wrote:
>
>> Hey Cody, the 'F' failure log line was far up from the stack trace, I
>> edited the stack trace to show it.
>>
>> On Thu, Apr 16, 2015 at 2:48 PM, Cody Maloney (JIRA) 
>> wrote:
>>
>> >
>> > [
>> >
>> https://issues.apache.org/jira/browse/MESOS-2144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14498789#comment-14498789
>> > ]
>> >
>> > Cody Maloney commented on MESOS-2144:
>> > -
>> >
>> > Just got one of these with full backtrace:
>> > {code}
>> > I0416 12:21:01.673476 36776 authenticatee.hpp:115] Initializing
>> client
>> > SASL
>> > @0x110e9284a  google::LogMessage::Fail()
>> > @0x110e917dd  google::LogMessage::SendToLog()
>> > @0x110e924ea  google::LogMessage::Flush()
>> > @0x110e99348  google::LogMessageFatal::~LogMessageFatal()
>> > I0416 12:21:01.747539 308416512 process.cpp:2091] Resuming reaper(1)@
>> > 127.0.0.1:52842 at 2015-04-16 19:21:33.747597056+00:00
>> > @0x110e92ca5  google::LogMessageFatal::~LogMessageFatal()
>> > @0x10f3d33d3  _CheckFatal::~_CheckFatal()
>> > @0x10f3d3025  _CheckFatal::~_CheckFatal()
>> > @0x10fd94da6  mesos::internal::slave::Slave::__recover()
>> > @0x10fe7f09d
>> >
>> _ZZN7process8dispatchIN5mesos8internal5slave5SlaveERKNS_6FutureI7NothingEES7_EEvRKNS_3PIDIT_EEMSB_FvT0_ET1_ENKUlPNS_11ProcessBaseEE_clESK_
>> > @0x10fe7ee7f
>> >
>> _ZNSt3__110__function6__funcIZN7process8dispatchIN5mesos8internal5slave5SlaveERKNS2_6FutureI7NothingEESA_EEvRKNS2_3PIDIT_EEMSE_FvT0_ET1_EUlPNS2_11ProcessBaseEE_NS_9allocatorISO_EEFvSN_EEclEOSN_
>> > @0x110d74e7b  std::__1::function<>::operator()()
>> > @0x110d5c5bf  process::ProcessBase::visit()
>> > @0x110de6c0e  process::DispatchEvent::visit()
>> > @0x10f3d0841  process::ProcessBase::serve()
>> > @0x110d45abe  process::ProcessManager::resume()
>> > @0x110d451de  process::schedule()
>> > @ 0x7fff8f1eb268  _pthread_body
>> > @ 0x7fff8f1eb1e5  _pthread_start
>> > @ 0x7fff8f1e941d  thread_start
>> > {code}
>> >
>> > The full log from the test (MESOS_VERBOSE, GLOG_v=2)
>> > {code}
>> > [ RUN  ] ExamplesTest.LowLevelSchedulerPthread
>> > Using temporary directory
>> > '/tmp/ExamplesTest_LowLevelSchedulerPthread_vVqryS'
>> > I0416 12:21:01.637110 2105078528 logging.cpp:177] Logging to STDERR
>> > Enabling authentication for the scheduler
>> > I0416 12:21:01.639566 2105078528 process.cpp:2081] Spawned process __
>> > gc__@127.0.0.1:52945
>> > I0416 12:21:01.639770 2105078528 process.cpp:2081] Spawned process
>> > help@127.0.0.1:52945
>> > I0416 12:21:01.639583 365723648 process.cpp:2091] Resuming __
>> > gc__@127.0.0.1:52945 at 2015-04-16 19:21:01.639622912+00:00
>> > I0416 12:21:01.639777 367869952 process.cpp:2091] Resuming
>> > help@127.0.0.1:52945 at 2015-04-16 19:21:01.639796992+00:00
>> > I0416 12:21:01.639875 366260224 process.cpp:2091] Resuming
>> > logging@127.0.0.1:52945 at 2015-04-16 19:21:01.639906816+00:00
>> > I0416 12:21:01.639909 2105078528 process.cpp:2081] Spawned process
>> > logging@127.0.0.1:52945
>> > I0416 12:21:01.639978 367869952 process.cpp:2091] Resuming
>> > profiler@127.0.0.1:52945 at 2015-04-16 19:21:01.640003840+00:00
>> > I0416 12:21:01.640033 368943104 process.cpp:2091] Resuming
>> > help@127.0.0.1:52945 at 2015-04-16 19:21:01.640058880+00:00
>> > I0416 12:21:01.640051 2105078528 process.cpp:2081] Spawned process
>> > profiler@127.0.0.1:52945
>> > I0416 12:21:01.640246 368406528 process.cpp:2091] Resuming
>> > system@127.0.0.1:52945 at 2015-04-16 19:21:01.640268032+00:00
>> > I0416 12:21:01.640236 368943104 process.cpp:2091] Resuming __
>> > gc__@127.0.0.1:52945 at 2015-04-16 19:21:01.640258048+00:00
>> > I0416 12:21:01.640318 2105078528 process.cpp:2081] Spawned process
>> > system@127.0.0.1:52945
>> > I0416 12:21:01.640321 368943104 process.cpp:2091] Resuming
>> __limiter__(1)@
>> > 127.0.0.1:52945 at 2015-04-16 19:21:01.640336128+00:00
>> > I0416 12:21:01.640390 368406528 process.cpp:2081] Spawned process
>> > __limiter__(1)@127.0.0.1:52945
>> > I0416 12:21:01.640425 365723648 process.cpp:2091] Resuming
>> > metrics@127.0.0.1:52945 at 2015-04-16 19:21:01.640440064+00:00
>> > I0416 12:21:01.640472 368406528 process.cpp:2081] Spawned process
>> > metrics@127.0.0.1:52945
>> > I0416 12:21:01.640521 367869952 process.cpp:2091] Resuming
>> > help@127.0.0.1:52945

Re: Build failed in Jenkins: Mesos » clang,docker||Hadoop,ubuntu:14.10 #154

2015-04-17 Thread Benjamin Mahler

+jie

Can you take a look?

On Fri, Apr 17, 2015 at 3:51 PM, Apache Jenkins Server <
jenk...@builds.apache.org> wrote:

> See <
> https://builds.apache.org/job/Mesos/COMPILER=clang,LABEL=docker%7C%7CHadoop,OS=ubuntu%3A14.10/154/changes
> >
>
> Changes:
>
> [vinodkone] Disable dmesg logging in jenkins build script.
>
> --
> [...truncated 87950 lines...]
> I0417 22:51:27.246206 22989 replica.cpp:744] Replica recovered with log
> positions 0 -> 0 with 1 holes and 0 unlearned
> I0417 22:51:27.247341 23015 leveldb.cpp:306] Persisting metadata (8 bytes)
> to leveldb took 770526ns
> I0417 22:51:27.247375 23015 replica.cpp:323] Persisted replica status to
> VOTING
> I0417 22:51:27.250511 22989 leveldb.cpp:176] Opened db in 2.611364ms
> I0417 22:51:27.253052 22989 leveldb.cpp:183] Compacted db in 2.519284ms
> I0417 22:51:27.253111 22989 leveldb.cpp:198] Created db iterator in 30734ns
> I0417 22:51:27.253150 22989 leveldb.cpp:204] Seeked to beginning of db in
> 28742ns
> I0417 22:51:27.253188 22989 leveldb.cpp:273] Iterated through 1 keys in
> the db in 32144ns
> I0417 22:51:27.253221 22989 replica.cpp:744] Replica recovered with log
> positions 0 -> 0 with 1 holes and 0 unlearned
> I0417 22:51:27.256274 22989 leveldb.cpp:176] Opened db in 2.91878ms
> I0417 22:51:27.259227 22989 leveldb.cpp:183] Compacted db in 2.932472ms
> I0417 22:51:27.259287 22989 leveldb.cpp:198] Created db iterator in 30732ns
> I0417 22:51:27.259326 22989 leveldb.cpp:204] Seeked to beginning of db in
> 26962ns
> I0417 22:51:27.259363 22989 leveldb.cpp:273] Iterated through 1 keys in
> the db in 31402ns
> I0417 22:51:27.259395 22989 replica.cpp:744] Replica recovered with log
> positions 0 -> 0 with 1 holes and 0 unlearned
> I0417 22:51:27.259820 23020 recover.cpp:449] Starting replica recovery
> I0417 22:51:27.260025 23020 recover.cpp:475] Replica is in VOTING status
> I0417 22:51:27.260146 23020 recover.cpp:464] Recover process terminated
> I0417 22:51:27.263207 23021 registrar.cpp:313] Recovering registrar
> [   OK ] Strict/RegistrarTest.fetchTimeout/0 (51 ms)
> [ RUN  ] Strict/RegistrarTest.fetchTimeout/1
> Using temporary directory '/tmp/Strict_RegistrarTest_fetchTimeout_1_pKSVOO'
> I0417 22:51:27.291038 22989 leveldb.cpp:176] Opened db in 3.175751ms
> I0417 22:51:27.292300 22989 leveldb.cpp:183] Compacted db in 1.24202ms
> I0417 22:51:27.292358 22989 leveldb.cpp:198] Created db iterator in 24829ns
> I0417 22:51:27.292377 22989 leveldb.cpp:204] Seeked to beginning of db in
> 7902ns
> I0417 22:51:27.292388 22989 leveldb.cpp:273] Iterated through 0 keys in
> the db in 6253ns
> I0417 22:51:27.292415 22989 replica.cpp:744] Replica recovered with log
> positions 0 -> 0 with 1 holes and 0 unlearned
> I0417 22:51:27.293623 23011 leveldb.cpp:306] Persisting metadata (8 bytes)
> to leveldb took 713330ns
> I0417 22:51:27.293654 23011 replica.cpp:323] Persisted replica status to
> VOTING
> I0417 22:51:27.297266 22989 leveldb.cpp:176] Opened db in 3.074596ms
> I0417 22:51:27.298435 22989 leveldb.cpp:183] Compacted db in 1.149528ms
> I0417 22:51:27.298511 22989 leveldb.cpp:198] Created db iterator in 25044ns
> I0417 22:51:27.298534 22989 leveldb.cpp:204] Seeked to beginning of db in
> 8292ns
> I0417 22:51:27.298545 22989 leveldb.cpp:273] Iterated through 0 keys in
> the db in 6043ns
> I0417 22:51:27.298573 22989 replica.cpp:744] Replica recovered with log
> positions 0 -> 0 with 1 holes and 0 unlearned
> I0417 22:51:27.299579 23019 leveldb.cpp:306] Persisting metadata (8 bytes)
> to leveldb took 724303ns
> I0417 22:51:27.299609 23019 replica.cpp:323] Persisted replica status to
> VOTING
> I0417 22:51:27.303488 22989 leveldb.cpp:176] Opened db in 3.412592ms
> I0417 22:51:27.306718 22989 leveldb.cpp:183] Compacted db in 3.210262ms
> I0417 22:51:27.306776 22989 leveldb.cpp:198] Created db iterator in 29649ns
> I0417 22:51:27.306813 22989 leveldb.cpp:204] Seeked to beginning of db in
> 25497ns
> I0417 22:51:27.306859 22989 leveldb.cpp:273] Iterated through 1 keys in
> the db in 41214ns
> I0417 22:51:27.306910 22989 replica.cpp:744] Replica recovered with log
> positions 0 -> 0 with 1 holes and 0 unlearned
> I0417 22:51:27.310102 22989 leveldb.cpp:176] Opened db in 3.02626ms
> I0417 22:51:27.312993 22989 leveldb.cpp:183] Compacted db in 2.869062ms
> I0417 22:51:27.313052 22989 leveldb.cpp:198] Created db iterator in 30682ns
> I0417 22:51:27.313091 22989 leveldb.cpp:204] Seeked to beginning of db in
> 28522ns
> I0417 22:51:27.313128 22989 leveldb.cpp:273] Iterated through 1 keys in
> the db in 31376ns
> I0417 22:51:27.313161 22989 replica.cpp:744] Replica recovered with log
> positions 0 -> 0 with 1 holes and 0 unlearned
> I0417 22:51:27.313639 23008 recover.cpp:449] Starting replica recovery
> I0417 22:51:27.314113 23015 recover.cpp:475] Replica is in VOTING status
> I0417 22:51:27.314254 23015 recover.cpp:464] Recover process terminated
> I0417 22:51:27.316705 23012 registrar.cpp:313] Recovering registrar
> [

Build failed in Jenkins: Mesos » clang,docker||Hadoop,ubuntu:14.10 #154

2015-04-17 Thread Apache Jenkins Server

See 


Changes:

[vinodkone] Disable dmesg logging in jenkins build script.

--
[...truncated 87950 lines...]
I0417 22:51:27.246206 22989 replica.cpp:744] Replica recovered with log 
positions 0 -> 0 with 1 holes and 0 unlearned
I0417 22:51:27.247341 23015 leveldb.cpp:306] Persisting metadata (8 bytes) to 
leveldb took 770526ns
I0417 22:51:27.247375 23015 replica.cpp:323] Persisted replica status to VOTING
I0417 22:51:27.250511 22989 leveldb.cpp:176] Opened db in 2.611364ms
I0417 22:51:27.253052 22989 leveldb.cpp:183] Compacted db in 2.519284ms
I0417 22:51:27.253111 22989 leveldb.cpp:198] Created db iterator in 30734ns
I0417 22:51:27.253150 22989 leveldb.cpp:204] Seeked to beginning of db in 
28742ns
I0417 22:51:27.253188 22989 leveldb.cpp:273] Iterated through 1 keys in the db 
in 32144ns
I0417 22:51:27.253221 22989 replica.cpp:744] Replica recovered with log 
positions 0 -> 0 with 1 holes and 0 unlearned
I0417 22:51:27.256274 22989 leveldb.cpp:176] Opened db in 2.91878ms
I0417 22:51:27.259227 22989 leveldb.cpp:183] Compacted db in 2.932472ms
I0417 22:51:27.259287 22989 leveldb.cpp:198] Created db iterator in 30732ns
I0417 22:51:27.259326 22989 leveldb.cpp:204] Seeked to beginning of db in 
26962ns
I0417 22:51:27.259363 22989 leveldb.cpp:273] Iterated through 1 keys in the db 
in 31402ns
I0417 22:51:27.259395 22989 replica.cpp:744] Replica recovered with log 
positions 0 -> 0 with 1 holes and 0 unlearned
I0417 22:51:27.259820 23020 recover.cpp:449] Starting replica recovery
I0417 22:51:27.260025 23020 recover.cpp:475] Replica is in VOTING status
I0417 22:51:27.260146 23020 recover.cpp:464] Recover process terminated
I0417 22:51:27.263207 23021 registrar.cpp:313] Recovering registrar
[   OK ] Strict/RegistrarTest.fetchTimeout/0 (51 ms)
[ RUN  ] Strict/RegistrarTest.fetchTimeout/1
Using temporary directory '/tmp/Strict_RegistrarTest_fetchTimeout_1_pKSVOO'
I0417 22:51:27.291038 22989 leveldb.cpp:176] Opened db in 3.175751ms
I0417 22:51:27.292300 22989 leveldb.cpp:183] Compacted db in 1.24202ms
I0417 22:51:27.292358 22989 leveldb.cpp:198] Created db iterator in 24829ns
I0417 22:51:27.292377 22989 leveldb.cpp:204] Seeked to beginning of db in 7902ns
I0417 22:51:27.292388 22989 leveldb.cpp:273] Iterated through 0 keys in the db 
in 6253ns
I0417 22:51:27.292415 22989 replica.cpp:744] Replica recovered with log 
positions 0 -> 0 with 1 holes and 0 unlearned
I0417 22:51:27.293623 23011 leveldb.cpp:306] Persisting metadata (8 bytes) to 
leveldb took 713330ns
I0417 22:51:27.293654 23011 replica.cpp:323] Persisted replica status to VOTING
I0417 22:51:27.297266 22989 leveldb.cpp:176] Opened db in 3.074596ms
I0417 22:51:27.298435 22989 leveldb.cpp:183] Compacted db in 1.149528ms
I0417 22:51:27.298511 22989 leveldb.cpp:198] Created db iterator in 25044ns
I0417 22:51:27.298534 22989 leveldb.cpp:204] Seeked to beginning of db in 8292ns
I0417 22:51:27.298545 22989 leveldb.cpp:273] Iterated through 0 keys in the db 
in 6043ns
I0417 22:51:27.298573 22989 replica.cpp:744] Replica recovered with log 
positions 0 -> 0 with 1 holes and 0 unlearned
I0417 22:51:27.299579 23019 leveldb.cpp:306] Persisting metadata (8 bytes) to 
leveldb took 724303ns
I0417 22:51:27.299609 23019 replica.cpp:323] Persisted replica status to VOTING
I0417 22:51:27.303488 22989 leveldb.cpp:176] Opened db in 3.412592ms
I0417 22:51:27.306718 22989 leveldb.cpp:183] Compacted db in 3.210262ms
I0417 22:51:27.306776 22989 leveldb.cpp:198] Created db iterator in 29649ns
I0417 22:51:27.306813 22989 leveldb.cpp:204] Seeked to beginning of db in 
25497ns
I0417 22:51:27.306859 22989 leveldb.cpp:273] Iterated through 1 keys in the db 
in 41214ns
I0417 22:51:27.306910 22989 replica.cpp:744] Replica recovered with log 
positions 0 -> 0 with 1 holes and 0 unlearned
I0417 22:51:27.310102 22989 leveldb.cpp:176] Opened db in 3.02626ms
I0417 22:51:27.312993 22989 leveldb.cpp:183] Compacted db in 2.869062ms
I0417 22:51:27.313052 22989 leveldb.cpp:198] Created db iterator in 30682ns
I0417 22:51:27.313091 22989 leveldb.cpp:204] Seeked to beginning of db in 
28522ns
I0417 22:51:27.313128 22989 leveldb.cpp:273] Iterated through 1 keys in the db 
in 31376ns
I0417 22:51:27.313161 22989 replica.cpp:744] Replica recovered with log 
positions 0 -> 0 with 1 holes and 0 unlearned
I0417 22:51:27.313639 23008 recover.cpp:449] Starting replica recovery
I0417 22:51:27.314113 23015 recover.cpp:475] Replica is in VOTING status
I0417 22:51:27.314254 23015 recover.cpp:464] Recover process terminated
I0417 22:51:27.316705 23012 registrar.cpp:313] Recovering registrar
[   OK ] Strict/RegistrarTest.fetchTimeout/1 (54 ms)
[ RUN  ] Strict/RegistrarTest.storeTimeout/0
Using temporary directory '/tmp/Strict_RegistrarTest_storeTimeout_0_SCMN4b'
I0417 22:51:27.344132 22989 leveldb.cpp:176] Opened db in 2.536669ms
I0417 22:51:27.345062 22989

Re: Review Request 27113: Libprocess benchmark cleanup

2015-04-17 Thread Ben Mahler

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

Ship it!

Wow, nice work, this is a lot cleaner!

I left some comments but since they are pretty minor, I've gone ahead and taken 
care of them to get this committed for you.

For the dependent change of disabling local messages, I've left it as a TODO 
here so we can get these great cleanups in. Let's follow up with your dependent 
patch to add the ability for the tests to run both the local and remote code 
paths, sound good? :)

3rdparty/libprocess/src/tests/benchmarks.cpp

Looks like we're missing some includes here?

3rdparty/libprocess/src/tests/benchmarks.cpp

Weird wrapping here? You could make this comment more succinct here if you 
omit the request parameters. We probably don't need a /help for now.

3rdparty/libprocess/src/tests/benchmarks.cpp

Maybe a newline in between these?

3rdparty/libprocess/src/tests/benchmarks.cpp

It seems a bit strange to have a `running` that isn't set to false?

3rdparty/libprocess/src/tests/benchmarks.cpp

Since we're capturing the duration for the run, how about just calling it 
'duration'? Finished makes it clear that it's a signal for finishing but that 
seems to express "how" it's used as opposed to "what" it is.

3rdparty/libprocess/src/tests/benchmarks.cpp

It's probably time we give this a more meaningful name than "Test". :)

3rdparty/libprocess/src/tests/benchmarks.cpp

No need for `&` here.

3rdparty/libprocess/src/tests/benchmarks.cpp

How about:

```
  // Start the ping / pongs!
  const string query = strings::join(
  "&",
  "server=" + stringify(serverPid),
  "requests=" + stringify(numRequests),
  "concurrency=" + stringify(concurrency),
  "messageSize=" + stringify(messageSize));
```

3rdparty/libprocess/src/tests/benchmarks.cpp

Can we use std::array in the future?

3rdparty/libprocess/src/tests/benchmarks.cpp

Hm.. we call this 'requests' but we store 'Responses'?

3rdparty/libprocess/src/tests/benchmarks.cpp

It seems like the "iterate and wait ..." aspect of this comment re-iterates 
what the code says?

3rdparty/libprocess/src/tests/benchmarks.cpp

A 'request' is actually a 'Response'?

3rdparty/libprocess/src/tests/benchmarks.cpp

Printing the total rpc throughput like this seems prone to error. For 
example, if each client ran __serially__ at throughput X, we would print X*N 
without realizing that each client's throughput was not overlapping.

Could we just print the throughput for each individual client for now? For 
server throughput, we can track that more precisely in the server itself, yes? 
For now, I'd propose just tracking the total time in the test and estimating 
total throughput based on that.

Also, why not use cout like we did elsewhere?

- Ben Mahler

On April 17, 2015, 5:28 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27113/
> ---
> 
> (Updated April 17, 2015, 5:28 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Joerg Schad, and Michael 
> Park.
> 
> 
> Bugs: MESOS-1980
> https://issues.apache.org/jira/browse/MESOS-1980
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clean up Libprocess for readability / extensibility.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> a927e4ecd8c8955cd9f85e716173a73a9a21c6cd 
> 
> Diff: https://reviews.apache.org/r/27113/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Jenkins build is back to normal : Mesos » gcc,docker||Hadoop,ubuntu:14.10 #153

2015-04-17 Thread Apache Jenkins Server

See

Jenkins build is back to normal : Mesos » clang,docker||Hadoop,ubuntu:14.10 #153

2015-04-17 Thread Apache Jenkins Server

See

Jenkins build is back to normal : mesos-reviewbot #5226

2015-04-17 Thread Apache Jenkins Server

See

Jenkins build is back to normal : Mesos » gcc,docker||Hadoop,centos:7 #153

2015-04-17 Thread Apache Jenkins Server

See

Re: Build failed in Jenkins: Mesos » gcc,docker||Hadoop,centos:7 #152

2015-04-17 Thread Timothy Chen

Hi Vinod,

BenH already pushed a commit to fix this.

Thanks,

Tim

On Fri, Apr 17, 2015 at 3:14 PM, Vinod Kone  wrote:
> ben, looks like this is your commit?
>
> In file included from 
> ../../3rdparty/libprocess/include/process/address.hpp:9:0,
>  from 
> ../../3rdparty/libprocess/include/process/process.hpp:10,
>  from
> ../../3rdparty/libprocess/include/process/c++11/dispatch.hpp:8,
>  from 
> ../../3rdparty/libprocess/include/process/dispatch.hpp:2,
>  from ../../src/slave/containerizer/containerizer.cpp:22:
> ../../src/slave/containerizer/docker.hpp: In constructor
> 'mesos::internal::slave::DockerContainerizerProcess::Container::Container(const
> mesos::ContainerID&, const Option&, const
> mesos::ExecutorInfo&, const string&, const
> Option >&, const mesos::SlaveID&, const
> process::PID&, bool, bool, const
> mesos::internal::slave::Flags&)':
> ../../src/slave/containerizer/docker.hpp:303:36: error: no match for
> 'operator>=' (operand types are 'const
> google::protobuf::RepeatedPtrField' and 'const
> google::protobuf::RepeatedPtrField')
>  CHECK(executor.resources() >= task.get().resources());
> ^
> ../3rdparty/libprocess/3rdparty/glog-0.3.3/src/glog/logging.h:548:5:
> note: in definition of macro 'LOG_IF'
>!(condition) ? (void) 0 : google::LogMessageVoidify() & LOG(severity)
>  ^
> ../3rdparty/libprocess/3rdparty/glog-0.3.3/src/glog/logging.h:562:21:
> note: in expansion of macro 'GOOGLE_PREDICT_BRANCH_NOT_TAKEN'
>LOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN(!(condition))) \
>  ^
> ../../src/slave/containerizer/docker.hpp:303:9: note: in expansion of
> macro 'CHECK'
>  CHECK(executor.resources() >= task.get().resources());
>
>
>
> On Fri, Apr 17, 2015 at 3:10 PM, Apache Jenkins Server <
> jenk...@builds.apache.org> wrote:
>
>> See <
>> https://builds.apache.org/job/Mesos/COMPILER=gcc,LABEL=docker%7C%7CHadoop,OS=centos%3A7/152/changes
>> >
>>
>> Changes:
>>
>> [benjamin.hindman] Fix docker containerizer usage and test.
>>
>> --
>> [...truncated 14953 lines...]
>> [5526919.944592] docker0: port 1(vethbaa02af) entered forwarding state
>> [5528260.552205] device veth2f4139b entered promiscuous mode
>> [5528260.552918] IPv6: ADDRCONF(NETDEV_UP): veth2f4139b: link is not ready
>> [5528260.587532] IPv6: ADDRCONF(NETDEV_CHANGE): veth2f4139b: link becomes
>> ready
>> [5528260.587590] docker0: port 2(veth2f4139b) entered forwarding state
>> [5528260.587602] docker0: port 2(veth2f4139b) entered forwarding state
>> [5528261.089483] docker0: port 2(veth2f4139b) entered disabled state
>> [5528261.090011] device veth2f4139b left promiscuous mode
>> [5528261.090020] docker0: port 2(veth2f4139b) entered disabled state
>> [5528264.183826] device veth6605cd0 entered promiscuous mode
>> [5528264.184472] IPv6: ADDRCONF(NETDEV_UP): veth6605cd0: link is not ready
>> [5528264.226441] IPv6: ADDRCONF(NETDEV_CHANGE): veth6605cd0: link becomes
>> ready
>> [5528264.226479] docker0: port 2(veth6605cd0) entered forwarding state
>> [5528264.226484] docker0: port 2(veth6605cd0) entered forwarding state
>> [5528279.281925] docker0: port 2(veth6605cd0) entered forwarding state
>> [5529322.453364] docker0: port 2(veth6605cd0) entered disabled state
>> [5529322.454146] device veth6605cd0 left promiscuous mode
>> [5529322.454154] docker0: port 2(veth6605cd0) entered disabled state
>> [5530892.344572] docker0: port 1(vethbaa02af) entered disabled state
>> [5530892.345423] device vethbaa02af left promiscuous mode
>> [5530892.345437] docker0: port 1(vethbaa02af) entered disabled state
>> [5531169.621507] device veth85da1b2 entered promiscuous mode
>> [5531169.622008] IPv6: ADDRCONF(NETDEV_UP): veth85da1b2: link is not ready
>> [5531169.656545] IPv6: ADDRCONF(NETDEV_CHANGE): veth85da1b2: link becomes
>> ready
>> [5531169.656584] docker0: port 1(veth85da1b2) entered forwarding state
>> [5531169.656591] docker0: port 1(veth85da1b2) entered forwarding state
>> [5531170.282755] docker0: port 1(veth85da1b2) entered disabled state
>> [5531170.283477] device veth85da1b2 left promiscuous mode
>> [5531170.283488] docker0: port 1(veth85da1b2) entered disabled state
>> [5531173.398973] device veth0a7f876 entered promiscuous mode
>> [5531173.399701] IPv6: ADDRCONF(NETDEV_UP): veth0a7f876: link is not ready
>> [5531173.439748] IPv6: ADDRCONF(NETDEV_CHANGE): veth0a7f876: link becomes
>> ready
>> [5531173.439796] docker0: port 1(veth0a7f876) entered forwarding state
>> [5531173.439806] docker0: port 1(veth0a7f876) entered forwarding state
>> [5531188.478086] docker0: port 1(veth0a7f876) entered forwarding state
>> [5532087.368782] docker0: port 1(veth0a7f876) entered disabled state
>> [5532087.369700] device veth0a7f876 left promiscuous mode
>> [5532087.369715] docker0: port 1(veth0a7f876) entered disabled state
>> [5534768.804830] device vethda651d2 entered prom

Re: Build failed in Jenkins: Mesos » gcc,docker||Hadoop,centos:7 #152

2015-04-17 Thread Vinod Kone

ben, looks like this is your commit?

In file included from ../../3rdparty/libprocess/include/process/address.hpp:9:0,
 from ../../3rdparty/libprocess/include/process/process.hpp:10,
 from
../../3rdparty/libprocess/include/process/c++11/dispatch.hpp:8,
 from ../../3rdparty/libprocess/include/process/dispatch.hpp:2,
 from ../../src/slave/containerizer/containerizer.cpp:22:
../../src/slave/containerizer/docker.hpp: In constructor
'mesos::internal::slave::DockerContainerizerProcess::Container::Container(const
mesos::ContainerID&, const Option&, const
mesos::ExecutorInfo&, const string&, const
Option >&, const mesos::SlaveID&, const
process::PID&, bool, bool, const
mesos::internal::slave::Flags&)':
../../src/slave/containerizer/docker.hpp:303:36: error: no match for
'operator>=' (operand types are 'const
google::protobuf::RepeatedPtrField' and 'const
google::protobuf::RepeatedPtrField')
 CHECK(executor.resources() >= task.get().resources());
^
../3rdparty/libprocess/3rdparty/glog-0.3.3/src/glog/logging.h:548:5:
note: in definition of macro 'LOG_IF'
   !(condition) ? (void) 0 : google::LogMessageVoidify() & LOG(severity)
 ^
../3rdparty/libprocess/3rdparty/glog-0.3.3/src/glog/logging.h:562:21:
note: in expansion of macro 'GOOGLE_PREDICT_BRANCH_NOT_TAKEN'
   LOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN(!(condition))) \
 ^
../../src/slave/containerizer/docker.hpp:303:9: note: in expansion of
macro 'CHECK'
 CHECK(executor.resources() >= task.get().resources());

On Fri, Apr 17, 2015 at 3:10 PM, Apache Jenkins Server <
jenk...@builds.apache.org> wrote:

> See <
> https://builds.apache.org/job/Mesos/COMPILER=gcc,LABEL=docker%7C%7CHadoop,OS=centos%3A7/152/changes
> >
>
> Changes:
>
> [benjamin.hindman] Fix docker containerizer usage and test.
>
> --
> [...truncated 14953 lines...]
> [5526919.944592] docker0: port 1(vethbaa02af) entered forwarding state
> [5528260.552205] device veth2f4139b entered promiscuous mode
> [5528260.552918] IPv6: ADDRCONF(NETDEV_UP): veth2f4139b: link is not ready
> [5528260.587532] IPv6: ADDRCONF(NETDEV_CHANGE): veth2f4139b: link becomes
> ready
> [5528260.587590] docker0: port 2(veth2f4139b) entered forwarding state
> [5528260.587602] docker0: port 2(veth2f4139b) entered forwarding state
> [5528261.089483] docker0: port 2(veth2f4139b) entered disabled state
> [5528261.090011] device veth2f4139b left promiscuous mode
> [5528261.090020] docker0: port 2(veth2f4139b) entered disabled state
> [5528264.183826] device veth6605cd0 entered promiscuous mode
> [5528264.184472] IPv6: ADDRCONF(NETDEV_UP): veth6605cd0: link is not ready
> [5528264.226441] IPv6: ADDRCONF(NETDEV_CHANGE): veth6605cd0: link becomes
> ready
> [5528264.226479] docker0: port 2(veth6605cd0) entered forwarding state
> [5528264.226484] docker0: port 2(veth6605cd0) entered forwarding state
> [5528279.281925] docker0: port 2(veth6605cd0) entered forwarding state
> [5529322.453364] docker0: port 2(veth6605cd0) entered disabled state
> [5529322.454146] device veth6605cd0 left promiscuous mode
> [5529322.454154] docker0: port 2(veth6605cd0) entered disabled state
> [5530892.344572] docker0: port 1(vethbaa02af) entered disabled state
> [5530892.345423] device vethbaa02af left promiscuous mode
> [5530892.345437] docker0: port 1(vethbaa02af) entered disabled state
> [5531169.621507] device veth85da1b2 entered promiscuous mode
> [5531169.622008] IPv6: ADDRCONF(NETDEV_UP): veth85da1b2: link is not ready
> [5531169.656545] IPv6: ADDRCONF(NETDEV_CHANGE): veth85da1b2: link becomes
> ready
> [5531169.656584] docker0: port 1(veth85da1b2) entered forwarding state
> [5531169.656591] docker0: port 1(veth85da1b2) entered forwarding state
> [5531170.282755] docker0: port 1(veth85da1b2) entered disabled state
> [5531170.283477] device veth85da1b2 left promiscuous mode
> [5531170.283488] docker0: port 1(veth85da1b2) entered disabled state
> [5531173.398973] device veth0a7f876 entered promiscuous mode
> [5531173.399701] IPv6: ADDRCONF(NETDEV_UP): veth0a7f876: link is not ready
> [5531173.439748] IPv6: ADDRCONF(NETDEV_CHANGE): veth0a7f876: link becomes
> ready
> [5531173.439796] docker0: port 1(veth0a7f876) entered forwarding state
> [5531173.439806] docker0: port 1(veth0a7f876) entered forwarding state
> [5531188.478086] docker0: port 1(veth0a7f876) entered forwarding state
> [5532087.368782] docker0: port 1(veth0a7f876) entered disabled state
> [5532087.369700] device veth0a7f876 left promiscuous mode
> [5532087.369715] docker0: port 1(veth0a7f876) entered disabled state
> [5534768.804830] device vethda651d2 entered promiscuous mode
> [5534768.805253] IPv6: ADDRCONF(NETDEV_UP): vethda651d2: link is not ready
> [5534768.841377] IPv6: ADDRCONF(NETDEV_CHANGE): vethda651d2: link becomes
> ready
> [5534768.841419] docker0: port 1(vethda651d2) entered forwarding state
> [553

Build failed in Jenkins: Mesos » gcc,docker||Hadoop,ubuntu:14.10 #152

2015-04-17 Thread Apache Jenkins Server

See 


Changes:

[benjamin.hindman] Fix docker containerizer usage and test.

--
[...truncated 14583 lines...]
[5574620.490751] docker0: port 1(veth38d4e7d) entered disabled state
[5574625.547969] device veth481f652 entered promiscuous mode
[5574625.549154] IPv6: ADDRCONF(NETDEV_UP): veth481f652: link is not ready
[5574625.589040] IPv6: ADDRCONF(NETDEV_CHANGE): veth481f652: link becomes ready
[5574625.589093] docker0: port 1(veth481f652) entered forwarding state
[5574625.589109] docker0: port 1(veth481f652) entered forwarding state
[5574637.554237] type=1400 audit(1429084067.845:102): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=23750 comm="systemctl" 
requested_mask="read" denied_mask="read" peer="docker-default"
[5574637.554254] type=1400 audit(1429084067.845:103): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=23750 comm="systemctl" 
requested_mask="read" denied_mask="read" peer="docker-default"
[5574637.554307] type=1400 audit(1429084067.845:104): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=23750 comm="systemctl" 
requested_mask="read" denied_mask="read" peer="docker-default"
[5574637.713580] type=1400 audit(1429084068.004:105): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=23764 comm="systemctl" 
requested_mask="read" denied_mask="read" peer="docker-default"
[5574637.713593] type=1400 audit(1429084068.004:106): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=23764 comm="systemctl" 
requested_mask="read" denied_mask="read" peer="docker-default"
[5574637.713613] type=1400 audit(1429084068.004:107): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=23764 comm="systemctl" 
requested_mask="read" denied_mask="read" peer="docker-default"
[5574639.009896] docker0: port 1(veth481f652) entered disabled state
[5574639.010823] device veth481f652 left promiscuous mode
[5574639.010836] docker0: port 1(veth481f652) entered disabled state
[5574640.490399] device veth27e2bfa entered promiscuous mode
[5574640.490880] IPv6: ADDRCONF(NETDEV_UP): veth27e2bfa: link is not ready
[5574640.524270] IPv6: ADDRCONF(NETDEV_CHANGE): veth27e2bfa: link becomes ready
[5574640.524317] docker0: port 1(veth27e2bfa) entered forwarding state
[5574640.524326] docker0: port 1(veth27e2bfa) entered forwarding state
[5574640.579939] docker0: port 1(veth27e2bfa) entered disabled state
[5574640.580626] device veth27e2bfa left promiscuous mode
[5574640.580639] docker0: port 1(veth27e2bfa) entered disabled state
[5574644.228735] device vethcec6701 entered promiscuous mode
[5574644.229137] IPv6: ADDRCONF(NETDEV_UP): vethcec6701: link is not ready
[5574644.229143] docker0: port 1(vethcec6701) entered forwarding state
[5574644.229149] docker0: port 1(vethcec6701) entered forwarding state
[5574644.248061] docker0: port 1(vethcec6701) entered disabled state
[5574644.268329] IPv6: ADDRCONF(NETDEV_CHANGE): vethcec6701: link becomes ready
[5574644.268368] docker0: port 1(vethcec6701) entered forwarding state
[5574644.268374] docker0: port 1(vethcec6701) entered forwarding state
[5574644.797561] docker0: port 1(vethcec6701) entered disabled state
[5574644.798494] device vethcec6701 left promiscuous mode
[5574644.798507] docker0: port 1(vethcec6701) entered disabled state
[5574647.907509] device vethbab449b entered promiscuous mode
[5574647.908015] IPv6: ADDRCONF(NETDEV_UP): vethbab449b: link is not ready
[5574647.951468] IPv6: ADDRCONF(NETDEV_CHANGE): vethbab449b: link becomes ready
[5574647.951513] docker0: port 1(vethbab449b) entered forwarding state
[5574647.951522] docker0: port 1(vethbab449b) entered forwarding state
[5574663.013879] docker0: port 1(vethbab449b) entered forwarding state
[5575796.578651] docker0: port 1(vethbab449b) entered disabled state
[5575796.579448] device vethbab449b left promiscuous mode
[5575796.579461] docker0: port 1(vethbab449b) entered disabled state
[5612074.324797] device veth99be534 entered promiscuous mode
[5612074.325228] IPv6: ADDRCONF(NETDEV_UP): veth99be534: link is not ready
[5612074.356558] IPv6: ADDRCONF(NETDEV_CHANGE): veth99be534: link becomes ready
[5612074.356600] docker0: port 1(veth99be534) entered forwarding state
[5612074.356607] docker0: port 1(veth99be534) entered forwarding state
[5612089.386042] docker0: port 1(veth99be534) entered forwarding state
[5612097.420086] docker0: port 1(veth99be534) entered disabled state
[5612097.420803] device veth99be534 left promiscuous mode
[5612097.420816] docker0: port 1(veth99be534) entered disabled state
[5612097.732763] device vethb02b956 entered promiscuous mode
[5612097.733212] IPv6: ADDRCONF(NETDEV_UP): vethb02b956: link is not ready
[5612097.733218] docker0: port 1(vethb02b956) entered forwarding state
[5612097.733227] docker0: port 1(vethb02b956) entered forwarding state
[5612097.775164] IPv6: AD

Build failed in Jenkins: Mesos » clang,docker||Hadoop,ubuntu:14.10 #152

2015-04-17 Thread Apache Jenkins Server

See 


Changes:

[benjamin.hindman] Fix docker containerizer usage and test.

--
[...truncated 14735 lines...]
[5203582.883697] type=1400 audit(1428713263.251:23101): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6163 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203582.883903] type=1400 audit(1428713263.251:23102): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6163 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203588.197947] audit_printk_skb: 8034 callbacks suppressed
[5203588.197952] type=1400 audit(1428713268.561:25781): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6167 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203588.197965] type=1400 audit(1428713268.561:25782): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6160 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203588.198106] type=1400 audit(1428713268.561:25783): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6160 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203588.198142] type=1400 audit(1428713268.561:25784): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6167 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203588.198235] type=1400 audit(1428713268.561:25785): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6160 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203588.198373] type=1400 audit(1428713268.562:25786): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6160 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203588.198464] type=1400 audit(1428713268.562:25787): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6167 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203588.198594] type=1400 audit(1428713268.562:25788): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6160 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203588.198633] type=1400 audit(1428713268.562:25789): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6167 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203588.198728] type=1400 audit(1428713268.562:25790): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6160 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203593.230932] audit_printk_skb: 1209 callbacks suppressed
[5203593.230939] type=1400 audit(1428713273.590:26194): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6154 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203593.230983] type=1400 audit(1428713273.590:26195): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6166 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203593.231168] type=1400 audit(1428713273.590:26196): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6154 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203593.231234] type=1400 audit(1428713273.590:26197): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6166 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203593.231300] type=1400 audit(1428713273.590:26198): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6154 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203593.231447] type=1400 audit(1428713273.591:26199): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6166 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203593.231523] type=1400 audit(1428713273.591:26200): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6154 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203593.231629] type=1400 audit(1428713273.591:26201): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6166 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203593.231661] type=1400 audit(1428713273.591:26202): apparmor="DENIED" 
operation="ptrace" profile="docker-default" pid=6154 comm="lt-mesos-tests" 
requested_mask="trace" denied_mask="trace" peer="docker-default"
[5203593.231773] type=1400 audit(1428713273.591:26203): apparmor="DENIED" 
operatio

Build failed in Jenkins: Mesos » gcc,docker||Hadoop,centos:7 #152

2015-04-17 Thread Apache Jenkins Server

See 


Changes:

[benjamin.hindman] Fix docker containerizer usage and test.

--
[...truncated 14953 lines...]
[5526919.944592] docker0: port 1(vethbaa02af) entered forwarding state
[5528260.552205] device veth2f4139b entered promiscuous mode
[5528260.552918] IPv6: ADDRCONF(NETDEV_UP): veth2f4139b: link is not ready
[5528260.587532] IPv6: ADDRCONF(NETDEV_CHANGE): veth2f4139b: link becomes ready
[5528260.587590] docker0: port 2(veth2f4139b) entered forwarding state
[5528260.587602] docker0: port 2(veth2f4139b) entered forwarding state
[5528261.089483] docker0: port 2(veth2f4139b) entered disabled state
[5528261.090011] device veth2f4139b left promiscuous mode
[5528261.090020] docker0: port 2(veth2f4139b) entered disabled state
[5528264.183826] device veth6605cd0 entered promiscuous mode
[5528264.184472] IPv6: ADDRCONF(NETDEV_UP): veth6605cd0: link is not ready
[5528264.226441] IPv6: ADDRCONF(NETDEV_CHANGE): veth6605cd0: link becomes ready
[5528264.226479] docker0: port 2(veth6605cd0) entered forwarding state
[5528264.226484] docker0: port 2(veth6605cd0) entered forwarding state
[5528279.281925] docker0: port 2(veth6605cd0) entered forwarding state
[5529322.453364] docker0: port 2(veth6605cd0) entered disabled state
[5529322.454146] device veth6605cd0 left promiscuous mode
[5529322.454154] docker0: port 2(veth6605cd0) entered disabled state
[5530892.344572] docker0: port 1(vethbaa02af) entered disabled state
[5530892.345423] device vethbaa02af left promiscuous mode
[5530892.345437] docker0: port 1(vethbaa02af) entered disabled state
[5531169.621507] device veth85da1b2 entered promiscuous mode
[5531169.622008] IPv6: ADDRCONF(NETDEV_UP): veth85da1b2: link is not ready
[5531169.656545] IPv6: ADDRCONF(NETDEV_CHANGE): veth85da1b2: link becomes ready
[5531169.656584] docker0: port 1(veth85da1b2) entered forwarding state
[5531169.656591] docker0: port 1(veth85da1b2) entered forwarding state
[5531170.282755] docker0: port 1(veth85da1b2) entered disabled state
[5531170.283477] device veth85da1b2 left promiscuous mode
[5531170.283488] docker0: port 1(veth85da1b2) entered disabled state
[5531173.398973] device veth0a7f876 entered promiscuous mode
[5531173.399701] IPv6: ADDRCONF(NETDEV_UP): veth0a7f876: link is not ready
[5531173.439748] IPv6: ADDRCONF(NETDEV_CHANGE): veth0a7f876: link becomes ready
[5531173.439796] docker0: port 1(veth0a7f876) entered forwarding state
[5531173.439806] docker0: port 1(veth0a7f876) entered forwarding state
[5531188.478086] docker0: port 1(veth0a7f876) entered forwarding state
[5532087.368782] docker0: port 1(veth0a7f876) entered disabled state
[5532087.369700] device veth0a7f876 left promiscuous mode
[5532087.369715] docker0: port 1(veth0a7f876) entered disabled state
[5534768.804830] device vethda651d2 entered promiscuous mode
[5534768.805253] IPv6: ADDRCONF(NETDEV_UP): vethda651d2: link is not ready
[5534768.841377] IPv6: ADDRCONF(NETDEV_CHANGE): vethda651d2: link becomes ready
[5534768.841419] docker0: port 1(vethda651d2) entered forwarding state
[5534768.841426] docker0: port 1(vethda651d2) entered forwarding state
[5534769.471955] docker0: port 1(vethda651d2) entered disabled state
[5534769.473032] device vethda651d2 left promiscuous mode
[5534769.473045] docker0: port 1(vethda651d2) entered disabled state
[5534772.599384] device veth2906e4c entered promiscuous mode
[5534772.599883] IPv6: ADDRCONF(NETDEV_UP): veth2906e4c: link is not ready
[5534772.640127] IPv6: ADDRCONF(NETDEV_CHANGE): veth2906e4c: link becomes ready
[5534772.640196] docker0: port 1(veth2906e4c) entered forwarding state
[5534772.640208] docker0: port 1(veth2906e4c) entered forwarding state
[5534787.702207] docker0: port 1(veth2906e4c) entered forwarding state
[5535680.238978] docker0: port 1(veth2906e4c) entered disabled state
[5535680.239910] device veth2906e4c left promiscuous mode
[5535680.239930] docker0: port 1(veth2906e4c) entered disabled state
[5541975.543734] device veth6cc9e48 entered promiscuous mode
[5541975.544249] IPv6: ADDRCONF(NETDEV_UP): veth6cc9e48: link is not ready
[5541975.579588] IPv6: ADDRCONF(NETDEV_CHANGE): veth6cc9e48: link becomes ready
[5541975.579649] docker0: port 1(veth6cc9e48) entered forwarding state
[5541975.579658] docker0: port 1(veth6cc9e48) entered forwarding state
[5541976.214763] docker0: port 1(veth6cc9e48) entered disabled state
[5541976.215504] device veth6cc9e48 left promiscuous mode
[5541976.215524] docker0: port 1(veth6cc9e48) entered disabled state
[5541979.277199] device vethf9341e1 entered promiscuous mode
[5541979.277684] IPv6: ADDRCONF(NETDEV_UP): vethf9341e1: link is not ready
[5541979.324809] IPv6: ADDRCONF(NETDEV_CHANGE): vethf9341e1: link becomes ready
[5541979.324876] docker0: port 1(vethf9341e1) entered forwarding state
[5541979.324887] docker0: port 1(vethf9341e1) entered forwarding state
[5541994.351844] docker0: port

Re: Review Request 33318: Fix docker containerizer usage and test.

2015-04-17 Thread Benjamin Hindman

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

Ship it!

src/slave/containerizer/docker.hpp

There is a big global invariant here that we should call out even more 
explicitly for future readers of the code. That is, we add the initial task 
resources to the ExecutorInfo so that the initial container we create is big 
enough since some ExecutorInfo's might not actually add resources. This is an 
unfortunate wart that got added that we should really clean up at some point 
(basically, we shouldn't need to have the Slave add the tasks resources to the 
ExecutorInfo because the Containerizer should just be able to figure that out 
itself). In the mean time, we should have a better comment and even:

if (task.isSome()) {
  CHECK(executor.resources() >= task.get().resources());
}

- Benjamin Hindman

On April 17, 2015, 6:21 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33318/
> ---
> 
> (Updated April 17, 2015, 6:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix docker containerizer usage and test.
> The docker usage test is failing with the most recent change in including 
> executor resources in docker containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 6893684e6d199a5d69fc8bba8e60c4acaae9c3c9 
>   src/tests/docker_containerizer_tests.cpp 
> c772d4c836de18b0e87636cb42200356d24ec73d 
> 
> Diff: https://reviews.apache.org/r/33318/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 31665: Changed '(used|offered)Resources' to 'total(Used|Offered)Resources' and added 'hashmap (used|offered)Resources' to 'master::Framework'

2015-04-17 Thread Ben Mahler


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

Ship it!


- Ben Mahler


On April 17, 2015, 5:31 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> ---
> 
> (Updated April 17, 2015, 5:31 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `master::Framework` holds 2 member variables of type `Resources`: 
> `usedResources` and `offeredResources`. Both of these are aggregates of 
> resources from multiple slaves. We add the `hashmap` 
> versions since we can lose non-scalar resources by summing them up across 
> multiple slaves. For further details refer to 
> [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).
> 
> In [r31666](https://reviews.apache.org/r/31666/), we report 
> `usedResourcesBySlaveId` rather than `usedResources` when adding the 
> framework to the allocator.
> 
> We don't actually use `offeredResourcesBySlaveId` currently, but it should be 
> kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` 
> as well as `offeredResourcesBySlaveId` via the HTTP endpoint.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp f2b123d7f84eafc792849ad4630c2e3b63be000b 
>   src/master/master.hpp 4a1df58e6756bff7d5f292c0542023a9215e33d8 
>   src/master/master.cpp 02f35ac2d98d288bf9d2b9195770b839bbfe6d7a 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: [jira] [Commented] (MESOS-2144) Segmentation Fault in ExamplesTest.LowLevelSchedulerPthread

2015-04-17 Thread Benjamin Mahler

Cody, your stack trace looks unrelated to the original issue here? It seems
related to checkpointing in the slave.

On Thu, Apr 16, 2015 at 5:02 PM, Benjamin Mahler 
wrote:

> Hey Cody, the 'F' failure log line was far up from the stack trace, I
> edited the stack trace to show it.
>
> On Thu, Apr 16, 2015 at 2:48 PM, Cody Maloney (JIRA) 
> wrote:
>
> >
> > [
> >
> https://issues.apache.org/jira/browse/MESOS-2144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14498789#comment-14498789
> > ]
> >
> > Cody Maloney commented on MESOS-2144:
> > -
> >
> > Just got one of these with full backtrace:
> > {code}
> > I0416 12:21:01.673476 36776 authenticatee.hpp:115] Initializing
> client
> > SASL
> > @0x110e9284a  google::LogMessage::Fail()
> > @0x110e917dd  google::LogMessage::SendToLog()
> > @0x110e924ea  google::LogMessage::Flush()
> > @0x110e99348  google::LogMessageFatal::~LogMessageFatal()
> > I0416 12:21:01.747539 308416512 process.cpp:2091] Resuming reaper(1)@
> > 127.0.0.1:52842 at 2015-04-16 19:21:33.747597056+00:00
> > @0x110e92ca5  google::LogMessageFatal::~LogMessageFatal()
> > @0x10f3d33d3  _CheckFatal::~_CheckFatal()
> > @0x10f3d3025  _CheckFatal::~_CheckFatal()
> > @0x10fd94da6  mesos::internal::slave::Slave::__recover()
> > @0x10fe7f09d
> >
> _ZZN7process8dispatchIN5mesos8internal5slave5SlaveERKNS_6FutureI7NothingEES7_EEvRKNS_3PIDIT_EEMSB_FvT0_ET1_ENKUlPNS_11ProcessBaseEE_clESK_
> > @0x10fe7ee7f
> >
> _ZNSt3__110__function6__funcIZN7process8dispatchIN5mesos8internal5slave5SlaveERKNS2_6FutureI7NothingEESA_EEvRKNS2_3PIDIT_EEMSE_FvT0_ET1_EUlPNS2_11ProcessBaseEE_NS_9allocatorISO_EEFvSN_EEclEOSN_
> > @0x110d74e7b  std::__1::function<>::operator()()
> > @0x110d5c5bf  process::ProcessBase::visit()
> > @0x110de6c0e  process::DispatchEvent::visit()
> > @0x10f3d0841  process::ProcessBase::serve()
> > @0x110d45abe  process::ProcessManager::resume()
> > @0x110d451de  process::schedule()
> > @ 0x7fff8f1eb268  _pthread_body
> > @ 0x7fff8f1eb1e5  _pthread_start
> > @ 0x7fff8f1e941d  thread_start
> > {code}
> >
> > The full log from the test (MESOS_VERBOSE, GLOG_v=2)
> > {code}
> > [ RUN  ] ExamplesTest.LowLevelSchedulerPthread
> > Using temporary directory
> > '/tmp/ExamplesTest_LowLevelSchedulerPthread_vVqryS'
> > I0416 12:21:01.637110 2105078528 logging.cpp:177] Logging to STDERR
> > Enabling authentication for the scheduler
> > I0416 12:21:01.639566 2105078528 process.cpp:2081] Spawned process __
> > gc__@127.0.0.1:52945
> > I0416 12:21:01.639770 2105078528 process.cpp:2081] Spawned process
> > help@127.0.0.1:52945
> > I0416 12:21:01.639583 365723648 process.cpp:2091] Resuming __
> > gc__@127.0.0.1:52945 at 2015-04-16 19:21:01.639622912+00:00
> > I0416 12:21:01.639777 367869952 process.cpp:2091] Resuming
> > help@127.0.0.1:52945 at 2015-04-16 19:21:01.639796992+00:00
> > I0416 12:21:01.639875 366260224 process.cpp:2091] Resuming
> > logging@127.0.0.1:52945 at 2015-04-16 19:21:01.639906816+00:00
> > I0416 12:21:01.639909 2105078528 process.cpp:2081] Spawned process
> > logging@127.0.0.1:52945
> > I0416 12:21:01.639978 367869952 process.cpp:2091] Resuming
> > profiler@127.0.0.1:52945 at 2015-04-16 19:21:01.640003840+00:00
> > I0416 12:21:01.640033 368943104 process.cpp:2091] Resuming
> > help@127.0.0.1:52945 at 2015-04-16 19:21:01.640058880+00:00
> > I0416 12:21:01.640051 2105078528 process.cpp:2081] Spawned process
> > profiler@127.0.0.1:52945
> > I0416 12:21:01.640246 368406528 process.cpp:2091] Resuming
> > system@127.0.0.1:52945 at 2015-04-16 19:21:01.640268032+00:00
> > I0416 12:21:01.640236 368943104 process.cpp:2091] Resuming __
> > gc__@127.0.0.1:52945 at 2015-04-16 19:21:01.640258048+00:00
> > I0416 12:21:01.640318 2105078528 process.cpp:2081] Spawned process
> > system@127.0.0.1:52945
> > I0416 12:21:01.640321 368943104 process.cpp:2091] Resuming
> __limiter__(1)@
> > 127.0.0.1:52945 at 2015-04-16 19:21:01.640336128+00:00
> > I0416 12:21:01.640390 368406528 process.cpp:2081] Spawned process
> > __limiter__(1)@127.0.0.1:52945
> > I0416 12:21:01.640425 365723648 process.cpp:2091] Resuming
> > metrics@127.0.0.1:52945 at 2015-04-16 19:21:01.640440064+00:00
> > I0416 12:21:01.640472 368406528 process.cpp:2081] Spawned process
> > metrics@127.0.0.1:52945
> > I0416 12:21:01.640521 367869952 process.cpp:2091] Resuming
> > help@127.0.0.1:52945 at 2015-04-16 19:21:01.640538880+00:00
> > I0416 12:21:01.640733 366796800 process.cpp:2091] Resuming
> > help@127.0.0.1:52945 at 2015-04-16 19:21:01.640760064+00:00
> > I0416 12:21:01.640913 2105078528 process.cpp:2081] Spawned process __
> > processes__@127.0.0.1:52945
> > I0416 12:21:01.640919 366796800 process.cpp:2091] Resuming __
> > processes__@127.0.0.1:52945 at 2015

Re: Review Request 31263: Refactored TestAllocator and allocator text fixture.

2015-04-17 Thread Michael Park



> On April 2, 2015, 7:58 p.m., Vinod Kone wrote:
> > src/tests/master_allocator_tests.cpp, lines 96-104
> > 
> >
> > While the TearDown() avoids flakiness by ensuring that an allocator 
> > process doesn't exist after a test, it doesn't address cases where a master 
> > is restarted with a new allocator in the same test (e.g., 
> > FrameworkReregistersFirst and SlaveReregistersFirst) causing both old and 
> > new allocators to coexist at the same time and talking to the same master.
> > 
> > I suggest to have methods on the test fixture that creates and deletes 
> > allocator explicitly (can be used by tests that restart master). The 
> > TearDown() will still delete any active allocator. For safety, ensure that 
> > only one allocator is active  at any given time.
> > 
> > Does that make sense?
> > 
> > ```
> > template 
> > class MasterAllocatorTest : public MesosTest
> > {
> > protected: 
> >   MasterAllocatorTest() : allocator(Null) {}
> >   
> >   Try create()
> >   {
> > // We don't support multiple allocators because...
> > if (allocator != NULL) {
> >   return Error("Multiple active allocators are not supported");
> > }
> > 
> > allocator = new TestAllocator(process::Owned(new T));
> > return *allocator;
> >   }
> >   
> >   void destroy()
> >   {
> > delete allocator;
> > allocator = NULL;
> >   }
> >   
> >   virtual void TearDown()
> >   {
> > destroy();
> > MeosTest::TearDown();
> >   }
> > 
> > private:
> >   TestAllocator* allocator;
> > };
> > 
> > ```
> 
> Alexander Rukletsov wrote:
> We used to have a lifecycle method but decided to remove it:
> be6246a11276074dfb60a892a890b80cb7cd37cd
> RR: https://reviews.apache.org/r/29927/
> 
> The two tests you point to are currently the only ones that create an 
> additional allocator. They both create a new leader instance and appoint the 
> slave instance to the new leader, and there is no possibility AFAIK to swap 
> allocators in a leader instance. So yes, there are two active allocators in 
> these tests, but they belong to different leaders.
> 
> Current design ensures there is only one active allocator in the fixture, 
> but allows to create more if needed (what those two tests do). The design you 
> propose doesn't prevent us from creating one more allocator in the test body 
> either.
> 
> Thoughts?
> 
> Michael Park wrote:
> @Vinod: Just for clarification,
> > causing both old and new allocators to coexist at the same time and 
> talking to the same master.
> They do coexist at the same time, but they don't talk to the same master.
> 
> My suggestion here is to contain each of the runs in their own lexical 
> scope. Here's what `FrameworkReregistersFirst` could look like with this 
> approach:
> ```
> TYPED_TEST(MasterAllocatorTest, FrameworkReregistersFirst)
> {
>   // Components that last both runs.
>   MockExecutor exec(DEFAULT_EXECUTOR_ID);
> 
>   StandaloneMasterDetector slaveDetector;
> 
>   slave::Flags flags = this->CreateSlaveFlags();
>   flags.resources = Some("cpus:2;mem:1024");
> 
>   Try > slave = this->StartSlave(&exec, &slaveDetector, flags);
>   ASSERT_SOME(slave);
> 
>   MockScheduler sched;
>   StandaloneMasterDetector schedulerDetector;
>   TestingMesosSchedulerDriver driver(&sched, &schedulerDetector);
> 
>   driver.start();
> 
>   /* run1 */ {
> TestAllocator allocator1 = TestAllocator::create();
> 
> Try> master1 = this->StartMaster(&allocator1);
> ASSERT_SOME(master1);
> 
> schedulerDetector.appoint(master1.get());
> 
> slaveDetector.appoint(master1.get());
> 
> ...
> 
> this->ShutdownMasters();
>   }
>   
>   /* run2 */ {
> TestAllocator allocator2 = TestAllocator::create();
> 
> Try> master2 = this->StartMaster(&allocator2);
> ASSERT_SOME(master2);
> 
> schedulerDetector.appoint(master2.get());
> 
> slaveDetector.appoint(master2.get());
> 
> ...
> 
> this->ShutdownMasters();
>   }
> 
>   // Shut everything down.
>   driver.stop();
>   driver.join();
> 
>   this->Shutdown();
> }  
> ```
> 
> I think the common components, and each of the runs are clearly captured 
> in this approach, the lifetimes of the objects are well contained within the 
> runs and we could use `allocator` and `master` in both scopes as well if we 
> felt like it.
> 
> The `TestAllocator::create` above is suggested in 
> https://reviews.apache.org/r/31265.
> If we find `TestAllocator::create()` to be too verbose, w

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-17 Thread Michael Park



> On April 17, 2015, 1:31 p.m., Michael Park wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 77
> > 
> >
> > I see this is the place where you wanted to use `ASSERT_SOME` but 
> > couldn't due to the [gtest 
> > limitation](https://code.google.com/p/googletest/wiki/AdvancedGuide#Assertion_Placement).
> > 
> > One way to solve this I think is to use `SetUp/TearDown` rather than 
> > `Ctor/Dtor`.
> > Another way may be to use `CHECK_SOME` in place of `ASSERT_SOME`. I 
> > think this makes sense as well since success of 
> > `HierarchicalDRFAllocator::create` isn't actually a testing condition.
> > 
> > Thoughts?

Actually, is it possible just use a `TestAllocator` constructed from 
`TestAllocator::create<>()` as suggested below?


- Michael


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


On April 15, 2015, 2:19 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> ---
> 
> (Updated April 15, 2015, 2:19 p.m.)
> 
> 
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The factory creates allocator instances in a way identical to how instances 
> from modules are created. It allows us to use same typed tests for built-in 
> and modularized allocators.
> 
> 
> Diffs
> -
> 
>   src/examples/test_allocator_module.cpp PRE-CREATION 
>   src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e 
>   src/master/allocator/mesos/allocator.hpp 
> fb898f1175b61b442204e6e38c69ccc2838a646f 
>   src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f 
>   src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec 
>   src/tests/hierarchical_allocator_tests.cpp 
> 8861bf398e4bb17b0f74eab4f4af26202447ccef 
>   src/tests/master_allocator_tests.cpp 
> 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302 
>   src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4 
> 
> Diff: https://reviews.apache.org/r/31265/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.9.5, Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-17 Thread Michael Park



> On April 17, 2015, 1:31 p.m., Michael Park wrote:
> > src/tests/mesos.hpp, lines 866-874
> > 
> >
> > An idea to avoid this: we could promote the `createAllocator` currently 
> > in `MasterAllocatorTest` to this file. Then we could do:
> > 
> > ```
> > TestAllocator(
> >   process::Owned realAllocator =
> > createAllocator<
> >   mesos::internal::master::allocator::HierarchicalDRFAllocator>())
> > ```

Scratch that. I think we can do better. I think rather than `createAllocator` 
or `createTestAllocator`, a `create` factory function for `TestAllocator` fits 
nicely!

```cpp
class TestAllocator : public mesos::master::allocator::Allocator
{
  public:

  template 
  static TestAllocator create() {
// T represents the allocator type. It can be a default built-in
// allocator, or one provided by an allocator module.
Try allocator = T::create();
CHECK_SOME(allocator);

// Wrap allocator instance in TestAllocator.
process::Owned realAllocator(CHECK_NOTNULL(allocator.get()));
return TestAllocator(realAllocator);
  }

  ...
  
  private:

  // No need for default argument here.
  TestAllocator(process::Owned 
realAllocator)
: real(realAllocator)
  {
...
  }

};
```


- Michael


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


On April 15, 2015, 2:19 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> ---
> 
> (Updated April 15, 2015, 2:19 p.m.)
> 
> 
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The factory creates allocator instances in a way identical to how instances 
> from modules are created. It allows us to use same typed tests for built-in 
> and modularized allocators.
> 
> 
> Diffs
> -
> 
>   src/examples/test_allocator_module.cpp PRE-CREATION 
>   src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e 
>   src/master/allocator/mesos/allocator.hpp 
> fb898f1175b61b442204e6e38c69ccc2838a646f 
>   src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f 
>   src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec 
>   src/tests/hierarchical_allocator_tests.cpp 
> 8861bf398e4bb17b0f74eab4f4af26202447ccef 
>   src/tests/master_allocator_tests.cpp 
> 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302 
>   src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4 
> 
> Diff: https://reviews.apache.org/r/31265/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.9.5, Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 31263: Refactored TestAllocator and allocator text fixture.

2015-04-17 Thread Michael Park



> On April 2, 2015, 7:58 p.m., Vinod Kone wrote:
> > src/tests/master_allocator_tests.cpp, lines 96-104
> > 
> >
> > While the TearDown() avoids flakiness by ensuring that an allocator 
> > process doesn't exist after a test, it doesn't address cases where a master 
> > is restarted with a new allocator in the same test (e.g., 
> > FrameworkReregistersFirst and SlaveReregistersFirst) causing both old and 
> > new allocators to coexist at the same time and talking to the same master.
> > 
> > I suggest to have methods on the test fixture that creates and deletes 
> > allocator explicitly (can be used by tests that restart master). The 
> > TearDown() will still delete any active allocator. For safety, ensure that 
> > only one allocator is active  at any given time.
> > 
> > Does that make sense?
> > 
> > ```
> > template 
> > class MasterAllocatorTest : public MesosTest
> > {
> > protected: 
> >   MasterAllocatorTest() : allocator(Null) {}
> >   
> >   Try create()
> >   {
> > // We don't support multiple allocators because...
> > if (allocator != NULL) {
> >   return Error("Multiple active allocators are not supported");
> > }
> > 
> > allocator = new TestAllocator(process::Owned(new T));
> > return *allocator;
> >   }
> >   
> >   void destroy()
> >   {
> > delete allocator;
> > allocator = NULL;
> >   }
> >   
> >   virtual void TearDown()
> >   {
> > destroy();
> > MeosTest::TearDown();
> >   }
> > 
> > private:
> >   TestAllocator* allocator;
> > };
> > 
> > ```
> 
> Alexander Rukletsov wrote:
> We used to have a lifecycle method but decided to remove it:
> be6246a11276074dfb60a892a890b80cb7cd37cd
> RR: https://reviews.apache.org/r/29927/
> 
> The two tests you point to are currently the only ones that create an 
> additional allocator. They both create a new leader instance and appoint the 
> slave instance to the new leader, and there is no possibility AFAIK to swap 
> allocators in a leader instance. So yes, there are two active allocators in 
> these tests, but they belong to different leaders.
> 
> Current design ensures there is only one active allocator in the fixture, 
> but allows to create more if needed (what those two tests do). The design you 
> propose doesn't prevent us from creating one more allocator in the test body 
> either.
> 
> Thoughts?

@Vinod: Just for clarification,
> causing both old and new allocators to coexist at the same time and talking 
> to the same master.
They do coexist at the same time, but they don't talk to the same master.

My suggestion here is to contain each of the runs in their own lexical scope. 
Here's what `FrameworkReregistersFirst` could look like with this approach:
```
TYPED_TEST(MasterAllocatorTest, FrameworkReregistersFirst)
{
  // Components that last both runs.
  MockExecutor exec(DEFAULT_EXECUTOR_ID);

  StandaloneMasterDetector slaveDetector;

  slave::Flags flags = this->CreateSlaveFlags();
  flags.resources = Some("cpus:2;mem:1024");

  Try > slave = this->StartSlave(&exec, &slaveDetector, flags);
  ASSERT_SOME(slave);

  MockScheduler sched;
  StandaloneMasterDetector schedulerDetector;
  TestingMesosSchedulerDriver driver(&sched, &schedulerDetector);

  driver.start();

  /* run1 */ {
TestAllocator allocator1 = TestAllocator::create();

Try> master1 = this->StartMaster(&allocator1);
ASSERT_SOME(master1);

schedulerDetector.appoint(master1.get());

slaveDetector.appoint(master1.get());

...

this->ShutdownMasters();
  }
  
  /* run2 */ {
TestAllocator allocator2 = TestAllocator::create();

Try> master2 = this->StartMaster(&allocator2);
ASSERT_SOME(master2);

schedulerDetector.appoint(master2.get());

slaveDetector.appoint(master2.get());

...

this->ShutdownMasters();
  }

  // Shut everything down.
  driver.stop();
  driver.join();

  this->Shutdown();
}  
```

I think the common components, and each of the runs are clearly captured in 
this approach, the lifetimes of the objects are well contained within the runs 
and we could use `allocator` and `master` in both scopes as well if we felt 
like it.

The `TestAllocator::create` above is suggested in 
https://reviews.apache.org/r/31265.
If we find `TestAllocator::create()` to be too verbose, we could 
also provide a `createTestAllocator` alias in `MasterAllocatorTest`:

```
template 
class MasterAllocatorTest : public MesosTest
{

  ...

  TestAllocator createTestAllocator() const {
return TestAllocator::create();
  }

  ...
};
```


- Michael


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

Re: Review Request 33257: Fixed recover tasks only by the intiated containerizer.

2015-04-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [33257]

All tests passed.

- Mesos ReviewBot


On April 17, 2015, 7:07 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33257/
> ---
> 
> (Updated April 17, 2015, 7:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Ian Downes, Jie 
> Yu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2601
> https://issues.apache.org/jira/browse/MESOS-2601
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed recover tasks only by the intiated containerizer.
> Currently both mesos and docker containerizer recovers tasks that wasn't 
> started by themselves.
> The proposed fix is to record the intended containerizer in the checkpointed 
> executorInfo, and reuse that information on recover to know if the 
> containerizer should recover or not. We are free to modify the executorInfo 
> since it's not being used to relaunch any task.
> The external containerizer doesn't need to change since it is only recovering 
> containers that are returned by the containers script.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 6893684e6d199a5d69fc8bba8e60c4acaae9c3c9 
>   src/slave/containerizer/docker.cpp f9fb07806e3b7d7d2afc1be3b8756eac23b32dcd 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e4136095fca55637864f495098189ab3ad8d8fe7 
>   src/slave/slave.cpp a0595f93ce4720f5b9926326d01210460ccb0667 
>   src/tests/containerizer_tests.cpp 5991aa628083dac7c5e8bf7ba297f4f9edeec05f 
>   src/tests/docker_containerizer_tests.cpp 
> c772d4c836de18b0e87636cb42200356d24ec73d 
> 
> Diff: https://reviews.apache.org/r/33257/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 33257: Fixed recover tasks only by the intiated containerizer.

2015-04-17 Thread Timothy Chen


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

(Updated April 17, 2015, 7:07 p.m.)


Review request for mesos, Benjamin Hindman, Bernd Mathiske, Ian Downes, Jie Yu, 
and Till Toenshoff.


Changes
---

This change also handles recovering docker containers after upgrade.


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


Repository: mesos


Description
---

Fixed recover tasks only by the intiated containerizer.
Currently both mesos and docker containerizer recovers tasks that wasn't 
started by themselves.
The proposed fix is to record the intended containerizer in the checkpointed 
executorInfo, and reuse that information on recover to know if the 
containerizer should recover or not. We are free to modify the executorInfo 
since it's not being used to relaunch any task.
The external containerizer doesn't need to change since it is only recovering 
containers that are returned by the containers script.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 6893684e6d199a5d69fc8bba8e60c4acaae9c3c9 
  src/slave/containerizer/docker.cpp f9fb07806e3b7d7d2afc1be3b8756eac23b32dcd 
  src/slave/containerizer/mesos/containerizer.cpp 
e4136095fca55637864f495098189ab3ad8d8fe7 
  src/slave/slave.cpp a0595f93ce4720f5b9926326d01210460ccb0667 
  src/tests/containerizer_tests.cpp 5991aa628083dac7c5e8bf7ba297f4f9edeec05f 
  src/tests/docker_containerizer_tests.cpp 
c772d4c836de18b0e87636cb42200356d24ec73d 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 33257: Fixed recover tasks only by the intiated containerizer.

2015-04-17 Thread Timothy Chen


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



src/slave/containerizer/docker.cpp


We'll fix this later altogether with another patch


- Timothy Chen


On April 16, 2015, 7:10 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33257/
> ---
> 
> (Updated April 16, 2015, 7:10 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Ian Downes, Jie 
> Yu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2601
> https://issues.apache.org/jira/browse/MESOS-2601
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed recover tasks only by the intiated containerizer.
> Currently both mesos and docker containerizer recovers tasks that wasn't 
> started by themselves.
> The proposed fix is to record the intended containerizer in the checkpointed 
> executorInfo, and reuse that information on recover to know if the 
> containerizer should recover or not. We are free to modify the executorInfo 
> since it's not being used to relaunch any task.
> The external containerizer doesn't need to change since it is only recovering 
> containers that are returned by the containers script.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp f9fb07806e3b7d7d2afc1be3b8756eac23b32dcd 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e4136095fca55637864f495098189ab3ad8d8fe7 
>   src/slave/slave.cpp a0595f93ce4720f5b9926326d01210460ccb0667 
>   src/tests/containerizer_tests.cpp 5991aa628083dac7c5e8bf7ba297f4f9edeec05f 
>   src/tests/docker_containerizer_tests.cpp 
> c772d4c836de18b0e87636cb42200356d24ec73d 
> 
> Diff: https://reviews.apache.org/r/33257/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 33318: Fix docker containerizer usage and test.

2015-04-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [33318]

All tests passed.

- Mesos ReviewBot


On April 17, 2015, 6:21 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33318/
> ---
> 
> (Updated April 17, 2015, 6:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix docker containerizer usage and test.
> The docker usage test is failing with the most recent change in including 
> executor resources in docker containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 6893684e6d199a5d69fc8bba8e60c4acaae9c3c9 
>   src/tests/docker_containerizer_tests.cpp 
> c772d4c836de18b0e87636cb42200356d24ec73d 
> 
> Diff: https://reviews.apache.org/r/33318/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 30961: Enabled label decorator to override.

2015-04-17 Thread Niklas Nielsen



> On April 8, 2015, 5:31 p.m., Adam B wrote:
> > src/examples/test_hook_module.cpp, line 36
> > 
> >
> > Unused? Or should you check that the value being removed is what you 
> > expect?

It was just to keep the module and test code in sync. Would you prefer I remove 
it?


> On April 8, 2015, 5:31 p.m., Adam B wrote:
> > src/examples/test_hook_module.cpp, lines 52-57
> > 
> >
> > label shadows label? Maybe oldLabel/newLabel? Then you can also reuse 
> > the `Label*` in line 52 and 59 (`label_`)

Good point - will get that in.


- Niklas


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


On April 7, 2015, 5:57 p.m., Niklas Nielsen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30961/
> ---
> 
> (Updated April 7, 2015, 5:57 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Kapil Arya.
> 
> 
> Bugs: MESOS-2351
> https://issues.apache.org/jira/browse/MESOS-2351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 9ae8b9455a86c7a5cbf4f1d1b1ce88f2811ce35d 
>   src/examples/test_hook_module.cpp 47409cd4d02e238d1d182571d92019114662cd41 
>   src/hook/manager.cpp 7a4cb09bc221af502e867cfb7fff2900b599ff1f 
>   src/master/master.cpp 618db68ee4163b06e479cf3413eda4b63c9c5a4b 
>   src/tests/hook_tests.cpp bb9de25bd2c4601d333a3ca1aec13820c7df7378 
> 
> Diff: https://reviews.apache.org/r/30961/diff/
> 
> 
> Testing
> ---
> 
> make check (with modified VerifyMasterLaunchTaskHook test)
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

Re: Review Request 31028: Added slave run task hook tests.

2015-04-17 Thread Niklas Nielsen



> On April 11, 2015, 3:54 a.m., Adam B wrote:
> > src/examples/test_hook_module.cpp, lines 80-85
> > 
> >
> > Create variables like testLabelKey, etc. above so it's easier to track 
> > all these label k/v strings.

I would prefer if we could defer this to a subsequent review; we use foo, bar, 
baz, qux quite a few places for label testing. Is that OK with you? I can 
create a JIRA for it now if you'd like.


- Niklas


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


On April 11, 2015, 3:03 a.m., Niklas Nielsen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31028/
> ---
> 
> (Updated April 11, 2015, 3:03 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Kapil Arya.
> 
> 
> Bugs: MESOS-2351
> https://issues.apache.org/jira/browse/MESOS-2351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary
> 
> 
> Diffs
> -
> 
>   src/examples/test_hook_module.cpp 47409cd4d02e238d1d182571d92019114662cd41 
>   src/tests/hook_tests.cpp bb9de25bd2c4601d333a3ca1aec13820c7df7378 
> 
> Diff: https://reviews.apache.org/r/31028/diff/
> 
> 
> Testing
> ---
> 
> make check (with newly added VerifySlaveRunTaskHook test)
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

Re: Review Request 31016: Added slave run task decorator.

2015-04-17 Thread Niklas Nielsen



> On April 11, 2015, 3:26 a.m., Adam B wrote:
> > src/slave/slave.cpp, lines 1186-1188
> > 
> >
> > What makes this the ideal place to do the label decoration? Looks like 
> > this is wedged between setting up different unschedule calls. Seems like we 
> > should either decorate the labels as early as possible, right after (or 
> > before?) the pid/id/state checks; or, if we need to delay it, we could do 
> > it after the unschedules.

Great point - I will probably move it up before the unschedule code. Does that 
sound OK to you?


- Niklas


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


On April 11, 2015, 3:03 a.m., Niklas Nielsen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31016/
> ---
> 
> (Updated April 11, 2015, 3:03 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Kapil Arya.
> 
> 
> Bugs: MESOS-2351
> https://issues.apache.org/jira/browse/MESOS-2351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added decorator which gets invoked on start of runTask() sequence in the 
> slave.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 9ae8b9455a86c7a5cbf4f1d1b1ce88f2811ce35d 
>   src/hook/manager.hpp da813492108974a7e26b366845368517589da876 
>   src/hook/manager.cpp 7a4cb09bc221af502e867cfb7fff2900b599ff1f 
>   src/slave/slave.hpp 19e6b44bc344c0ca509674803f401cbb4e1f47ae 
>   src/slave/slave.cpp 9fec023b643d410f4d511fa6f80e9835bab95b7e 
> 
> Diff: https://reviews.apache.org/r/31016/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

Re: Review Request 31028: Added slave run task hook tests.

2015-04-17 Thread Niklas Nielsen



> On April 11, 2015, 3:54 a.m., Adam B wrote:
> > src/tests/hook_tests.cpp, lines 132-140
> > 
> >
> > Why this change? What's wrong with the TestContainerizer?

I was running into issues as the test containerizer doesn't support using the 
command executor. Let me double check real quick if this is a moot change 
compared to the test refactor later in this patch set.


- Niklas


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


On April 11, 2015, 3:03 a.m., Niklas Nielsen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31028/
> ---
> 
> (Updated April 11, 2015, 3:03 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Kapil Arya.
> 
> 
> Bugs: MESOS-2351
> https://issues.apache.org/jira/browse/MESOS-2351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary
> 
> 
> Diffs
> -
> 
>   src/examples/test_hook_module.cpp 47409cd4d02e238d1d182571d92019114662cd41 
>   src/tests/hook_tests.cpp bb9de25bd2c4601d333a3ca1aec13820c7df7378 
> 
> Diff: https://reviews.apache.org/r/31028/diff/
> 
> 
> Testing
> ---
> 
> make check (with newly added VerifySlaveRunTaskHook test)
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

Review Request 33318: Fix docker containerizer usage and test.

2015-04-17 Thread Timothy Chen


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

Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.


Repository: mesos


Description
---

Fix docker containerizer usage and test.
The docker usage test is failing with the most recent change in including 
executor resources in docker containerizer.


Diffs
-

  src/slave/containerizer/docker.hpp 6893684e6d199a5d69fc8bba8e60c4acaae9c3c9 
  src/tests/docker_containerizer_tests.cpp 
c772d4c836de18b0e87636cb42200356d24ec73d 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 27113: Libprocess benchmark cleanup

2015-04-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [33315, 27113]

All tests passed.

- Mesos ReviewBot


On April 17, 2015, 5:28 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27113/
> ---
> 
> (Updated April 17, 2015, 5:28 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Joerg Schad, and Michael 
> Park.
> 
> 
> Bugs: MESOS-1980
> https://issues.apache.org/jira/browse/MESOS-1980
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clean up Libprocess for readability / extensibility.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> a927e4ecd8c8955cd9f85e716173a73a9a21c6cd 
> 
> Diff: https://reviews.apache.org/r/27113/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 29947: Fixed a race condition in hook tests for remove-executor hook.

2015-04-17 Thread Niklas Nielsen

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

This patch introduce some technical debt: There is currently no good way to 
synchronize between the test body and the hook code, so we wire a promise 
(owned by the test code). Let's capture this well by creating JIRA's, 
commenting the code and add it to the description of this review request, so it 
goes into the commit message.

src/examples/test_hook_module.cpp

Let's add a lot of context here as to why we need to share this between the 
module and the test body :)

src/tests/hook_tests.cpp

Symbol doesn't describe if it is going to be a function or just a variable. 
I wonder if we can come up with a bit more precise name

src/tests/hook_tests.cpp

Can you expand on why you need to do this?

src/tests/hook_tests.cpp

This is the part I hoped we could get around. Can you add a todo with the 
jira referring to the technical debt?

src/tests/hook_tests.cpp

s/  / /

- Niklas Nielsen

On Jan. 27, 2015, 4:39 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29947/
> ---
> 
> (Updated Jan. 27, 2015, 4:39 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2226
> https://issues.apache.org/jira/browse/MESOS-2226
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The task must be killed properly before shutting down the driver. 
> 
> 
> Diffs
> -
> 
>   src/examples/test_hook_module.cpp 04fd43eb3eacae0d850dd7f4e191116d20620f10 
>   src/tests/hook_tests.cpp 44f73effdce2d03627215418007ccbc3263a0c52 
> 
> Diff: https://reviews.apache.org/r/29947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 32850: Moved cram-md5 authenticatee process definition into implementation file.

2015-04-17 Thread Michael Park



> On April 14, 2015, 2:16 p.m., Alexander Rukletsov wrote:
> > src/authentication/cram_md5/authenticatee.cpp, lines 46-47
> > 
> >
> > Let's move to newline to avoid jaggeddness. Btw, what does clang-format 
> > suggest?
> 
> Kapil Arya wrote:
> According to our style guide, this is okay but we keep seeing issues 
> being raised about this. Should we just update the style guide instead?
> 
> Alexander Rukletsov wrote:
> I would like to have just one way of doing this. I think the one with 
> newline scales better.
> 
> Till Toenshoff wrote:
> This was exactly clang-format's suggestion. Raised 
> https://issues.apache.org/jira/browse/MESOS-2618

Sorry about that, `clang-format` gets this "wrong" currently. It's documented 
under [Known 
Limitations](https://github.com/apache/mesos/blob/master/docs/clang-format.md#known-limitations).


- Michael


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


On April 15, 2015, 12:44 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32850/
> ---
> 
> (Updated April 15, 2015, 12:44 a.m.)
> 
> 
> Review request for mesos, Adam B, Joris Van Remoortere, and switched to 
> 'mcypark'.
> 
> 
> Bugs: MESOS-2584
> https://issues.apache.org/jira/browse/MESOS-2584
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Removing the process from the header is much cleaner and also fixes the 
> linked clang 3.4.2 JIRA. Apart from that moving, no code is changed.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am d15a373 
>   src/authentication/cram_md5/authenticatee.hpp 55fac68 
>   src/authentication/cram_md5/authenticatee.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/32850/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 32536: Updated variable naming style.

2015-04-17 Thread Michael Park


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


Captured my thoughts on this topic in the JIRA ticket. I think this would be a 
good step towards what I think we ought to get to, which is to assign the 
following groups into different naming schemes: types, local variables 
(including function parameters), member variables, and member-access functions.

- Michael Park


On April 14, 2015, 1:08 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32536/
> ---
> 
> (Updated April 14, 2015, 1:08 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Michael Park, Niklas 
> Nielsen, and Timothy Chen.
> 
> 
> Bugs: MESOS-2616
> https://issues.apache.org/jira/browse/MESOS-2616
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Documents the patterns we use to name variables and function arguments in our 
> codebase.
> 
> Leading underscores to avoid ambiguity.
> ===
> 
> We use this pattern extensively in libprocess, stout and mesos, a few 
> examples below.
> 
> * `stout/try.hpp:105`
> ```
> Try(State _state, T* _t = NULL, const std::string& _message = "")
>   : state(_state), t(_t), message(_message) {}
> ```
> 
> * `process/http.hpp:480`
> ```
>   URL(const std::string& _scheme,
>   const std::string& _domain,
>   const uint16_t _port = 80,
>   const std::string& _path = "/",
>   const hashmap& _query =
> (hashmap()),
>   const Option& _fragment = None())
> : scheme(_scheme),
>   domain(_domain),
>   port(_port),
>   path(_path),
>   query(_query),
>   fragment(_fragment) {}
> ```
> 
> * `slave/containerizer/linux_launcher.cpp:56`
> ```
> LinuxLauncher::LinuxLauncher(
> const Flags& _flags,
> int _namespaces,
> const string& _hierarchy)
>   : flags(_flags),
> namespaces(_namespaces),
> hierarchy(_hierarchy) {}
> ```
> 
> Trailing undescores as prime symbols.
> =
> 
> We use this pattern in the code, though not extensively. I would like to see 
> more pass-by-value instead of creating copies from a variable passed by const 
> reference.
> 
> * `master.cpp:2942`
> ```
> // Create and add the slave id.
> SlaveInfo slaveInfo_ = slaveInfo;
> slaveInfo_.mutable_id()->CopyFrom(newSlaveId());
> ```
> 
> * `slave.cpp:4180`
> ```
> ExecutorInfo executorInfo_ = executor->info;
> Resources resources = executorInfo_.resources();
> resources += taskInfo.resources();
> executorInfo_.mutable_resources()->CopyFrom(resources);
> ```
> 
> * `status_update_manager.cpp:474`
> ```
> // Bounded exponential backoff.
> Duration duration_ =
> std::min(duration * 2, STATUS_UPDATE_RETRY_INTERVAL_MAX);
> ```
> 
> * `containerizer/mesos/containerizer.cpp:109`
> ```
> // Modify the flags to include any changes to isolation.
> Flags flags_ = flags;
> flags_.isolation = isolation;
> ```
> 
> Passing arguments by value.
> ===
> 
> * `slave.cpp:2480`
> ```
> void Slave::statusUpdate(StatusUpdate update, const UPID& pid)
> {
>   ...
>   // Set the source before forwarding the status update.
>   update.mutable_status()->set_source(
>   pid == UPID() ? TaskStatus::SOURCE_SLAVE : TaskStatus::SOURCE_EXECUTOR);
>   ...
> }
> ```
> 
> * `process/metrics/timer.hpp:103`
> ```
>   static void _time(Time start, Timer that)
>   {
> const Time stop = Clock::now();
> 
> double value;
> 
> process::internal::acquire(&that.data->lock);
> {
>   that.data->lastValue = T(stop - start).value();
>   value = that.data->lastValue.get();
> }
> process::internal::release(&that.data->lock);
> 
> that.push(value);
>   }
> ```
> 
> 
> Diffs
> -
> 
>   docs/mesos-c++-style-guide.md 439fe12 
> 
> Diff: https://reviews.apache.org/r/32536/diff/
> 
> 
> Testing
> ---
> 
> None (not a functional change).
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 27113: Libprocess benchmark cleanup

2015-04-17 Thread Joris Van Remoortere



> On April 14, 2015, 9:28 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/tests/benchmarks.cpp, lines 108-114
> > 
> >
> > This seems to suggest support for multiple "runs" of the client. 
> > However, if a /run request arrives while the previous run is in progress, 
> > it looks like this will behave strangely. Did you want to deny requests 
> > while a run is in progress, or chain the runs after one another?

Caught this state and return an error message for now.


> On April 14, 2015, 9:28 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/tests/benchmarks.cpp, line 266
> > 
> >
> > Could you do a self review of this test body, from the perspective of 
> > someone approaching this code for the first time?
> > 
> > I had a hard time understanding this, at a basic level what is the 
> > overall structure of the forking here? It's also a bit tricky to figure out 
> > the lower level details, like what the `requestPipes` and `resultPipes` are 
> > for, and why they're needed. What the vectors of pipes are used for, why 
> > they're needed. Etc.
> > 
> > In the process of being able to articulate what this does, we might 
> > figure out how we can make it more easily understandable :)

Take a look at the updated review. I'll let you mark this as fixed if you 
approve :-)


- Joris


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


On April 17, 2015, 5:28 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27113/
> ---
> 
> (Updated April 17, 2015, 5:28 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Joerg Schad, and Michael 
> Park.
> 
> 
> Bugs: MESOS-1980
> https://issues.apache.org/jira/browse/MESOS-1980
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clean up Libprocess for readability / extensibility.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> a927e4ecd8c8955cd9f85e716173a73a9a21c6cd 
> 
> Diff: https://reviews.apache.org/r/27113/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 27113: Libprocess benchmark cleanup

2015-04-17 Thread Joris Van Remoortere


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

(Updated April 17, 2015, 5:28 p.m.)


Review request for mesos, Ben Mahler, Cody Maloney, Joerg Schad, and Michael 
Park.


Changes
---

Address bmahler's comments.
Rebased on 33315 which allows us to disable short-circuit passing of messages 
between actors. This means we can get rid of the forking code path, which 
simplifies the test significantly.


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


Repository: mesos


Description
---

Clean up Libprocess for readability / extensibility.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/benchmarks.cpp 
a927e4ecd8c8955cd9f85e716173a73a9a21c6cd 

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


Testing
---

make check


Thanks,

Joris Van Remoortere

Re: Review Request 29406: Introduce libevent ssl socket.

2015-04-17 Thread Joris Van Remoortere


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

(Updated April 17, 2015, 4:05 p.m.)


Review request for Benjamin Hindman, Bernd Mathiske, Cody Maloney, Joerg Schad, 
Marco Massenzio, and Michael Park.


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


Repository: mesos


Description (updated)
---

Requires:
configure --enable-libevent --enable-libevent-socket --enable-ssl
New environment variables:
USE_SSL=(0,1)
SSL_CERT=(path to certificate)
SSL_KEY=(path to key)
SSL_VERIFY_CERT=(0,1)
SSL_REQUIRE_CERT=(0,1)
SSL_CA_DIR=(path to CA directory)
SSL_CA_FILE=(path to CA file)

TODO:
Restrict SSL version more tightly
Track down leak in crypto from accept


Diffs
-

  3rdparty/libprocess/Makefile.am 8f96f49a386a70f14324d3a4744aa0b8bf3995f9 
  3rdparty/libprocess/include/process/socket.hpp 
ddb9e365fc1e65a568bdac4973964df1ab8cc05e 
  3rdparty/libprocess/src/libevent.hpp f6cc72178613a30446629532a773afccfd404212 
  3rdparty/libprocess/src/libevent.cpp 28c2cf7f49cc153158f2a470a1812e35f7d4b93a 
  3rdparty/libprocess/src/libevent_ssl_socket.hpp PRE-CREATION 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp PRE-CREATION 
  3rdparty/libprocess/src/openssl.hpp PRE-CREATION 
  3rdparty/libprocess/src/openssl.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp 67b6b3b9c13d95fa1a24b48a12c5c831c7f249bf 
  3rdparty/libprocess/src/socket.cpp 4b0f6bec8051f938812dbc90a7312e4082ea203f 

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


Testing
---

make check (uses non-ssl socket)
benchmarks using ssl sockets
master, slave, framework, webui launch with ssl sockets


Thanks,

Joris Van Remoortere

Re: Review Request 33263: Extended SlaveTest.ShutdownUnregisteredExecutor test with a reason check.

2015-04-17 Thread Alexander Rojas


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

Ship it!


Ship It!

- Alexander Rojas


On April 16, 2015, 4:31 p.m., Andrey Dyatlov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33263/
> ---
> 
> (Updated April 16, 2015, 4:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-2625
> https://issues.apache.org/jira/browse/MESOS-2625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extended SlaveTest.ShutdownUnregisteredExecutor test with a reason check. 
> Check that the reason is REASON_COMMAND_EXECUTOR_FAILED. According to the 
> Slave::sendExecutorTerminatedStatusUpdate member function, this reason is 
> expected instead of more general REASON_EXECUTOR_TERMINATED because the 
> command executer is used in this test.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp b826000e0a4221690f956ea51f49ad4c99d5e188 
> 
> Diff: https://reviews.apache.org/r/33263/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andrey Dyatlov
> 
>

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-17 Thread Michael Park


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



src/local/local.cpp


What's the point of setting `_allocator` here? It looks like it only gets 
passed to the `new Master(...)` call.

Can we just leave `_allocator` alone and do

```
if (allocator_.isError()) {
  LOG(ERROR) << "Failed to create an instance of HierarchicalDRFAllocator: "
 << allocator_.error();
  allocator = NULL;
}

// Save the instance for deleting later.
allocator = allocator_.get();
```

then pass `allocator` to `new Master` below?



src/tests/hierarchical_allocator_tests.cpp


I see this is the place where you wanted to use `ASSERT_SOME` but couldn't 
due to the [gtest 
limitation](https://code.google.com/p/googletest/wiki/AdvancedGuide#Assertion_Placement).

One way to solve this I think is to use `SetUp/TearDown` rather than 
`Ctor/Dtor`.
Another way may be to use `CHECK_SOME` in place of `ASSERT_SOME`. I think 
this makes sense as well since success of `HierarchicalDRFAllocator::create` 
isn't actually a testing condition.

Thoughts?



src/tests/mesos.hpp


An idea to avoid this: we could promote the `createAllocator` currently in 
`MasterAllocatorTest` to this file. Then we could do:

```
TestAllocator(
  process::Owned realAllocator =
createAllocator<
  mesos::internal::master::allocator::HierarchicalDRFAllocator>())
```


- Michael Park


On April 15, 2015, 2:19 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31265/
> ---
> 
> (Updated April 15, 2015, 2:19 p.m.)
> 
> 
> Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2160
> https://issues.apache.org/jira/browse/MESOS-2160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The factory creates allocator instances in a way identical to how instances 
> from modules are created. It allows us to use same typed tests for built-in 
> and modularized allocators.
> 
> 
> Diffs
> -
> 
>   src/examples/test_allocator_module.cpp PRE-CREATION 
>   src/local/local.cpp 289b9bced7bab0d745fe14823efa4e90ec36905e 
>   src/master/allocator/mesos/allocator.hpp 
> fb898f1175b61b442204e6e38c69ccc2838a646f 
>   src/master/main.cpp 7cce3a0bb808a1cb7bac9acab31eb1c67a15ea9f 
>   src/tests/cluster.hpp a56b6541adcdebc5866571bbdbb6828df97b34ec 
>   src/tests/hierarchical_allocator_tests.cpp 
> 8861bf398e4bb17b0f74eab4f4af26202447ccef 
>   src/tests/master_allocator_tests.cpp 
> 03a1bb8c92b44bc1ad1b5f5cff8d1fb971df2302 
>   src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4 
> 
> Diff: https://reviews.apache.org/r/31265/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.9.5, Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 33155: Added commented-out tests for slave removal metrics.

2015-04-17 Thread Alexander Rojas


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



src/tests/master_tests.cpp


+1


- Alexander Rojas


On April 14, 2015, 3:46 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33155/
> ---
> 
> (Updated April 14, 2015, 3:46 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2485
> https://issues.apache.org/jira/browse/MESOS-2485
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> I updated the existing tests to validate the metrics. However, it blocks the 
> tests due to the slave metrics never getting satisfied. Added a TODO.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 32b1e9bb58d8046e5363fafe2ab8f662b6c9a666 
>   src/tests/partition_tests.cpp 1018e479a77a6b533f2dd392fedbdccb80e3ac1f 
>   src/tests/slave_tests.cpp b826000e0a4221690f956ea51f49ad4c99d5e188 
> 
> Diff: https://reviews.apache.org/r/33155/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 33263: Extended SlaveTest.ShutdownUnregisteredExecutor test with a reason check.

2015-04-17 Thread Alexander Rukletsov


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

Ship it!


Ship It!

- Alexander Rukletsov


On April 16, 2015, 2:31 p.m., Andrey Dyatlov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33263/
> ---
> 
> (Updated April 16, 2015, 2:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-2625
> https://issues.apache.org/jira/browse/MESOS-2625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extended SlaveTest.ShutdownUnregisteredExecutor test with a reason check. 
> Check that the reason is REASON_COMMAND_EXECUTOR_FAILED. According to the 
> Slave::sendExecutorTerminatedStatusUpdate member function, this reason is 
> expected instead of more general REASON_EXECUTOR_TERMINATED because the 
> command executer is used in this test.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp b826000e0a4221690f956ea51f49ad4c99d5e188 
> 
> Diff: https://reviews.apache.org/r/33263/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andrey Dyatlov
> 
>

Re: Review Request 33154: Added reason metrics for slave removals.

2015-04-17 Thread Alexander Rojas


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



src/master/master.hpp


I find this quite counter intuitive for two reasons:
1. If I just check this header, I would expect reason to behave a little 
like an enum, telling me why the slave is being removed. What i didn't expect, 
is that it is a counter to the number of slaves being removed by `reason`.
2. The parameter is a const ref, last thing I expected is that it would 
change later on.

Can you add a comment explaining the semantics of reason, and if possible, 
getting rid of the const ref?


- Alexander Rojas


On April 14, 2015, 3:46 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33154/
> ---
> 
> (Updated April 14, 2015, 3:46 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2485
> https://issues.apache.org/jira/browse/MESOS-2485
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See [MESOS-2485](https://issues.apache.org/jira/browse/MESOS-2485).
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3a8e8bf303e0576c212951f6028af77e54d93537 
>   src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
>   src/master/master.cpp 44b0a0147f5354824d86332a67b30018634c9a36 
>   src/master/metrics.hpp 52a83289cfe7e6b6fd8d5bff0774ebe5ce51d0ed 
>   src/master/metrics.cpp 14486bf7130250f561c9fb7a43c95f3fc1e76a4b 
> 
> Diff: https://reviews.apache.org/r/33154/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 33153: Moved a partition test into partition_tests.cpp.

2015-04-17 Thread Alexander Rojas


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

Ship it!


As in the previous case, can you add the JIRA issue link. I also would like to 
understand why the move is needed (though I might see it later).

- Alexander Rojas


On April 14, 2015, 3:46 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33153/
> ---
> 
> (Updated April 14, 2015, 3:46 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> a637c32f004638a110390b22cf5b626e904097cf 
>   src/tests/partition_tests.cpp 1018e479a77a6b533f2dd392fedbdccb80e3ac1f 
> 
> Diff: https://reviews.apache.org/r/33153/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 33152: Moved the slave shutdown test into slave_tests.cpp.

2015-04-17 Thread Alexander Rojas


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

Ship it!


Can you please add the JIRA number in the review?


src/tests/slave_tests.cpp


One line break too many.


- Alexander Rojas


On April 14, 2015, 3:46 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33152/
> ---
> 
> (Updated April 14, 2015, 3:46 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> a637c32f004638a110390b22cf5b626e904097cf 
>   src/tests/slave_tests.cpp b826000e0a4221690f956ea51f49ad4c99d5e188 
> 
> Diff: https://reviews.apache.org/r/33152/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 31667: Piped hashmap from allocator through to sorter.

2015-04-17 Thread Michael Park



> On April 8, 2015, 2:09 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 349
> > 
> >
> > Does it need to be inside the loop?

Absolutely not! Good catch. Fixed.


> On April 8, 2015, 2:09 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/sorter/drf/sorter.cpp, line 254
> > 
> >
> > Is it a TODO or a NOTE?

Should have been a `NOTE`. Fixed.


> On April 8, 2015, 2:09 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/sorter/drf/sorter.cpp, line 258
> > 
> >
> > `_allocations` is per-client allocation, let's reflect this in the name 
> > and get rid of leading underscore. How about `clientAllocations`?

Fixed to `s/_allocations/clientAllocation/`. Dropped the 's' because it's a 
single allocation for a client out of `allocations`. The semantics are the same 
as the `allocation` function.

Also `s/_resources/totalResources/`.


- Michael


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


On April 17, 2015, 4:57 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31667/
> ---
> 
> (Updated April 17, 2015, 4:57 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `Sorter` changes:
> 
> - Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with 
> `SlaveID`.
> - `allocation` returns `hashmap`.
> 
> `DRFSorter` changes:
> 
> - `allocations` is updated from `hashmap` to 
> `hashmap>`.
> - `resources` is updated from `Resources` to `hashmap`.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 9f9a631ee52a3ab194e678f9be139e8dabc7f3db 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 4366710d6530b784aa5094813328d0e338239ba0 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b 
>   src/master/allocator/sorter/sorter.hpp 
> e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b 
>   src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd 
> 
> Diff: https://reviews.apache.org/r/31667/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

2015-04-17 Thread Michael Park



> On April 16, 2015, 7:27 p.m., Ben Mahler wrote:
> > src/master/master.hpp, line 1071
> > 
> >
> > For example, could we document in NOTE on these total variables that 
> > subtraction is safe even though non-scalars lose information?
> > 
> > Specifically, is my understanding correct here: 
> > 
> > (1) For overlapping set items / ranges across slaves, these will get 
> > added N times but only represented 1 time in the Resources. 
> > (2) So, when an initial subtraction occurs (N-1), the resource is no 
> > longer represented. We don't expose non-scalars for now, so this is safe 
> > for now.
> > (3) When any further subtractions occur (N-(1+M)), the Resources simply 
> > ignores the subtraction since there's nothing to remove, so this is safe 
> > for now.

I think this was meant as a subsequent comment to the one above, so I'll answer 
here.

I'm not sure if I correctly understand your definition of __safety__ and what's 
considered a __bug__.
> (1) For overlapping set items / ranges across slaves, these will get added N 
> times but only represented 1 time in the Resources.

Correct.
> (2) So, when an initial subtraction occurs (N-1), the resource is no longer 
> represented.

Correct.
> We don't expose non-scalars for now, so this is safe for now.

We actually do expose non-scalars through the master HTTP endpoint. We used to 
only expose `cpus`, `mem`, `disk`, and `ports` but since 
[r31082](https://reviews.apache.org/r/31082) we expose all resources. But even 
before, `ports` was reported incorrectly. If we mean __safe__ as in we don't 
crash, then yes it's safe. But I would probably consider this to be at least a 
__bug__.
> (3) When any further subtractions occur (N-(1+M)), the Resources simply 
> ignores the subtraction since there's nothing to remove, so this is safe for 
> now.

Correct.

After the sequence of patches land, the totals (used and offered) are merely 
used to be exposed via `state.json`. The non-scalars are still incorrect, but 
I'm not sure that we can assume that nobody's using them. If we decide to only 
expose the scalar resources since the non-scalar information was wrong anyway, 
I think that should definitely be done in a separate patch at least to keep the 
commits clean (as I mentioned, MESOS-2623 will track that). Furthermore, I 
think changing what resources get exposed via `state.json` is considered an API 
change and therefore is probably worthwhile mentioning in the `CHANGELOG` for 
the next release.


- Michael


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


On April 15, 2015, 10:45 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> ---
> 
> (Updated April 15, 2015, 10:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `master::Framework` holds 2 member variables of type `Resources`: 
> `usedResources` and `offeredResources`. Both of these are aggregates of 
> resources from multiple slaves. We add the `hashmap` 
> versions since we can lose non-scalar resources by summing them up across 
> multiple slaves. For further details refer to 
> [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).
> 
> In [r31666](https://reviews.apache.org/r/31666/), we report 
> `usedResourcesBySlaveId` rather than `usedResources` when adding the 
> framework to the allocator.
> 
> We don't actually use `offeredResourcesBySlaveId` currently, but it should be 
> kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` 
> as well as `offeredResourcesBySlaveId` via the HTTP endpoint.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

2015-04-17 Thread Michael Park


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

(Updated April 17, 2015, 5:02 a.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


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


Repository: mesos


Description
---

`master::Framework` holds 2 member variables of type `Resources`: 
`usedResources` and `offeredResources`. Both of these are aggregates of 
resources from multiple slaves. We add the `hashmap` 
versions since we can lose non-scalar resources by summing them up across 
multiple slaves. For further details refer to 
[MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).

In [r31666](https://reviews.apache.org/r/31666/), we report 
`usedResourcesBySlaveId` rather than `usedResources` when adding the framework 
to the allocator.

We don't actually use `offeredResourcesBySlaveId` currently, but it should be 
kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as 
well as `offeredResourcesBySlaveId` via the HTTP endpoint.


Diffs (updated)
-

  src/master/master.hpp 4a1df58e6756bff7d5f292c0542023a9215e33d8 

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


Testing
---

make check


Thanks,

Michael Park

Re: Review Request 31665: Changed '(used|offered)Resources' to 'total(Used|Offered)Resources' and added 'hashmap (used|offered)Resources' to 'master::Framework'

2015-04-17 Thread Michael Park


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

(Updated April 17, 2015, 5:22 a.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Summary (updated)
-

Changed '(used|offered)Resources' to 'total(Used|Offered)Resources' and added 
'hashmap (used|offered)Resources' to 'master::Framework'


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


Repository: mesos


Description
---

`master::Framework` holds 2 member variables of type `Resources`: 
`usedResources` and `offeredResources`. Both of these are aggregates of 
resources from multiple slaves. We add the `hashmap` 
versions since we can lose non-scalar resources by summing them up across 
multiple slaves. For further details refer to 
[MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).

In [r31666](https://reviews.apache.org/r/31666/), we report 
`usedResourcesBySlaveId` rather than `usedResources` when adding the framework 
to the allocator.

We don't actually use `offeredResourcesBySlaveId` currently, but it should be 
kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as 
well as `offeredResourcesBySlaveId` via the HTTP endpoint.


Diffs
-

  src/master/master.hpp 4a1df58e6756bff7d5f292c0542023a9215e33d8 

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


Testing
---

make check


Thanks,

Michael Park

Re: Review Request 31666: Sent 'framework->usedResources' to allocator instead for 'addFramework'.

2015-04-17 Thread Michael Park


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

(Updated April 17, 2015, 5:32 a.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Summary (updated)
-

Sent 'framework->usedResources' to allocator instead for 'addFramework'.


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


Repository: mesos


Description (updated)
---

Piped the master through by 
`s/framework->totalUsedResources/framework->usedResources/`
Lots of `s/Resources()/hashmap()/` in 
`src/tests/hierarchical_allocator_tests.cpp`.


Diffs (updated)
-

  src/master/allocator/allocator.hpp 91f80501aa9bc733fd53f9cb1ac0f15949a74964 
  src/master/allocator/mesos/allocator.hpp 
fb898f1175b61b442204e6e38c69ccc2838a646f 
  src/master/allocator/mesos/hierarchical.hpp 
9f9a631ee52a3ab194e678f9be139e8dabc7f3db 
  src/master/master.cpp 02f35ac2d98d288bf9d2b9195770b839bbfe6d7a 
  src/tests/hierarchical_allocator_tests.cpp 
8861bf398e4bb17b0f74eab4f4af26202447ccef 
  src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4 

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


Testing
---

make check


Thanks,

Michael Park

Re: Review Request 31667: Piped hashmap from allocator through to sorter.

2015-04-17 Thread Michael Park


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

(Updated April 17, 2015, 4:57 a.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Addressed AlexR's comments.


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


Repository: mesos


Description
---

`Sorter` changes:

- Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with 
`SlaveID`.
- `allocation` returns `hashmap`.

`DRFSorter` changes:

- `allocations` is updated from `hashmap` to 
`hashmap>`.
- `resources` is updated from `Resources` to `hashmap`.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
9f9a631ee52a3ab194e678f9be139e8dabc7f3db 
  src/master/allocator/sorter/drf/sorter.hpp 
4366710d6530b784aa5094813328d0e338239ba0 
  src/master/allocator/sorter/drf/sorter.cpp 
2f69f384b95ff20d3ee429a4570a8cffa74d8e8b 
  src/master/allocator/sorter/sorter.hpp 
e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b 
  src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd 

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


Testing
---

make check


Thanks,

Michael Park

Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

2015-04-17 Thread Michael Park



> On April 16, 2015, 7:27 p.m., Ben Mahler wrote:
> > src/master/master.hpp, line 1071
> > 
> >
> > For example, could we document in NOTE on these total variables that 
> > subtraction is safe even though non-scalars lose information?
> > 
> > Specifically, is my understanding correct here: 
> > 
> > (1) For overlapping set items / ranges across slaves, these will get 
> > added N times but only represented 1 time in the Resources. 
> > (2) So, when an initial subtraction occurs (N-1), the resource is no 
> > longer represented. We don't expose non-scalars for now, so this is safe 
> > for now.
> > (3) When any further subtractions occur (N-(1+M)), the Resources simply 
> > ignores the subtraction since there's nothing to remove, so this is safe 
> > for now.
> 
> Michael Park wrote:
> I think this was meant as a subsequent comment to the one above, so I'll 
> answer here.
> 
> I'm not sure if I correctly understand your definition of __safety__ and 
> what's considered a __bug__.
> > (1) For overlapping set items / ranges across slaves, these will get 
> added N times but only represented 1 time in the Resources.
> 
> Correct.
> > (2) So, when an initial subtraction occurs (N-1), the resource is no 
> longer represented.
> 
> Correct.
> > We don't expose non-scalars for now, so this is safe for now.
> 
> We actually do expose non-scalars through the master HTTP endpoint. We 
> used to only expose `cpus`, `mem`, `disk`, and `ports` but since 
> [r31082](https://reviews.apache.org/r/31082) we expose all resources. But 
> even before, `ports` was reported incorrectly. If we mean __safe__ as in we 
> don't crash, then yes it's safe. But I would probably consider this to be at 
> least a __bug__.
> > (3) When any further subtractions occur (N-(1+M)), the Resources simply 
> ignores the subtraction since there's nothing to remove, so this is safe for 
> now.
> 
> Correct.
> 
> After the sequence of patches land, the totals (used and offered) are 
> merely used to be exposed via `state.json`. The non-scalars are still 
> incorrect, but I'm not sure that we can assume that nobody's using them. If 
> we decide to only expose the scalar resources since the non-scalar 
> information was wrong anyway, I think that should definitely be done in a 
> separate patch at least to keep the commits clean (as I mentioned, MESOS-2623 
> will track that). Furthermore, I think changing what resources get exposed 
> via `state.json` is considered an API change and therefore is probably 
> worthwhile mentioning in the `CHANGELOG` for the next release.

I'll add a `NOTE` for why keeping the totals is __safe__ followed by a `TODO` 
to remove the non-scalars to avoid reporting incorrect totals.


- Michael


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


On April 15, 2015, 10:45 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> ---
> 
> (Updated April 15, 2015, 10:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `master::Framework` holds 2 member variables of type `Resources`: 
> `usedResources` and `offeredResources`. Both of these are aggregates of 
> resources from multiple slaves. We add the `hashmap` 
> versions since we can lose non-scalar resources by summing them up across 
> multiple slaves. For further details refer to 
> [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).
> 
> In [r31666](https://reviews.apache.org/r/31666/), we report 
> `usedResourcesBySlaveId` rather than `usedResources` when adding the 
> framework to the allocator.
> 
> We don't actually use `offeredResourcesBySlaveId` currently, but it should be 
> kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` 
> as well as `offeredResourcesBySlaveId` via the HTTP endpoint.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 31666: Sent 'framework->usedResourcesBySlaveId' to allocator instead for 'addFramework'.

2015-04-17 Thread Michael Park



> On April 15, 2015, 7:35 p.m., Ben Mahler wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 313
> > 
> >
> > Should there be a TODO here related to what needs to be done?

Added a TODO.


- Michael


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


On April 17, 2015, 4:31 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31666/
> ---
> 
> (Updated April 17, 2015, 4:31 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Piped the master through by 
> `s/framework->usedResources/framework->usedResourcesBySlaveId/`
> Lots of `s/Resources()/hashmap()/` in 
> `src/tests/hierarchical_allocator_tests.cpp`.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/allocator.hpp 91f80501aa9bc733fd53f9cb1ac0f15949a74964 
>   src/master/allocator/mesos/allocator.hpp 
> fb898f1175b61b442204e6e38c69ccc2838a646f 
>   src/master/allocator/mesos/hierarchical.hpp 
> 9f9a631ee52a3ab194e678f9be139e8dabc7f3db 
>   src/master/master.cpp 02f35ac2d98d288bf9d2b9195770b839bbfe6d7a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 8861bf398e4bb17b0f74eab4f4af26202447ccef 
>   src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4 
> 
> Diff: https://reviews.apache.org/r/31666/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 31666: Sent 'framework->usedResourcesBySlaveId' to allocator instead for 'addFramework'.

2015-04-17 Thread Michael Park


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

(Updated April 17, 2015, 4:31 a.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Added a `TODO` as per `bmahler`'s comment.


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


Repository: mesos


Description
---

Piped the master through by 
`s/framework->usedResources/framework->usedResourcesBySlaveId/`
Lots of `s/Resources()/hashmap()/` in 
`src/tests/hierarchical_allocator_tests.cpp`.


Diffs (updated)
-

  src/master/allocator/allocator.hpp 91f80501aa9bc733fd53f9cb1ac0f15949a74964 
  src/master/allocator/mesos/allocator.hpp 
fb898f1175b61b442204e6e38c69ccc2838a646f 
  src/master/allocator/mesos/hierarchical.hpp 
9f9a631ee52a3ab194e678f9be139e8dabc7f3db 
  src/master/master.cpp 02f35ac2d98d288bf9d2b9195770b839bbfe6d7a 
  src/tests/hierarchical_allocator_tests.cpp 
8861bf398e4bb17b0f74eab4f4af26202447ccef 
  src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4 

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


Testing
---

make check


Thanks,

Michael Park

Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

2015-04-17 Thread Michael Park



> On April 15, 2015, 7:20 p.m., Ben Mahler wrote:
> > src/master/master.hpp, lines 1138-1143
> > 
> >
> > Thanks for this NOTE! We might want to take this opportunity to 
> > articulate why we can safely keep the totals. Or, should we strip 
> > non-scalars and store a 'totalUsedScalarResources'?
> 
> Michael Park wrote:
> We can't do the former since we can't safely keep the totals. We might 
> want to do something like the latter but I think we may break some people 
> that way, and therefore should be done as a subsequent task. I've created 
> [MESOS-2623](https://issues.apache.org/jira/browse/MESOS-2623) to track this.
> 
> Ben Mahler wrote:
> > We can't do the former since we can't safely keep the totals.
> 
> Ah ok great, can we articulate why for posterity? It's not immediately 
> obvious to me. As in, is there a bug due to this? Or is it just that the 
> non-scalar information is not correct, and there's not yet a bug due to this 
> since we only expose scalars? Something else?

Answered to your comment below. In short, if you mean __safe__ as in we don't 
crash then yes it's safe. There __is__ a bug due to this since the non-scalar 
information is not correct and we do expose non-scalar resources.


- Michael


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


On April 15, 2015, 10:45 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> ---
> 
> (Updated April 15, 2015, 10:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `master::Framework` holds 2 member variables of type `Resources`: 
> `usedResources` and `offeredResources`. Both of these are aggregates of 
> resources from multiple slaves. We add the `hashmap` 
> versions since we can lose non-scalar resources by summing them up across 
> multiple slaves. For further details refer to 
> [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).
> 
> In [r31666](https://reviews.apache.org/r/31666/), we report 
> `usedResourcesBySlaveId` rather than `usedResources` when adding the 
> framework to the allocator.
> 
> We don't actually use `offeredResourcesBySlaveId` currently, but it should be 
> kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` 
> as well as `offeredResourcesBySlaveId` via the HTTP endpoint.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 31667: Piped hashmap from allocator through to sorter.

2015-04-17 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [31183, 31664, 31665, 31666, 31667]

All tests passed.

- Mesos ReviewBot


On April 17, 2015, 4:57 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31667/
> ---
> 
> (Updated April 17, 2015, 4:57 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `Sorter` changes:
> 
> - Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with 
> `SlaveID`.
> - `allocation` returns `hashmap`.
> 
> `DRFSorter` changes:
> 
> - `allocations` is updated from `hashmap` to 
> `hashmap>`.
> - `resources` is updated from `Resources` to `hashmap`.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 9f9a631ee52a3ab194e678f9be139e8dabc7f3db 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 4366710d6530b784aa5094813328d0e338239ba0 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b 
>   src/master/allocator/sorter/sorter.hpp 
> e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b 
>   src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd 
> 
> Diff: https://reviews.apache.org/r/31667/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 31665: Changed '(used|offered)Resources' to 'total(Used|Offered)Resources' and added 'hashmap (used|offered)Resources' to 'master::Framework'

2015-04-17 Thread Michael Park


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

(Updated April 17, 2015, 5:31 a.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

(1) `s/usedResources/totalUsedResources/`
(2) `s/offeredResources/totalOfferedResources/`
(3) `s/usedResourcesBySlaveId/usedResources/`
(4) `s/offeredResourcesBySlaveId/offeredResources/`
(5) Added a `NOTE` regarding why keeping totals is safe followed by a `TODO` to 
address [MESOS-2623](https://issues.apache.org/jira/browse/MESOS-2373).
(6) Added a `TODO` for the call to `allocator->addFramework` for what needs to 
be done in the next step.


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


Repository: mesos


Description
---

`master::Framework` holds 2 member variables of type `Resources`: 
`usedResources` and `offeredResources`. Both of these are aggregates of 
resources from multiple slaves. We add the `hashmap` 
versions since we can lose non-scalar resources by summing them up across 
multiple slaves. For further details refer to 
[MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).

In [r31666](https://reviews.apache.org/r/31666/), we report 
`usedResourcesBySlaveId` rather than `usedResources` when adding the framework 
to the allocator.

We don't actually use `offeredResourcesBySlaveId` currently, but it should be 
kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as 
well as `offeredResourcesBySlaveId` via the HTTP endpoint.


Diffs (updated)
-

  src/master/http.cpp f2b123d7f84eafc792849ad4630c2e3b63be000b 
  src/master/master.hpp 4a1df58e6756bff7d5f292c0542023a9215e33d8 
  src/master/master.cpp 02f35ac2d98d288bf9d2b9195770b839bbfe6d7a 

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


Testing
---

make check


Thanks,

Michael Park

Re: Review Request 31665: Changed '(used|offered)Resources' to 'total(Used|Offered)Resources' and added 'hashmap (used|offered)Resources' in 'master::Framework'

2015-04-17 Thread Michael Park


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

(Updated April 17, 2015, 5:22 a.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Summary (updated)
-

Changed '(used|offered)Resources' to 'total(Used|Offered)Resources' and added 
'hashmap (used|offered)Resources' in 'master::Framework'


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


Repository: mesos


Description
---

`master::Framework` holds 2 member variables of type `Resources`: 
`usedResources` and `offeredResources`. Both of these are aggregates of 
resources from multiple slaves. We add the `hashmap` 
versions since we can lose non-scalar resources by summing them up across 
multiple slaves. For further details refer to 
[MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).

In [r31666](https://reviews.apache.org/r/31666/), we report 
`usedResourcesBySlaveId` rather than `usedResources` when adding the framework 
to the allocator.

We don't actually use `offeredResourcesBySlaveId` currently, but it should be 
kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as 
well as `offeredResourcesBySlaveId` via the HTTP endpoint.


Diffs
-

  src/master/master.hpp 4a1df58e6756bff7d5f292c0542023a9215e33d8 

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


Testing
---

make check


Thanks,

Michael Park

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-04-17 Thread Jay Buffington



> On April 16, 2015, 5:34 p.m., Timothy Chen wrote:
> > src/tests/containerizer.cpp, line 95
> > 
> >
> > How about setting a flag in the TestContainerizer constructor that 
> > tells it to send a failure on launch? Checking value like is too implicit, 
> > and someone else might not know the magic meaning on the other side (as I 
> > don't see any comment in the test to mark that this has sepecial meaning).

Are there other examples of inducing failure other places in the test suite?


- Jay


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


On April 16, 2015, 3:16 p.m., Jay Buffington wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33249/
> ---
> 
> (Updated April 16, 2015, 3:16 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Timothy Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-2020
> https://issues.apache.org/jira/browse/MESOS-2020
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When mesos is unable to launch the containerizer the scheduler should
> get a TASK_FAILED with a status message that includes the error the
> containerizer encounted when trying to launch.
> 
> Introduces a new TaskStatus: REASON_CONTAINERIZER_LAUNCH_FAILED
> 
> Fixes MESOS-2020
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3a8e8bf303e0576c212951f6028af77e54d93537 
>   src/slave/slave.cpp a0595f93ce4720f5b9926326d01210460ccb0667 
>   src/tests/containerizer.cpp 26b87ac6b16dfeaf84888e80296ef540697bd775 
>   src/tests/slave_tests.cpp b826000e0a4221690f956ea51f49ad4c99d5e188 
> 
> Diff: https://reviews.apache.org/r/33249/diff/
> 
> 
> Testing
> ---
> 
> I added test case to slave_test.cpp.  I also tried this with Aurora, supplied 
> a bogus docker image url and saw the "docker pull" failure stderr message in 
> Aurora's web UI.
> 
> 
> Thanks,
> 
> Jay Buffington
> 
>

70 matches

Mail list logo