date:20150904


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


Patch looks great!

Reviews applied: [38003]

All tests passed.

- Mesos ReviewBot


On Sept. 5, 2015, 2:46 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38003/
> ---
> 
> (Updated Sept. 5, 2015, 2:46 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3351
> https://issues.apache.org/jira/browse/MESOS-3351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon:__
> In some race condition, the slave was shutdown when after master failover.
> 
> __Root Cause:__
> The slave was shutdown because of duplicated SlavID: in master, the SlaveID 
> is genereated by masterInfo.id + "-S" + nextSlaveId; when master failover, 
> nextSlaveId was reset to 0 and masterInfo.id (generated by date + ip + port + 
> pid) maybe un-changed which lead to duplicated SlaveID. 
> 
> __Solution/Fix:__
> Generate masterInfo.id by UUID instead of "date + ip + port + pid".
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 5589eca 
>   src/tests/master_tests.cpp 8a6b98b 
> 
> Diff: https://reviews.apache.org/r/38003/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38143: Adding executor HTTP API protobuf to V1


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



include/mesos/v1/executor/executor.proto (line 47)


I think that the framework id need to be set when call "SUBSCRIBE" but not 
after "SUBSCRIBED"?



include/mesos/v1/executor/executor.proto (line 55)


s/successfuly/successfully



include/mesos/v1/executor/executor.proto (line 91)


s/arojas/yourid?



include/mesos/v1/executor/executor.proto (line 169)


Do you mean that when type is Subscribe, there is no need to set any 
message? Why? I think that the framework id, executor id, command should be set?


- Guangya Liu


On 九月 5, 2015, 2:35 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated 九月 5, 2015, 2:35 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 5fdca0f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 37531: MESOS-3070 (Master CHECK failure if a framework uses duplicated task id)


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

(Updated Sept. 5, 2015, 3:27 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Add summary & description


Summary (updated)
-

MESOS-3070 (Master CHECK failure if a framework uses duplicated task id)


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


Repository: mesos


Description (updated)
---

__Phenomenon:__
The master crash because of duplicated task id

__Root Cause:__
The task id are stored in slave agent; if master failover, there's a time 
window that new slave lanched a task with same task id; so if the old task 
re-registered back, the master will crash because of duplicated task id.

__Solution:__
Stores tasks info in Master::Framework by SlaveID to avoid duplicated issue.


Diffs
-

  src/master/http.cpp 37d76ee 
  src/master/master.hpp 36c6759 
  src/master/master.cpp 95207d2 
  src/tests/master_tests.cpp 8a6b98b 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 38143: Adding executor HTTP API protobuf to V1


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


Patch looks great!

Reviews applied: [38143]

All tests passed.

- Mesos ReviewBot


On Sept. 5, 2015, 2:35 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated Sept. 5, 2015, 2:35 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 5fdca0f 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)


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

(Updated Sept. 5, 2015, 3:14 a.m.)


Review request for mesos and Michael Park.


Changes
---

Update summary & description


Summary (updated)
-

MESOS-3063 (Add an example framework using dynamic reservation)


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


Repository: mesos


Description (updated)
---

Provide example for dynamic reservation features.


Diffs
-

  src/Makefile.am 7b620ff 
  src/examples/dynamic_reservation_executor.cpp PRE-CREATION 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 3f56b30 
  src/tests/flags.hpp 3644956 
  src/tests/script.cpp bcc1fab 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 38102: MESOS-3046 (Stout's UUID re-seeds a new random generator during each call to UUID::random)


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

(Updated Sept. 5, 2015, 3:08 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Update summary & description


Summary (updated)
-

MESOS-3046 (Stout's UUID re-seeds a new random generator during each call to 
UUID::random)


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


Repository: mesos


Description (updated)
---

__Phenomenon:__
Performance downgrade

__Root Cause:__
stout's UUID abstraction is re-seeding the random generator during each call to 
UUID::random(), which is really expensive.

__Solution:__
Seeding the random generator only once per thread.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp 
e90dabb0c572923a50490ecb17867dc50c6d161d 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 38003: MESOS-3351 (duplicated slave id in master after master failover)



> On Sept. 4, 2015, 7:35 p.m., Vinod Kone wrote:
> >
> 
> Vinod Kone wrote:
> Also, please make the summary and description more meaningful than just 
> the ticket ID.

Yes, both summary & description are updated for this fix


> On Sept. 4, 2015, 7:35 p.m., Vinod Kone wrote:
> > src/master/master.cpp, lines 306-317
> > 
> >
> > Just do this.
> > 
> > ```
> > 
> > // Master ID is generated randomly based on UUID.
> > info_.set_id(UUID::random().toString());
> > 
> > ```

addressed


> On Sept. 4, 2015, 7:35 p.m., Vinod Kone wrote:
> > src/tests/master_tests.cpp, line 3607
> > 
> >
> > All our comments are expected to be proper sentences, i.e., start with 
> > a capital letter and end with period. Please fix here and everywhere.

addressed


> On Sept. 4, 2015, 7:35 p.m., Vinod Kone wrote:
> > src/tests/master_tests.cpp, lines 3638-3666
> > 
> >
> > Why do you need to launch a scheduler and task for this test?
> > 
> > I think you can simplify this test by not launching them.

Agree, scheduler & tasks are not necessary, both of them are removed.


- Klaus


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


On Sept. 5, 2015, 2:46 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38003/
> ---
> 
> (Updated Sept. 5, 2015, 2:46 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3351
> https://issues.apache.org/jira/browse/MESOS-3351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon:__
> In some race condition, the slave was shutdown when after master failover.
> 
> __Root Cause:__
> The slave was shutdown because of duplicated SlavID: in master, the SlaveID 
> is genereated by masterInfo.id + "-S" + nextSlaveId; when master failover, 
> nextSlaveId was reset to 0 and masterInfo.id (generated by date + ip + port + 
> pid) maybe un-changed which lead to duplicated SlaveID. 
> 
> __Solution/Fix:__
> Generate masterInfo.id by UUID instead of "date + ip + port + pid".
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 5589eca 
>   src/tests/master_tests.cpp 8a6b98b 
> 
> Diff: https://reviews.apache.org/r/38003/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38003: MESOS-3351 (duplicated slave id in master after master failover)


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

(Updated Sept. 5, 2015, 2:46 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Address Vinod's comments


Summary (updated)
-

MESOS-3351 (duplicated slave id in master after master failover)


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


Repository: mesos


Description (updated)
---

__Phenomenon:__
In some race condition, the slave was shutdown when after master failover.

__Root Cause:__
The slave was shutdown because of duplicated SlavID: in master, the SlaveID is 
genereated by masterInfo.id + "-S" + nextSlaveId; when master failover, 
nextSlaveId was reset to 0 and masterInfo.id (generated by date + ip + port + 
pid) maybe un-changed which lead to duplicated SlaveID. 

__Solution/Fix:__
Generate masterInfo.id by UUID instead of "date + ip + port + pid".


Diffs (updated)
-

  src/master/master.cpp 5589eca 
  src/tests/master_tests.cpp 8a6b98b 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Review Request 38143: Adding executor HTTP API protobuf to V1

2015-09-04 Thread Isabel Jimenez


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

Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
ought to be changed as the executor HTTP API design evolves.


Diffs
-

  include/mesos/v1/executor/executor.hpp PRE-CREATION 
  include/mesos/v1/executor/executor.proto PRE-CREATION 
  src/Makefile.am 5fdca0f 

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


Testing
---

make && make check


Thanks,

Isabel Jimenez

Re: Review Request 38074: Calculate schedule latency with trace events


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



include/mesos/mesos.proto (line 915)


Can you please add some commentes here to declear what does p50, p90, p95, 
p99 mean? Thanks.



src/slave/flags.cpp (line 306)


Interval between which two values?



src/slave/flags.cpp (line 307)


Does this also affect the event, so what is the difference of this flag and 
sched_sample_duration?


- Guangya Liu


On 九月 4, 2015, 11:16 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38074/
> ---
> 
> (Updated 九月 4, 2015, 11:16 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Finally, calculate schedule latency with the sched trace events, and add it 
> to our statistics
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/slave/containerizer/isolators/cgroups/perf_event.hpp 
> 1f722ef3ef7ab7fce5542d4affae961d6cec2406 
>   src/slave/containerizer/isolators/cgroups/perf_event.cpp 
> 7dc8b7a99074b74ade019ef4df296780650a2e4e 
>   src/slave/flags.hpp b8335aa585376d27b71897f8cbaefaa12f4b3a5c 
>   src/slave/flags.cpp 7539441c685828027db07173e62a4e5fc1e8b54d 
> 
> Diff: https://reviews.apache.org/r/38074/diff/
> 
> 
> Testing
> ---
> 
> manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38141: Refactor shared paths in provisioners.


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


Bad patch!

Reviews applied: [38141]

Failed command: make -j3 distcheck

Error:
 make  dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
if test -d "mesos-0.25.0"; then find "mesos-0.25.0" -type d ! -perm -200 -exec 
chmod u+w {} ';' && rm -rf "mesos-0.25.0" || { sleep 5 && rm -rf 
"mesos-0.25.0"; }; else :; fi
test -d "mesos-0.25.0" || mkdir "mesos-0.25.0"
 (cd 3rdparty && make  top_distdir=../mesos-0.25.0 
distdir=../mesos-0.25.0/3rdparty \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
 (cd libprocess && make  top_distdir=../../mesos-0.25.0 
distdir=../../mesos-0.25.0/3rdparty/libprocess \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[3]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
:
test -d "../../mesos-0.25.0/3rdparty/libprocess" || mkdir 
"../../mesos-0.25.0/3rdparty/libprocess"
 (cd 3rdparty && make  top_distdir=../../../mesos-0.25.0 
distdir=../../../mesos-0.25.0/3rdparty/libprocess/3rdparty \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[4]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
 (cd stout && make  top_distdir=../../../../mesos-0.25.0 
distdir=../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[5]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
:
test -d "../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout" || mkdir 
"../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout"
 (cd include && make  top_distdir=../../../../../mesos-0.25.0 
distdir=../../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout/include \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[6]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
make[6]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
test -n ":" \
|| find "../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout" 
-type d ! -perm -755 \
-exec chmod u+rwx,go+rx {} \; -o \
  ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -444 -exec /bin/bash 
/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/install-sh
 -c -m a+r {} {} \; \
|| chmod -R a+r 
"../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout"
make[5]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
make[4]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
 (cd include && make  top_distdir=../../../mesos-0.25.0 
distdir=../../../mesos-0.25.0/3rdparty/libprocess/include \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[4]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
make[4]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
test -n ":" \
|| find "../../mesos-0.25.0/3rdparty/libprocess" -type d ! -perm -755 \
-exec chmod u+rwx,go+rx {} \; -o \
  ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -444 -exec /bin/bash 
/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/install-sh
 -c -m a+r {} {} \; \
|| chmod -R a+r "../../mesos-0.25.0/3rdparty/libprocess"
make[3]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
make[2]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
 (cd src && make  top_distdir=../mesos-0.25.0 distdir=../mesos-0.25.0/src \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
make[2]: *** No rule to make target 
`slave/containerizer/provisioners/path.hpp', needed by `distdir'.  Stop.
make[2]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
make[1]: *** [distdir] Error 1
make[1]: Leaving directory 
`/home/jenkins/jen

Review Request 38141: Refactor shared paths in provisioners.


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

Review request for mesos and Jiang Yan Xu.


Repository: mesos


Description
---

Refactor shared paths in provisioners.


Diffs
-

  src/Makefile.am 5fdca0f574e7e08c4b1aebed0fac39140c19adfe 
  src/slave/containerizer/provisioners/appc.cpp 
fc5ee19df51a3543aaf01c2301b976700610ff57 
  src/slave/containerizer/provisioners/appc/paths.hpp 
fb3a1a7295809d745dd15bee6db1f7e8dd99ab33 
  src/slave/containerizer/provisioners/appc/paths.cpp 
e6be851e24886d7c886adad4c7ea29ded17bdcbe 
  src/slave/containerizer/provisioners/paths.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/paths.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 37873: Add quiesce logic in allocator

2015-09-04 Thread Robert Lacroix


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

Ship it!


Ship It!

- Robert Lacroix


On Sept. 4, 2015, 12:06 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37873/
> ---
> 
> (Updated Sept. 4, 2015, 12:06 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add quiesce logic in allocator
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> cb4020dea897ef198cd9898cabecf61edfade834 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
> 
> Diff: https://reviews.apache.org/r/37873/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38137: Docker provisioner and local store


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

(Updated Sept. 5, 2015, 12:20 a.m.)


Review request for mesos, Jojy Varghese, Till Toenshoff, and Jiang Yan Xu.


Repository: mesos


Description
---

Joining all the commits around provisioner and local store into one review so 
it's easier to review, as patches
are changing code on top of each other.

All the commits are going to committed together.


Diffs (updated)
-

  src/Makefile.am 5fdca0f 
  src/messages/docker_provisioner.hpp PRE-CREATION 
  src/messages/docker_provisioner.proto PRE-CREATION 
  src/slave/containerizer/provisioner.hpp 9e0e0b8 
  src/slave/containerizer/provisioner.cpp 95894c0 
  src/slave/containerizer/provisioners/docker.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/local_store.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/local_store.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/paths.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/paths.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/reference_store.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/reference_store.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
  src/slave/flags.hpp b8335aa 
  src/slave/flags.cpp 7539441 
  src/tests/containerizer/docker_provisioner_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 37197: Docker image store.



> On Aug. 26, 2015, 5:50 p.m., Till Toenshoff wrote:
> > src/slave/containerizer/provisioners/docker/store.cpp, line 135
> > 
> >
> > I am not a big fan of such magic numbers (referring to that '7'). How 
> > about following the pattern the fetcher is using here 
> > (src/slave/containerizer/fetcher.cpp)?
> > 
> > 
> > ```
> > [...]
> > namespace slave {
> > 
> > static const string FILE_URI_PREFIX = "file://";
> > [...]
> > 
> > if (strings::startsWith(imageUri, FILE_URI_PREFIX)) {
> > imageUri = imageUri.substr(FILE_URI_PREFIX.size());
> > }
> > ```
> 
> Till Toenshoff wrote:
> Why exactly did you close this without fixing the issue or giving some 
> more insights on why you insist on magic numbers?

Sorry I dropped it because this code is removed later on. I opened up a new 
review that holds everything now.


- Timothy


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


On Aug. 27, 2015, 11:39 p.m., Lily Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37197/
> ---
> 
> (Updated Aug. 27, 2015, 11:39 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Till Toenshoff, 
> and Timothy Chen.
> 
> 
> Bugs: MESOS-2849
> https://issues.apache.org/jira/browse/MESOS-2849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stored images currently kept indefinitely.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION 
>   src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
>   src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 
> 
> Diff: https://reviews.apache.org/r/37197/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Lily Chen
> 
>

Re: Review Request 37197: Docker image store.



> On Sept. 3, 2015, 6:59 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/provisioners/docker/store.hpp, line 100
> > 
> >
> > Does this need to be in the hpp file?

It is in the latest review.


> On Sept. 3, 2015, 6:59 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/provisioners/docker/store.hpp, line 146
> > 
> >
> > I would rethink the lifecycle management of this poiner.

We should rethink to fix this from the provisioner interface.


> On Sept. 3, 2015, 6:59 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/provisioners/docker/store.cpp, line 49
> > 
> >
> > Ususally if you want to provide a factory class that hands out 
> > implementations, you use Factory class (commonly known as Factory design 
> > pattern).

In this case Store is providing this factory as we have multple implemetations 
later on.


- Timothy


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


On Aug. 27, 2015, 11:39 p.m., Lily Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37197/
> ---
> 
> (Updated Aug. 27, 2015, 11:39 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Till Toenshoff, 
> and Timothy Chen.
> 
> 
> Bugs: MESOS-2849
> https://issues.apache.org/jira/browse/MESOS-2849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stored images currently kept indefinitely.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION 
>   src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
>   src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 
> 
> Diff: https://reviews.apache.org/r/37197/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Lily Chen
> 
>

Re: Review Request 38074: Calculate schedule latency with trace events


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


Patch looks great!

Reviews applied: [37540, 37541, 38074]

All tests passed.

- Mesos ReviewBot


On Sept. 4, 2015, 11:16 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38074/
> ---
> 
> (Updated Sept. 4, 2015, 11:16 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Finally, calculate schedule latency with the sched trace events, and add it 
> to our statistics
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/slave/containerizer/isolators/cgroups/perf_event.hpp 
> 1f722ef3ef7ab7fce5542d4affae961d6cec2406 
>   src/slave/containerizer/isolators/cgroups/perf_event.cpp 
> 7dc8b7a99074b74ade019ef4df296780650a2e4e 
>   src/slave/flags.hpp b8335aa585376d27b71897f8cbaefaa12f4b3a5c 
>   src/slave/flags.cpp 7539441c685828027db07173e62a4e5fc1e8b54d 
> 
> Diff: https://reviews.apache.org/r/38074/diff/
> 
> 
> Testing
> ---
> 
> manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Review Request 38137: Docker provisioner and local store


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

Review request for mesos, Jojy Varghese, Till Toenshoff, and Jiang Yan Xu.


Repository: mesos


Description
---

Joining all the commits around provisioner and local store into one review so 
it's easier to review, as patches
are changing code on top of each other.

All the commits are going to committed together.


Diffs
-

  src/Makefile.am 5fdca0f 
  src/messages/docker_provisioner.hpp PRE-CREATION 
  src/messages/docker_provisioner.proto PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/linux.cpp 0970b3d 
  src/slave/containerizer/provisioner.hpp 9e0e0b8 
  src/slave/containerizer/provisioner.cpp 95894c0 
  src/slave/containerizer/provisioners/docker.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/local_store.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/local_store.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/paths.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/paths.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/reference_store.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/reference_store.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
  src/slave/flags.hpp b8335aa 
  src/slave/flags.cpp 7539441 
  src/tests/containerizer/docker_provisioner_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 37532: Add QUIESCE call interface to the scheduler


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

(Updated 九月 5, 2015, 12:01 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This is just part of MESOS-3037, this patch only add the interface
of QUIESCE call.


Diffs (updated)
-

  include/mesos/scheduler.hpp ee198b6955882f4f31466ca05429ca16fbf2f5cd 
  include/mesos/scheduler/scheduler.proto 
89daf8a6b74057ee156b3ad691397e76fcb835b8 
  src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6 
  src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
  src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 37532: Add QUIESCE call interface to the scheduler



> On 九月 4, 2015, 7:49 p.m., Vinod Kone wrote:
> > this review blocks a bunch of reviews? can you make the dependency linear? 
> > it would make my life easier to follow the chain of reviews.

Thanks Vinod, both 37867 and 37866 are now discared but I do not know how to 
remove them from depenency list, can you help?


- Guangya


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


On 九月 5, 2015, 12:01 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37532/
> ---
> 
> (Updated 九月 5, 2015, 12:01 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is just part of MESOS-3037, this patch only add the interface
> of QUIESCE call.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp ee198b6955882f4f31466ca05429ca16fbf2f5cd 
>   include/mesos/scheduler/scheduler.proto 
> 89daf8a6b74057ee156b3ad691397e76fcb835b8 
>   src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 
> 
> Diff: https://reviews.apache.org/r/37532/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.


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

(Updated Sept. 4, 2015, 11:20 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris 
Van Remoortere, and Vinod Kone.


Changes
---

Rebased. NNFR.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e 
  src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6 
  src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
  src/tests/master_validation_tests.cpp 
3513bca6fd6773f712d1a647b1757766dc34f948 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

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

(Updated Sept. 4, 2015, 11:19 p.m.)

Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris
Van Remoortere, and Vinod Kone.

Changes
---

Rebased. NNFR.

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

Repository: mesos

Description
---

This involved a lot more challenges than I anticipated, I've captured the
various approaches and limitations and deal-breakers of those approaches here:
[Master Endpoint Implementation
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
(1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
(2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
(3) `updateAvailable` never leaves the allocator in an over-allocated state
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like
"available" resources will be gone.
This is due to the race between the allocator scheduling an `allocate`
call to itself vs master's
`allocator->updateAvailable` invocation.
As such, we first try to satisfy the request only with the offered
resources.
* We greedily rescind one offer at a time until we've rescinded
sufficiently many offers.
IMPORTANT: We perform `recoverResources(..., Filters())` which has a
default `refuse_sec` of 5 seconds,
rather than `recoverResources(..., None())` so that we can virtually
always win the race against `allocate`.
In the rare case that we do lose, no disaster occurs. We simply fail to
satisfy the request.
* If we still don't have enough resources after resciding all offers, be
semi-optimistic and forward the
request to the allocator since there may be available resources to
satisfy the request.
* If the allocator returns a failure, report the error to the user with
`Conflict`.

This approach is clearly not ideal, since we would prefer to rescind as little
offers as possible.

Diffs (updated)
-

src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e
src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6
src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac
src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a
src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5

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

Testing
---

`make check`

Thanks,

Michael Park

Re: Review Request 38074: Calculate schedule latency with trace events


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

(Updated Sept. 4, 2015, 11:16 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Rebase


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


Repository: mesos


Description
---

Finally, calculate schedule latency with the sched trace events, and add it to 
our statistics


Diffs (updated)
-

  include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
  src/slave/containerizer/isolators/cgroups/perf_event.hpp 
1f722ef3ef7ab7fce5542d4affae961d6cec2406 
  src/slave/containerizer/isolators/cgroups/perf_event.cpp 
7dc8b7a99074b74ade019ef4df296780650a2e4e 
  src/slave/flags.hpp b8335aa585376d27b71897f8cbaefaa12f4b3a5c 
  src/slave/flags.cpp 7539441c685828027db07173e62a4e5fc1e8b54d 

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


Testing
---

manual tests


Thanks,

Cong Wang

Re: Review Request 37541: Add trace event API


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

(Updated Sept. 4, 2015, 11:15 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Rebase and cleanup


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


Repository: mesos


Description
---

Based on the PerfEvent API's, add API for Linux kernel trace events, especially 
the schedule trace events.


Diffs (updated)
-

  src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
  src/linux/perf.cpp 0011482cf9d920485728798518d32af0e9627724 
  src/tests/containerizer/cgroups_tests.cpp 
75a3bc0009c037dc18ce319db2eb44630f083e8c 
  src/tests/containerizer/perf_tests.cpp 
bef475e4b573f02b69f2c4dd6d1c6a6e10f34c38 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 37540: Add perf event API


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

(Updated Sept. 4, 2015, 11:13 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Address review comments


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


Repository: mesos


Description
---

Abstract Linux kernel perf event API and provide API to collect schedule events.


Diffs (updated)
-

  src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
  src/linux/perf.cpp 0011482cf9d920485728798518d32af0e9627724 
  src/tests/containerizer/perf_tests.cpp 
bef475e4b573f02b69f2c4dd6d1c6a6e10f34c38 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 37821: Join threads in libprocess when shutting down.


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


Patch looks great!

Reviews applied: [37821]

All tests passed.

- Mesos ReviewBot


On Sept. 4, 2015, 9 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37821/
> ---
> 
> (Updated Sept. 4, 2015, 9 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and switched to 'mcypark'.
> 
> 
> Bugs: MESOS-3158
> https://issues.apache.org/jira/browse/MESOS-3158
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Join threads in libprocess when shutting down.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/event_loop.hpp 
> 36a4cd2b1ff59f6922173ad17115bf80cc3c8f30 
>   3rdparty/libprocess/src/libev.cpp 97a2694f9b10bc61841443b21f4f96055493e840 
>   3rdparty/libprocess/src/libevent.cpp 
> ee7906470069b0391dde7cd685b1d4eb3a158c03 
>   3rdparty/libprocess/src/process.cpp 
> 0e5394acff16376809918d583d7aee582cc6da54 
> 
> Diff: https://reviews.apache.org/r/37821/diff/
> 
> 
> Testing
> ---
> 
> After configuring with both "../configure" and "../configure 
> --enable-libevent --enable-ssl":
> 
> make check
> 
> 
> Also, to check for race conditions related to the initialization/shutdown of 
> libprocess, try something like:
> 
> for n in {1..1000}; do echo $n; 3rdparty/libprocess/tests 
> --gtest_filter=ProcessTest.Spawn; done
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37540: Add perf event API



> On Sept. 3, 2015, 12:46 a.m., Vinod Kone wrote:
> > src/linux/perf.cpp, line 819
> > 
> >
> > who can discard this future?

finalize().


- Cong


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


On Sept. 2, 2015, 10:16 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37540/
> ---
> 
> (Updated Sept. 2, 2015, 10:16 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Abstract Linux kernel perf event API and provide API to collect schedule 
> events.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp c5630118f4858b1a805f25a2db7a24ca0989 
>   src/linux/perf.cpp bb8b5918e4c8f4379784d4fa18c3b4709696e1ea 
>   src/tests/containerizer/perf_tests.cpp 
> 8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 
> 
> Diff: https://reviews.apache.org/r/37540/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 37540: Add perf event API



> On Sept. 3, 2015, 12:46 a.m., Vinod Kone wrote:
> > src/linux/perf.hpp, line 119
> > 
> >
> > We use "_" prefix for continuation. Since this is just a private 
> > overload, I would remove "_".

Compile failed after removing the "_":

../../3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp: In 
instantiation of 'Option::Option(_Some&&) [with U = 
Try >; T = hashmap]':
../../3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp:53:19:   
required from 'Try::Try(const U&) [with U = 
Try >; T = hashmap]'
../../src/linux/perf.cpp:660:12:   required from here
../../3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp:53:61: error: 
no matching function for call to 'hashmap::hashmap(std::remove_reference
 >&>::type)'
   Option(_Some&& some) : state(SOME), t(std::move(some.t)) {}


- Cong


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


On Sept. 2, 2015, 10:16 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37540/
> ---
> 
> (Updated Sept. 2, 2015, 10:16 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Abstract Linux kernel perf event API and provide API to collect schedule 
> events.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp c5630118f4858b1a805f25a2db7a24ca0989 
>   src/linux/perf.cpp bb8b5918e4c8f4379784d4fa18c3b4709696e1ea 
>   src/tests/containerizer/perf_tests.cpp 
> 8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 
> 
> Diff: https://reviews.apache.org/r/37540/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 37821: Join threads in libprocess when shutting down.



> On Sept. 4, 2015, 8:14 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 2217-2218
> > 
> >
> > Could you change the spacing to:
> > ```
> > threads.emplace_back(
> > new std::thread(&internal::schedule, std::cref(joining_threads)));
> > ```
> > 
> > (I vaguely remember that this was the preference over vertically 
> > aligning arguments.)

Thanks Joseph! Comments addressed.


- Greg


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


On Sept. 4, 2015, 9 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37821/
> ---
> 
> (Updated Sept. 4, 2015, 9 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and switched to 'mcypark'.
> 
> 
> Bugs: MESOS-3158
> https://issues.apache.org/jira/browse/MESOS-3158
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Join threads in libprocess when shutting down.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/event_loop.hpp 
> 36a4cd2b1ff59f6922173ad17115bf80cc3c8f30 
>   3rdparty/libprocess/src/libev.cpp 97a2694f9b10bc61841443b21f4f96055493e840 
>   3rdparty/libprocess/src/libevent.cpp 
> ee7906470069b0391dde7cd685b1d4eb3a158c03 
>   3rdparty/libprocess/src/process.cpp 
> 0e5394acff16376809918d583d7aee582cc6da54 
> 
> Diff: https://reviews.apache.org/r/37821/diff/
> 
> 
> Testing
> ---
> 
> After configuring with both "../configure" and "../configure 
> --enable-libevent --enable-ssl":
> 
> make check
> 
> 
> Also, to check for race conditions related to the initialization/shutdown of 
> libprocess, try something like:
> 
> for n in {1..1000}; do echo $n; 3rdparty/libprocess/tests 
> --gtest_filter=ProcessTest.Spawn; done
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37821: Join threads in libprocess when shutting down.


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

(Updated Sept. 4, 2015, 9 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van 
Remoortere, Joseph Wu, and switched to 'mcypark'.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Join threads in libprocess when shutting down.


Diffs (updated)
-

  3rdparty/libprocess/src/event_loop.hpp 
36a4cd2b1ff59f6922173ad17115bf80cc3c8f30 
  3rdparty/libprocess/src/libev.cpp 97a2694f9b10bc61841443b21f4f96055493e840 
  3rdparty/libprocess/src/libevent.cpp ee7906470069b0391dde7cd685b1d4eb3a158c03 
  3rdparty/libprocess/src/process.cpp 0e5394acff16376809918d583d7aee582cc6da54 

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


Testing
---

After configuring with both "../configure" and "../configure --enable-libevent 
--enable-ssl":

make check


Also, to check for race conditions related to the initialization/shutdown of 
libprocess, try something like:

for n in {1..1000}; do echo $n; 3rdparty/libprocess/tests 
--gtest_filter=ProcessTest.Spawn; done


Thanks,

Greg Mann

Re: Review Request 37540: Add perf event API



> On Sept. 3, 2015, 12:46 a.m., Vinod Kone wrote:
> > src/linux/perf.hpp, lines 84-89
> > 
> >
> > Do we need this overload considering users can call the overload below?

I think it is still useful, because the return value of the other overload is a 
hashmap, not convenient to use for a single cgroup.


- Cong


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


On Sept. 2, 2015, 10:16 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37540/
> ---
> 
> (Updated Sept. 2, 2015, 10:16 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Abstract Linux kernel perf event API and provide API to collect schedule 
> events.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp c5630118f4858b1a805f25a2db7a24ca0989 
>   src/linux/perf.cpp bb8b5918e4c8f4379784d4fa18c3b4709696e1ea 
>   src/tests/containerizer/perf_tests.cpp 
> 8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 
> 
> Diff: https://reviews.apache.org/r/37540/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 37540: Add perf event API



> On Sept. 3, 2015, 12:46 a.m., Vinod Kone wrote:
> > src/linux/perf.cpp, line 791
> > 
> >
> > why is __read() a separate function? can you just pull the logic down 
> > here?

__read() is used by the next patch.


- Cong


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


On Sept. 2, 2015, 10:16 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37540/
> ---
> 
> (Updated Sept. 2, 2015, 10:16 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Abstract Linux kernel perf event API and provide API to collect schedule 
> events.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp c5630118f4858b1a805f25a2db7a24ca0989 
>   src/linux/perf.cpp bb8b5918e4c8f4379784d4fa18c3b4709696e1ea 
>   src/tests/containerizer/perf_tests.cpp 
> 8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 
> 
> Diff: https://reviews.apache.org/r/37540/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38040: Refactor docker provisioner store to act as read-through cache.


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


Bad patch!

Reviews applied: [37196, 37197]

Failed command: ./support/apply-review.sh -n -r 37197

Error:
 2015-09-04 20:48:22 URL:https://reviews.apache.org/r/37197/diff/raw/ 
[19289/19289] -> "37197.patch" [1]
error: patch failed: src/Makefile.am:743
error: src/Makefile.am: patch does not apply
error: patch failed: src/slave/flags.hpp:49
error: src/slave/flags.hpp: patch does not apply
error: patch failed: src/slave/flags.cpp:69
error: src/slave/flags.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 4, 2015, 8:34 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38040/
> ---
> 
> (Updated Sept. 4, 2015, 8:34 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactor docker provisioner store to act as read-through cache.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioners/docker.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/local_store.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
> 
> Diff: https://reviews.apache.org/r/38040/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38040: Refactor docker provisioner store to act as read-through cache.


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

(Updated Sept. 4, 2015, 8:34 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Repository: mesos


Description
---

Refactor docker provisioner store to act as read-through cache.


Diffs (updated)
-

  src/slave/containerizer/provisioner.hpp 
9e0e0b8ef290a31b67bd2415253408e811e1c720 
  src/slave/containerizer/provisioners/docker.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/local_store.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
  src/tests/containerizer/provisioner.hpp 
a26b8138d8cc3086058b15a797dd15354a84019f 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 37821: Join threads in libprocess when shutting down.

2015-09-04 Thread Joseph Wu


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

Ship it!


Looks good.  Just a few little nits.


3rdparty/libprocess/src/event_loop.hpp (lines 45 - 46)


This fits in one line.



3rdparty/libprocess/src/process.cpp (line 436)


Missing period at the end.



3rdparty/libprocess/src/process.cpp (lines 442 - 443)


Fits on one line, but I think this should be fine (77 characters).



3rdparty/libprocess/src/process.cpp (lines 2144 - 2146)


Can you update this comment to note that the actual "popping" of the 
process is done in `ProcessManager::cleanup`, which can happen as a result of 
`Process::terminate`?



3rdparty/libprocess/src/process.cpp (line 2178)


I think you should tweak this to use backticks instead of single quotes, 
even if it's a copy-pasted comment.



3rdparty/libprocess/src/process.cpp (lines 2191 - 2192)


Also fits on one line (73 characters).



3rdparty/libprocess/src/process.cpp (lines 2193 - 2194)


Could you change the spacing to:
```
threads.emplace_back(
new std::thread(&internal::schedule, std::cref(joining_threads)));
```

(I vaguely remember that this was the preference over vertically aligning 
arguments.)


- Joseph Wu


On Sept. 4, 2015, 11:13 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37821/
> ---
> 
> (Updated Sept. 4, 2015, 11:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and switched to 'mcypark'.
> 
> 
> Bugs: MESOS-3158
> https://issues.apache.org/jira/browse/MESOS-3158
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Join threads in libprocess when shutting down.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/event_loop.hpp 
> 36a4cd2b1ff59f6922173ad17115bf80cc3c8f30 
>   3rdparty/libprocess/src/libev.cpp 97a2694f9b10bc61841443b21f4f96055493e840 
>   3rdparty/libprocess/src/libevent.cpp 
> ee7906470069b0391dde7cd685b1d4eb3a158c03 
>   3rdparty/libprocess/src/process.cpp 
> 755187c8761137cb2bf2f7295b29a63f63c68bc6 
> 
> Diff: https://reviews.apache.org/r/37821/diff/
> 
> 
> Testing
> ---
> 
> After configuring with both "../configure" and "../configure 
> --enable-libevent --enable-ssl":
> 
> make check
> 
> 
> Also, to check for race conditions related to the initialization/shutdown of 
> libprocess, try something like:
> 
> for n in {1..1000}; do echo $n; 3rdparty/libprocess/tests 
> --gtest_filter=ProcessTest.Spawn; done
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37532: Add QUIESCE call interface to the scheduler


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


this review blocks a bunch of reviews? can you make the dependency linear? it 
would make my life easier to follow the chain of reviews.


include/mesos/scheduler/scheduler.proto (line 173)


period at the end.

does this fit within 80 char limit?



src/master/master.cpp (line 2540)


period.



src/master/master.cpp (line 2541)


s/INFO/WARNING/


- Vinod Kone


On Sept. 4, 2015, 12:06 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37532/
> ---
> 
> (Updated Sept. 4, 2015, 12:06 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is just part of MESOS-3037, this patch only add the interface
> of QUIESCE call.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp ee198b6955882f4f31466ca05429ca16fbf2f5cd 
>   include/mesos/scheduler/scheduler.proto 
> 89daf8a6b74057ee156b3ad691397e76fcb835b8 
>   src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 
> 
> Diff: https://reviews.apache.org/r/37532/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.


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



src/master/master.cpp (line 4118)


Instead of not calling updateTask() the logic should be inside updateTask() 
to do the right thing. Note that updateTask() already has logic to deal with 
duplicate terminal updates.


- Vinod Kone


On Sept. 2, 2015, 1:13 p.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 2, 2015, 1:13 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.



> On Sept. 4, 2015, 7:42 p.m., Vinod Kone wrote:
> > src/master/master.cpp, line 4118
> > 
> >
> > Instead of not calling updateTask() the logic should be inside 
> > updateTask() to do the right thing. Note that updateTask() already has 
> > logic to deal with duplicate terminal updates.

Also, needs a test!


- Vinod


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


On Sept. 2, 2015, 1:13 p.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 2, 2015, 1:13 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38003: MESOS-3351



> On Sept. 4, 2015, 7:35 p.m., Vinod Kone wrote:
> >

Also, please make the summary and description more meaningful than just the 
ticket ID.


- Vinod


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


On Sept. 1, 2015, 2:44 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38003/
> ---
> 
> (Updated Sept. 1, 2015, 2:44 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3351
> https://issues.apache.org/jira/browse/MESOS-3351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3351 (duplicated slave id in master after master failover)
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
> 
> Diff: https://reviews.apache.org/r/38003/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38003: MESOS-3351


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



src/master/master.cpp (lines 306 - 312)


Just do this.

```

// Master ID is generated randomly based on UUID.
info_.set_id(UUID::random().toString());

```



src/tests/master_tests.cpp (line 3607)


All our comments are expected to be proper sentences, i.e., start with a 
capital letter and end with period. Please fix here and everywhere.



src/tests/master_tests.cpp (lines 3638 - 3666)


Why do you need to launch a scheduler and task for this test?

I think you can simplify this test by not launching them.


- Vinod Kone


On Sept. 1, 2015, 2:44 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38003/
> ---
> 
> (Updated Sept. 1, 2015, 2:44 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3351
> https://issues.apache.org/jira/browse/MESOS-3351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3351 (duplicated slave id in master after master failover)
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
> 
> Diff: https://reviews.apache.org/r/38003/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37699: Removed remnants of LIBPROCESS_STATISTICS_WINDOW.

2015-09-04 Thread Ben Mahler


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

Ship it!


Ship It!

- Ben Mahler


On Sept. 3, 2015, 3:08 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37699/
> ---
> 
> (Updated Sept. 3, 2015, 3:08 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3304
> https://issues.apache.org/jira/browse/MESOS-3304
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed remnants of LIBPROCESS_STATISTICS_WINDOW.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 755187c8761137cb2bf2f7295b29a63f63c68bc6 
> 
> Diff: https://reviews.apache.org/r/37699/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37586: Generate usage help information in libprocess.


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

(Updated Sept. 4, 2015, 6:53 p.m.)


Review request for mesos, Michael Park and Vinod Kone.


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


Repository: mesos


Description
---

Generate usage help information in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/include/process/help.hpp 
441f6d1c15afebdd98b73bb7430fcd8c8cf1e333 
  3rdparty/libprocess/include/process/system.hpp 
7c8b49e78f76f9e131a4367f411c6dba447ccd90 
  3rdparty/libprocess/src/help.cpp e4e0bb617956c7f79380069015b7170aa7716231 
  3rdparty/libprocess/src/logging.cpp 3d855e90e83a54cd344e49f075af0eadef1a1358 
  3rdparty/libprocess/src/metrics/metrics.cpp 
b9617507a16318b7de25d4875d6bc0b4409fcd29 
  3rdparty/libprocess/src/profiler.cpp 65a2e05e8f6005a378ae3647698dcba60fb95e9f 

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


Testing
---

manual test


Thanks,

haosdent huang

Re: Review Request 37586: Generate usage help information in libprocess.



> On Aug. 30, 2015, 10:16 p.m., Michael Park wrote:
> > 3rdparty/libprocess/src/profiler.cpp, lines 45-46
> > 
> >
> > Do you happen to know what the deal is with all the instances of `...` 
> > in this particular help message?

Because it is not need any params here, I update the description here.


- haosdent


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


On Sept. 4, 2015, 6:43 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37586/
> ---
> 
> (Updated Sept. 4, 2015, 6:43 p.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-3239
> https://issues.apache.org/jira/browse/MESOS-3239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Generate usage help information in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> 441f6d1c15afebdd98b73bb7430fcd8c8cf1e333 
>   3rdparty/libprocess/include/process/system.hpp 
> 7c8b49e78f76f9e131a4367f411c6dba447ccd90 
>   3rdparty/libprocess/src/help.cpp e4e0bb617956c7f79380069015b7170aa7716231 
>   3rdparty/libprocess/src/logging.cpp 
> 3d855e90e83a54cd344e49f075af0eadef1a1358 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> b9617507a16318b7de25d4875d6bc0b4409fcd29 
>   3rdparty/libprocess/src/profiler.cpp 
> 65a2e05e8f6005a378ae3647698dcba60fb95e9f 
> 
> Diff: https://reviews.apache.org/r/37586/diff/
> 
> 
> Testing
> ---
> 
> manual test
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37586: Generate usage help information in libprocess.


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

(Updated Sept. 4, 2015, 6:43 p.m.)


Review request for mesos, Michael Park and Vinod Kone.


Changes
---

Update according @mcypark reviews.


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


Repository: mesos


Description
---

Generate usage help information in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/include/process/help.hpp 
441f6d1c15afebdd98b73bb7430fcd8c8cf1e333 
  3rdparty/libprocess/include/process/system.hpp 
7c8b49e78f76f9e131a4367f411c6dba447ccd90 
  3rdparty/libprocess/src/help.cpp e4e0bb617956c7f79380069015b7170aa7716231 
  3rdparty/libprocess/src/logging.cpp 3d855e90e83a54cd344e49f075af0eadef1a1358 
  3rdparty/libprocess/src/metrics/metrics.cpp 
b9617507a16318b7de25d4875d6bc0b4409fcd29 
  3rdparty/libprocess/src/profiler.cpp 65a2e05e8f6005a378ae3647698dcba60fb95e9f 

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


Testing
---

manual test


Thanks,

haosdent huang

Re: Review Request 38118: Fix webui display problems when starting lots of tasks with small cpu value.



> On Sept. 4, 2015, 3:32 p.m., Michael Park wrote:
> > src/webui/master/static/home.html, line 101
> > 
> >
> > Does `number:4` not give us what we want here?
> 
> haosdent huang wrote:
> could not, if number is small than "0.01", use number:4 here would 
> become "0.". For normal value, "4.2" also would become "4.2000".

@karya @mcypark Do you know anyone also could help review this and 
https://reviews.apache.org/r/38056/ ? Most code in controller.js are written by 
Ross Allen, but seems he is not active any more now.


- haosdent


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


On Sept. 4, 2015, 12:54 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38118/
> ---
> 
> (Updated Sept. 4, 2015, 12:54 p.m.)
> 
> 
> Review request for mesos, Kapil Arya and Michael Park.
> 
> 
> Bugs: MESOS-2635
> https://issues.apache.org/jira/browse/MESOS-2635
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix webui display problems when starting lots of tasks with small cpu value.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827 
>   src/webui/master/static/js/controllers.js 
> 3445028138fd621d0e3a5427668c5eca7db5224f 
> 
> Diff: https://reviews.apache.org/r/38118/diff/
> 
> 
> Testing
> ---
> 
> manually test
> 
> * Before fix
> ![before_fix](https://issues.apache.org/jira/secure/attachment/12754133/fixed_before.png)
> * After fix
> ![after_fix](https://issues.apache.org/jira/secure/attachment/12754132/fixed_after.png)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37814: Added documentation for libprocess environment variables


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


Patch looks great!

Reviews applied: [37814]

All tests passed.

- Mesos ReviewBot


On Sept. 4, 2015, 4:14 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37814/
> ---
> 
> (Updated Sept. 4, 2015, 4:14 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, haosdent huang, and Michael Park.
> 
> 
> Bugs: MESOS-2466
> https://issues.apache.org/jira/browse/MESOS-2466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for libprocess environment variables
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
> 
> Diff: https://reviews.apache.org/r/37814/diff/
> 
> 
> Testing
> ---
> 
> Built the updated website using the docker site builder 
> (https://github.com/mesosphere/mesos-website-container).
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38118: Fix webui display problems when starting lots of tasks with small cpu value.



> On Sept. 4, 2015, 3:32 p.m., Michael Park wrote:
> > src/webui/master/static/home.html, line 101
> > 
> >
> > Does `number:4` not give us what we want here?

could not, if number is small than "0.01", use number:4 here would become 
"0.". For normal value, "4.2" also would become "4.2000".


- haosdent


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


On Sept. 4, 2015, 12:54 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38118/
> ---
> 
> (Updated Sept. 4, 2015, 12:54 p.m.)
> 
> 
> Review request for mesos, Kapil Arya and Michael Park.
> 
> 
> Bugs: MESOS-2635
> https://issues.apache.org/jira/browse/MESOS-2635
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix webui display problems when starting lots of tasks with small cpu value.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827 
>   src/webui/master/static/js/controllers.js 
> 3445028138fd621d0e3a5427668c5eca7db5224f 
> 
> Diff: https://reviews.apache.org/r/38118/diff/
> 
> 
> Testing
> ---
> 
> manually test
> 
> * Before fix
> ![before_fix](https://issues.apache.org/jira/secure/attachment/12754133/fixed_before.png)
> * After fix
> ![after_fix](https://issues.apache.org/jira/secure/attachment/12754132/fixed_after.png)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37814: Added documentation for libprocess environment variables



> On Sept. 4, 2015, 3:45 p.m., Michael Park wrote:
> > docs/configuration.md, lines 1502-1510
> > 
> >
> > Hm, looks like the closing tag, `` is missing here...? It also 
> > looks like it's missing in all of the other tables in this file. Is this 
> > valid HTML?
> 
> Greg Mann wrote:
> According to this: http://www.w3.org/TR/html5/syntax.html#optional-tags 
> it is legal, but it's probably a bit sloppy :-) I went ahead and added the 
> closing tags throughout the document to make things consistent.

Ah I see. Great, thank you!


- Michael


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


On Sept. 4, 2015, 4:14 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37814/
> ---
> 
> (Updated Sept. 4, 2015, 4:14 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, haosdent huang, and Michael Park.
> 
> 
> Bugs: MESOS-2466
> https://issues.apache.org/jira/browse/MESOS-2466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for libprocess environment variables
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
> 
> Diff: https://reviews.apache.org/r/37814/diff/
> 
> 
> Testing
> ---
> 
> Built the updated website using the docker site builder 
> (https://github.com/mesosphere/mesos-website-container).
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37814: Added documentation for libprocess environment variables



> On Sept. 4, 2015, 3:45 p.m., Michael Park wrote:
> > docs/configuration.md, lines 1502-1510
> > 
> >
> > Hm, looks like the closing tag, `` is missing here...? It also 
> > looks like it's missing in all of the other tables in this file. Is this 
> > valid HTML?

According to this: http://www.w3.org/TR/html5/syntax.html#optional-tags it is 
legal, but it's probably a bit sloppy :-) I went ahead and added the closing 
tags throughout the document to make things consistent.


- Greg


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


On Sept. 4, 2015, 4:14 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37814/
> ---
> 
> (Updated Sept. 4, 2015, 4:14 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, haosdent huang, and Michael Park.
> 
> 
> Bugs: MESOS-2466
> https://issues.apache.org/jira/browse/MESOS-2466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for libprocess environment variables
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
> 
> Diff: https://reviews.apache.org/r/37814/diff/
> 
> 
> Testing
> ---
> 
> Built the updated website using the docker site builder 
> (https://github.com/mesosphere/mesos-website-container).
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37814: Added documentation for libprocess environment variables


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

(Updated Sept. 4, 2015, 4:14 p.m.)


Review request for mesos, Alexander Rojas, haosdent huang, and Michael Park.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added documentation for libprocess environment variables


Diffs (updated)
-

  docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 

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


Testing
---

Built the updated website using the docker site builder 
(https://github.com/mesosphere/mesos-website-container).


Thanks,

Greg Mann

Re: Review Request 37814: Added documentation for libprocess environment variables


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



docs/configuration.md (lines 1502 - 1510)


Hm, looks like the closing tag, `` is missing here...? It also looks 
like it's missing in all of the other tables in this file. Is this valid HTML?


- Michael Park


On Sept. 4, 2015, 3:37 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37814/
> ---
> 
> (Updated Sept. 4, 2015, 3:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, haosdent huang, and Michael Park.
> 
> 
> Bugs: MESOS-2466
> https://issues.apache.org/jira/browse/MESOS-2466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for libprocess environment variables
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
> 
> Diff: https://reviews.apache.org/r/37814/diff/
> 
> 
> Testing
> ---
> 
> Built the updated website using the docker site builder 
> (https://github.com/mesosphere/mesos-website-container).
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38118: Fix webui display problems when starting lots of tasks with small cpu value.


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



src/webui/master/static/home.html (line 101)


Does `number:4` not give us what we want here?


- Michael Park


On Sept. 4, 2015, 12:54 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38118/
> ---
> 
> (Updated Sept. 4, 2015, 12:54 p.m.)
> 
> 
> Review request for mesos, Kapil Arya and Michael Park.
> 
> 
> Bugs: MESOS-2635
> https://issues.apache.org/jira/browse/MESOS-2635
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix webui display problems when starting lots of tasks with small cpu value.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827 
>   src/webui/master/static/js/controllers.js 
> 3445028138fd621d0e3a5427668c5eca7db5224f 
> 
> Diff: https://reviews.apache.org/r/38118/diff/
> 
> 
> Testing
> ---
> 
> manually test
> 
> * Before fix
> ![before_fix](https://issues.apache.org/jira/secure/attachment/12754133/fixed_before.png)
> * After fix
> ![after_fix](https://issues.apache.org/jira/secure/attachment/12754132/fixed_after.png)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37821: Join threads in libprocess when shutting down.


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


Patch looks great!

Reviews applied: [37821]

All tests passed.

- Mesos ReviewBot


On Sept. 4, 2015, 2:23 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37821/
> ---
> 
> (Updated Sept. 4, 2015, 2:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, and 
> switched to 'mcypark'.
> 
> 
> Bugs: MESOS-3158
> https://issues.apache.org/jira/browse/MESOS-3158
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Join threads in libprocess when shutting down.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/event_loop.hpp 
> 36a4cd2b1ff59f6922173ad17115bf80cc3c8f30 
>   3rdparty/libprocess/src/libev.cpp 97a2694f9b10bc61841443b21f4f96055493e840 
>   3rdparty/libprocess/src/libevent.cpp 
> ee7906470069b0391dde7cd685b1d4eb3a158c03 
>   3rdparty/libprocess/src/process.cpp 
> 755187c8761137cb2bf2f7295b29a63f63c68bc6 
> 
> Diff: https://reviews.apache.org/r/37821/diff/
> 
> 
> Testing
> ---
> 
> After configuring with both "../configure" and "../configure 
> --enable-libevent --enable-ssl":
> 
> make check
> 
> 
> Also, to check for race conditions related to the initialization/shutdown of 
> libprocess, try something like:
> 
> for n in {1..1000}; do echo $n; 3rdparty/libprocess/tests 
> --gtest_filter=ProcessTest.Spawn; done
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37821: Join threads in libprocess when shutting down.


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

(Updated Sept. 4, 2015, 2:23 p.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, and switched 
to 'mcypark'.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Join threads in libprocess when shutting down.


Diffs (updated)
-

  3rdparty/libprocess/src/event_loop.hpp 
36a4cd2b1ff59f6922173ad17115bf80cc3c8f30 
  3rdparty/libprocess/src/libev.cpp 97a2694f9b10bc61841443b21f4f96055493e840 
  3rdparty/libprocess/src/libevent.cpp ee7906470069b0391dde7cd685b1d4eb3a158c03 
  3rdparty/libprocess/src/process.cpp 755187c8761137cb2bf2f7295b29a63f63c68bc6 

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


Testing
---

After configuring with both "../configure" and "../configure --enable-libevent 
--enable-ssl":

make check


Also, to check for race conditions related to the initialization/shutdown of 
libprocess, try something like:

for n in {1..1000}; do echo $n; 3rdparty/libprocess/tests 
--gtest_filter=ProcessTest.Spawn; done


Thanks,

Greg Mann

Re: Review Request 38125: Included hostname in SlaveID.


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



src/master/master.cpp (line 5846)


IMHO, adding comments here cannot help the one who is reading log messages, 
can we use some other characters to connnect? Thanks.


- Guangya Liu


On 九月 4, 2015, 10:57 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38125/
> ---
> 
> (Updated 九月 4, 2015, 10:57 a.m.)
> 
> 
> Review request for mesos, Jan Schlicht and Till Toenshoff.
> 
> 
> Bugs: MESOS-115
> https://issues.apache.org/jira/browse/MESOS-115
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now including the hostname in the SlaveID, for human consumption. Also 
> testing this in a unit test.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
> 
> Diff: https://reviews.apache.org/r/38125/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 37669: Ignore overflow docker components in version parsing.



> On Sept. 4, 2015, 6:59 a.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 203
> > 
> >
> > We remove the overflow components
> 
> haosdent huang wrote:
> Change “remove the rest components here before parsing the docker 
> version” to "We remove the overflow components here before parsing the docker 
> version", right?

Oh, thanks for @brend help.


- haosdent


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


On Sept. 4, 2015, 2:27 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37669/
> ---
> 
> (Updated Sept. 4, 2015, 2:27 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Timothy Chen.
> 
> 
> Bugs: MESOS-2986
> https://issues.apache.org/jira/browse/MESOS-2986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignore overflow docker components in version parsing.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 2e17cedb3e29548bdd2d10bd87bd0f61bf123be8 
>   src/tests/containerizer/docker_tests.cpp 
> cfad36850b40ade7a41f5b92255320ca1ed1bf93 
> 
> Diff: https://reviews.apache.org/r/37669/diff/
> 
> 
> Testing
> ---
> 
> # Add new test case
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="DockerTest.ROOT_DOCKER_parsing_version" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38125: Included hostname in SlaveID.

2015-09-04 Thread Bernd Mathiske



> On Sept. 4, 2015, 5:43 a.m., Guangya Liu wrote:
> > src/master/master.cpp, line 5846
> > 
> >
> > I think that we cannot make the assumpation here as the host FQDN name 
> > may include the underscore, so how to handle the csae of if slave name is 
> > FQDN name? What about using other charactors to connect? Such as # or 
> > others?
> > 
> > http://stackoverflow.com/questions/10959757/the-use-of-the-underscore-in-host-names

There seems to be no perfect solution. I added a comment that explains it a bit 
better. Side condition: we must use a char that can also appear in a directory 
name.


- Bernd


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


On Sept. 4, 2015, 3:57 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38125/
> ---
> 
> (Updated Sept. 4, 2015, 3:57 a.m.)
> 
> 
> Review request for mesos, Jan Schlicht and Till Toenshoff.
> 
> 
> Bugs: MESOS-115
> https://issues.apache.org/jira/browse/MESOS-115
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now including the hostname in the SlaveID, for human consumption. Also 
> testing this in a unit test.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
> 
> Diff: https://reviews.apache.org/r/38125/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 37197: Docker image store.

2015-09-04 Thread Till Toenshoff



> On Aug. 26, 2015, 5:50 p.m., Till Toenshoff wrote:
> > src/slave/containerizer/provisioners/docker/store.cpp, line 135
> > 
> >
> > I am not a big fan of such magic numbers (referring to that '7'). How 
> > about following the pattern the fetcher is using here 
> > (src/slave/containerizer/fetcher.cpp)?
> > 
> > 
> > ```
> > [...]
> > namespace slave {
> > 
> > static const string FILE_URI_PREFIX = "file://";
> > [...]
> > 
> > if (strings::startsWith(imageUri, FILE_URI_PREFIX)) {
> > imageUri = imageUri.substr(FILE_URI_PREFIX.size());
> > }
> > ```

Why exactly did you close this without fixing the issue or giving some more 
insights on why you insist on magic numbers?


- Till


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


On Aug. 27, 2015, 11:39 p.m., Lily Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37197/
> ---
> 
> (Updated Aug. 27, 2015, 11:39 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Till Toenshoff, 
> and Timothy Chen.
> 
> 
> Bugs: MESOS-2849
> https://issues.apache.org/jira/browse/MESOS-2849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stored images currently kept indefinitely.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION 
>   src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
>   src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 
> 
> Diff: https://reviews.apache.org/r/37197/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Lily Chen
> 
>

Re: Review Request 38118: Fix webui display problems when starting lots of tasks with small cpu value.


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

(Updated Sept. 4, 2015, 12:54 p.m.)


Review request for mesos, Kapil Arya and Michael Park.


Changes
---

Attach test images


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


Repository: mesos


Description
---

Fix webui display problems when starting lots of tasks with small cpu value.


Diffs
-

  src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827 
  src/webui/master/static/js/controllers.js 
3445028138fd621d0e3a5427668c5eca7db5224f 

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


Testing (updated)
---

manually test

* Before fix
![before_fix](https://issues.apache.org/jira/secure/attachment/12754133/fixed_before.png)
* After fix
![after_fix](https://issues.apache.org/jira/secure/attachment/12754132/fixed_after.png)


Thanks,

haosdent huang

Re: Review Request 38125: Included hostname in SlaveID.


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



src/master/master.cpp (line 3672)


Since you are also adding port to slaveID, so can you please also update 
the log message here by adding slaveInfo.port()?



src/master/master.cpp (line 5846)


I think that we cannot make the assumpation here as the host FQDN name may 
include the underscore, so how to handle the csae of if slave name is FQDN 
name? What about using other charactors to connect? Such as # or others?

http://stackoverflow.com/questions/10959757/the-use-of-the-underscore-in-host-names


- Guangya Liu


On 九月 4, 2015, 10:57 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38125/
> ---
> 
> (Updated 九月 4, 2015, 10:57 a.m.)
> 
> 
> Review request for mesos, Jan Schlicht and Till Toenshoff.
> 
> 
> Bugs: MESOS-115
> https://issues.apache.org/jira/browse/MESOS-115
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now including the hostname in the SlaveID, for human consumption. Also 
> testing this in a unit test.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
> 
> Diff: https://reviews.apache.org/r/38125/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 38126: Add UT for QuiesceOffers


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


Patch looks great!

Reviews applied: [37532, 37873, 38119, 38120, 38121, 38124, 38126]

All tests passed.

- Mesos ReviewBot


On Sept. 4, 2015, 12:06 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38126/
> ---
> 
> (Updated Sept. 4, 2015, 12:06 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add UT for QuiesceOffers
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 
> 
> Diff: https://reviews.apache.org/r/38126/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38118: Fix webui display problems when starting lots of tasks with small cpu value.

2015-09-04 Thread Kapil Arya


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

Ship it!


Ship It!

- Kapil Arya


On Sept. 3, 2015, 10:54 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38118/
> ---
> 
> (Updated Sept. 3, 2015, 10:54 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-2635
> https://issues.apache.org/jira/browse/MESOS-2635
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix webui display problems when starting lots of tasks with small cpu value.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827 
>   src/webui/master/static/js/controllers.js 
> 3445028138fd621d0e3a5427668c5eca7db5224f 
> 
> Diff: https://reviews.apache.org/r/38118/diff/
> 
> 
> Testing
> ---
> 
> manually test
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 38126: Add UT for QuiesceOffers


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

Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
Kone.


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


Repository: mesos


Description
---

Add UT for QuiesceOffers


Diffs
-

  src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 

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


Testing
---


Thanks,

Guangya Liu

Review Request 38121: Add Python Support for QuiesceOffers


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

Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
Kone.


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


Repository: mesos


Description
---

Add Python Support for QuiesceOffers


Diffs
-

  src/python/interface/src/mesos/interface/__init__.py 
686abb24d88765d7ec7cab5b76b57299339b7b00 
  src/python/native/src/mesos/native/mesos_scheduler_driver_impl.hpp 
b1ad8e529806a047480e8d7affe4e7ecdc7d9725 
  src/python/native/src/mesos/native/mesos_scheduler_driver_impl.cpp 
609dfa3735f19eea2242dfb82ed3a4af3a0b8b9b 

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


Testing
---


Thanks,

Guangya Liu

Review Request 38120: Add Java Support for QuiesceOffers


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

Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
Kone.


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


Repository: mesos


Description
---

Add Java Support for QuiesceOffers


Diffs
-

  src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
a89ebed00f93801fc5bfb18e947f7120ef77d095 
  src/java/src/org/apache/mesos/MesosSchedulerDriver.java 
b9b2ea8734bb910c543708dd8adcdac45e03c34b 
  src/java/src/org/apache/mesos/SchedulerDriver.java 
183eec898553ed25149c58d2c1f85cf6579d5660 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 37873: Add quiesce logic in allocator


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

(Updated 九月 4, 2015, 12:06 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
Kone.


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


Repository: mesos


Description
---

Add quiesce logic in allocator


Diffs (updated)
-

  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  src/master/allocator/mesos/allocator.hpp 
aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 
cb4020dea897ef198cd9898cabecf61edfade834 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 

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


Testing
---


Thanks,

Guangya Liu

Review Request 38119: Add metrics of messages_quiesce_offers


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

Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
Kone.


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


Repository: mesos


Description
---

Add metrics of messages_quiesce_offers


Diffs
-

  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/master/metrics.hpp 2d07a16f2dc6811973c82259c3cccff07401b542 
  src/master/metrics.cpp d79206f08181c99641bef697be57b5d57b627285 
  src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
  src/tests/metrics_tests.cpp 3e9d7c2eadfcdd2522d65e4a55dc76808ba39159 

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


Testing
---


Thanks,

Guangya Liu

Review Request 38124: Add V1 Support for QuiesceOffers


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

Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
Kone.


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


Repository: mesos


Description
---

Add V1 Support for QuiesceOffers


Diffs
-

  include/mesos/v1/scheduler/scheduler.proto 
bd5e82a614b1163b29f9b20e562208efa1ba4b55 
  src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 37532: Add QUIESCE call interface to the scheduler


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

(Updated 九月 4, 2015, 12:06 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This is just part of MESOS-3037, this patch only add the interface
of QUIESCE call.


Diffs (updated)
-

  include/mesos/scheduler.hpp ee198b6955882f4f31466ca05429ca16fbf2f5cd 
  include/mesos/scheduler/scheduler.proto 
89daf8a6b74057ee156b3ad691397e76fcb835b8 
  src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 38125: Included hostname in SlaveID.

2015-09-04 Thread Till Toenshoff


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


Looks very good to me.


src/master/master.cpp (lines 5847 - 5849)


How about preventing the variable indentation by concentrating on the fact 
that can be considered  a parameter continuation;
```
  slaveId.set_value(
  info_.id() + "_" + stringify(slaveInfo.hostname()) + "_" +
  stringify(slaveInfo.port()) + "_S" + stringify(nextSlaveId++));
```



src/tests/master_tests.cpp (line 43)


Missing include for net::IP
```
#include 
```



src/tests/master_tests.cpp (line 3655)


Please add a heading comment that briefly describes this test in 1 or 2 
lines.



src/tests/master_tests.cpp (line 3657)


Seems rather obvious, does this comment really add any value?

Also the first two comments use this "step-scheme" but then you dont follow 
that pattern anymore - I think we can safely remove both (start master and 
start slave).



src/tests/master_tests.cpp (line 3676)


Lets add a blank line here.



src/tests/master_tests.cpp (line 3678)


Insert blank line here, please


- Till Toenshoff


On Sept. 4, 2015, 10:57 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38125/
> ---
> 
> (Updated Sept. 4, 2015, 10:57 a.m.)
> 
> 
> Review request for mesos, Jan Schlicht and Till Toenshoff.
> 
> 
> Bugs: MESOS-115
> https://issues.apache.org/jira/browse/MESOS-115
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now including the hostname in the SlaveID, for human consumption. Also 
> testing this in a unit test.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
> 
> Diff: https://reviews.apache.org/r/38125/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 38125: Included hostname in SlaveID.

2015-09-04 Thread Jan Schlicht


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



src/master/master.cpp (line 5847)


Indent with 4 spaces and break after the opening brace to reduce jaggedness.



src/master/master.cpp (line 5849)


s/"_S"/"-S"



src/tests/master_tests.cpp (line 3672)


s/client/slave? Or what is meant with client?


- Jan Schlicht


On Sept. 4, 2015, 12:57 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38125/
> ---
> 
> (Updated Sept. 4, 2015, 12:57 p.m.)
> 
> 
> Review request for mesos, Jan Schlicht and Till Toenshoff.
> 
> 
> Bugs: MESOS-115
> https://issues.apache.org/jira/browse/MESOS-115
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now including the hostname in the SlaveID, for human consumption. Also 
> testing this in a unit test.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
> 
> Diff: https://reviews.apache.org/r/38125/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Review Request 38125: Included hostname in SlaveID.

2015-09-04 Thread Bernd Mathiske


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

Review request for mesos, Jan Schlicht and Till Toenshoff.


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


Repository: mesos


Description
---

Now including the hostname in the SlaveID, for human consumption. Also testing 
this in a unit test.


Diffs
-

  src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6 
  src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
  src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 

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


Testing
---

make check


Thanks,

Bernd Mathiske

Re: Review Request 38118: Fix webui display problems when starting lots of tasks with small cpu value.


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


Patch looks great!

Reviews applied: [38118]

All tests passed.

- Mesos ReviewBot


On Sept. 4, 2015, 2:54 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38118/
> ---
> 
> (Updated Sept. 4, 2015, 2:54 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-2635
> https://issues.apache.org/jira/browse/MESOS-2635
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix webui display problems when starting lots of tasks with small cpu value.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827 
>   src/webui/master/static/js/controllers.js 
> 3445028138fd621d0e3a5427668c5eca7db5224f 
> 
> Diff: https://reviews.apache.org/r/38118/diff/
> 
> 
> Testing
> ---
> 
> manually test
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37669: Ignore overflow docker components in version parsing.