Re: Review Request 44454: Returned real path for PORT_MAPPING_BIND_MOUNT_ROOT.

[Jie Yu]

> On March 8, 2016, 1:22 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.hpp, lines 
> > 66-72
> > 
> >
> > This is not the reliable solution. What if /var/run points to some 
> > other place?
> > 
> > I think we should make this a field member (instead of a constant), and 
> > call realpath when creating the isolator.
> 
> Guangya Liu wrote:
> @yujie, what about the following?
> 
> inline std::string PORT_MAPPING_BIND_MOUNT_ROOT()
> {
>   return path::join(
>   os::realpath("/var/run").get(),
>   "netns");
> }

I posted a fix:
https://reviews.apache.org/r/45520/

The above patch also fixed another issue.


- Jie


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


On March 7, 2016, 3:42 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44454/
> ---
> 
> (Updated March 7, 2016, 3:42 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4662
> https://issues.apache.org/jira/browse/MESOS-4662
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Returned real path for PORT_MAPPING_BIND_MOUNT_ROOT.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 0fe2f486eb733acf738c1c61fc44f820d7401afc 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 134b6c759b769cf335539e49eff817973c7f96a4 
> 
> Diff: https://reviews.apache.org/r/44454/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45520: Fixed the bind mount root issue in port mapping isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45520]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 31, 2016, 1:47 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45520/
> ---
> 
> (Updated March 31, 2016, 1:47 a.m.)
> 
> 
> Review request for mesos, Ian Downes and Cong Wang.
> 
> 
> Bugs: MESOS-4662
> https://issues.apache.org/jira/browse/MESOS-4662
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the bind mount root issue in port mapping isolator. This patch fixed 
> two issues:
> 1) no long assume /var/run/netns is a realpath
> 2) made sure /var/run/netns is a shared mount in its own mount peer group
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 0fe2f486eb733acf738c1c61fc44f820d7401afc 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 323c84a3d960a196d8ba87f753814e9d43a07957 
>   src/tests/containerizer/port_mapping_tests.cpp 
> e062daa9fcfc776144b48325daa1f1284c5e59a4 
> 
> Diff: https://reviews.apache.org/r/45520/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on Fedora23
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 45474: MESOS-1739: Allow slave reconfiguration on restart, Phase 1.

[Deshi Xiao]

> On 三月 31, 2016, 2:03 a.m., Jie Yu wrote:
> > This is a high level question: I am now sure if adding attributes is safe 
> > or not. For instance, my framework has the following rule: only schedule 
> > tasks to agents that do not have attribute "not_safe". Now, say agent A is 
> > initially without that attribute. My framework lands several tasks on that 
> > agent. Later, when agent restarts, the operator adds the new attribute 
> > "not_safe". Suddently, i have tasks running on unsafe boxes. oops.

I think so, this is very common case. it need more detail discussion with team.


- Deshi


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


On 三月 30, 2016, 8:13 a.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45474/
> ---
> 
> (Updated 三月 30, 2016, 8:13 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, haosdent huang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1739
> https://issues.apache.org/jira/browse/MESOS-1739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Phase 1
> Make SlaveInfo mutable throughout the stack, and allow for expansion of 
> resources and attributes only (Which allows testing to make sure it 
> propagates to the allocator, shows up in offers, etc). Ensure there is 
> unified checking for incompatibilities in both the slave and master (the 
> slave should validate the config, the master should validate that all 
> operations the slave takes are legal).
> 
> it derived from another PR(https://reviews.apache.org/r/25525/)
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1f1a31020096efa5db698e86ac74e61dfdb4b94a 
> 
> Diff: https://reviews.apache.org/r/45474/diff/
> 
> 
> Testing
> ---
> 
> make check on localhost
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 45500: Fix Mesos-1104, linux/fs.hpp remove mesos::internal:: should be enough.

[Deshi Xiao]

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

(Updated 三月 31, 2016, 3:38 a.m.)


Review request for mesos, haosdent huang and Cong Wang.


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


Repository: mesos


Description
---

other exists code under src/linux/, e.g. perf.hpp, ns.hpp
I think remove mesos::internal:: should be enough.


Diffs (updated)
-

  src/linux/cgroups.cpp b7420c682970c4838e84973198ac4fe7af5f68f9 
  src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
  src/linux/fs.cpp dbf94759b6382bfafd3f3b8b4c2047af36a53ad5 
  src/slave/containerizer/mesos/mount.cpp 
bf17b015e45579882fd31248e8609eec6d58a9da 
  src/tests/containerizer/fs_tests.cpp 020fd8a4bf5911671e038a96f8b50c0f58e22ed5 
  src/tests/containerizer/port_mapping_tests.cpp 
e062daa9fcfc776144b48325daa1f1284c5e59a4 

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


Testing
---

make check on localhost(centos)


Thanks,

Deshi Xiao

Review Request 45505: WIP : Providing starting implementation for appc runtime isolator.

[Srinivas Brahmaroutu]

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

Review request for mesos, Guangya Liu and Jie Yu.


Repository: mesos


Description
---

WIP : Providing starting implementation for appc runtime isolator.


Diffs
-

  include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
  include/mesos/slave/isolator.proto 60a9bb637e12593a97ed1a7c510ebccd4e5a9615 
  src/CMakeLists.txt 366a7ae8f6ef1d55202699df0502a30f15a35e1f 
  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/appc/runtime.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/appc/runtime.cpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
5c96e9f6603d39889e6bc807874d35d0cb3556be 
  src/tests/containerizer/runtime_isolator_tests.cpp 
9f3b0b08da7cebba722062a9932fae1b5f825efb 

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


Testing
---


Thanks,

Srinivas Brahmaroutu

Re: Review Request 44441: Treated command as executable value and arguments in mesos-execute.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [1]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 31, 2016, 1:03 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1/
> ---
> 
> (Updated March 31, 2016, 1:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4882
> https://issues.apache.org/jira/browse/MESOS-4882
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Treated command as executable value and arguments in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp af62f41622e1c43acd8f257c54f8754162c433b8 
> 
> Diff: https://reviews.apache.org/r/1/diff/
> 
> 
> Testing
> ---
> 
> ./src/mesos-execute --master=192.168.56.12:5050  --name=test_mesos 
> --docker_image=busybox:latest --containerizer=mesos --no-shell  
> --command="ls,/etc/passwd" 
> I0307 15:48:09.834506 27450 sched.cpp:222] Version: 0.29.0
> I0307 15:48:09.841404 27468 sched.cpp:326] New master detected at 
> master@192.168.56.12:5050
> I0307 15:48:09.843992 27468 sched.cpp:336] No credentials provided. 
> Attempting to register without authentication
> I0307 15:48:09.848901 27468 sched.cpp:703] Framework registered with 
> a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001
> Framework registered with a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001
> task test_mesos submitted to slave 6fa2afa1-768b-4f2e-9c69-9f1017634e72-S2
> Received status update TASK_RUNNING for task test_mesos
> Received status update TASK_FINISHED for task test_mesos
> I0307 15:48:10.284418 27466 sched.cpp:1903] Asked to stop the driver
> I0307 15:48:10.284494 27466 sched.cpp:1143] Stopping framework 
> 'a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001'
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45474: MESOS-1739: Allow slave reconfiguration on restart, Phase 1.

[Jie Yu]

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



This is a high level question: I am now sure if adding attributes is safe or 
not. For instance, my framework has the following rule: only schedule tasks to 
agents that do not have attribute "not_safe". Now, say agent A is initially 
without that attribute. My framework lands several tasks on that agent. Later, 
when agent restarts, the operator adds the new attribute "not_safe". Suddently, 
i have tasks running on unsafe boxes. oops.

- Jie Yu


On March 30, 2016, 8:13 a.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45474/
> ---
> 
> (Updated March 30, 2016, 8:13 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, haosdent huang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1739
> https://issues.apache.org/jira/browse/MESOS-1739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Phase 1
> Make SlaveInfo mutable throughout the stack, and allow for expansion of 
> resources and attributes only (Which allows testing to make sure it 
> propagates to the allocator, shows up in offers, etc). Ensure there is 
> unified checking for incompatibilities in both the slave and master (the 
> slave should validate the config, the master should validate that all 
> operations the slave takes are legal).
> 
> it derived from another PR(https://reviews.apache.org/r/25525/)
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1f1a31020096efa5db698e86ac74e61dfdb4b94a 
> 
> Diff: https://reviews.apache.org/r/45474/diff/
> 
> 
> Testing
> ---
> 
> make check on localhost
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 45513: Fixed mount point check in port mapping isolator.

[Jie Yu]

> On March 31, 2016, 12:14 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp, lines 
> > 1901-1909
> > 
> >
> > Thanks for the patch!
> > 
> > Actually, I am working on a patch already. There are more issues here. 
> > We need to make sure /var/run/netns (or /run/netns) is a shared mount in 
> > its own mount peer group so that we don't create extra reference to the 
> > mount when launching a container (which will clone a new mount namespace, 
> > thus adding a reference to the mount). That'll make sure the rmdir of the 
> > mount points will succeeds after umount (othewise, we might get EBUSY).

My patch posted here:
https://reviews.apache.org/r/45520

Can you take a look (and test it in your environment)?


- Jie


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


On March 31, 2016, 12:04 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45513/
> ---
> 
> (Updated March 31, 2016, 12:04 a.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On Fedora, /var/run is a symlink to /run, which causes the mount point check 
> code fail to detect it therefore keeps mounting it each time when we restart 
> the slave.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 323c84a3d960a196d8ba87f753814e9d43a07957 
> 
> Diff: https://reviews.apache.org/r/45513/diff/
> 
> 
> Testing
> ---
> 
> Manually start mesos slave and verify there is no more leak in /proc/mounts.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 44441: Treated command as executable value and arguments in mesos-execute.

[Guangya Liu]

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

(Updated 三月 31, 2016, 1:03 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Rebase


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


Repository: mesos


Description
---

Treated command as executable value and arguments in mesos-execute.


Diffs (updated)
-

  src/cli/execute.cpp af62f41622e1c43acd8f257c54f8754162c433b8 

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


Testing
---

./src/mesos-execute --master=192.168.56.12:5050  --name=test_mesos 
--docker_image=busybox:latest --containerizer=mesos --no-shell  
--command="ls,/etc/passwd" 
I0307 15:48:09.834506 27450 sched.cpp:222] Version: 0.29.0
I0307 15:48:09.841404 27468 sched.cpp:326] New master detected at 
master@192.168.56.12:5050
I0307 15:48:09.843992 27468 sched.cpp:336] No credentials provided. Attempting 
to register without authentication
I0307 15:48:09.848901 27468 sched.cpp:703] Framework registered with 
a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001
Framework registered with a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001
task test_mesos submitted to slave 6fa2afa1-768b-4f2e-9c69-9f1017634e72-S2
Received status update TASK_RUNNING for task test_mesos
Received status update TASK_FINISHED for task test_mesos
I0307 15:48:10.284418 27466 sched.cpp:1903] Asked to stop the driver
I0307 15:48:10.284494 27466 sched.cpp:1143] Stopping framework 
'a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001'


Thanks,

Guangya Liu

Re: Review Request 45471: Fix a test case bug due to upgrade http-parser to 2.6.1.

[Zhiwei Chen]

> On March 31, 2016, 2:10 a.m., Greg Mann wrote:
> > This could break existing clients which include extra whitespace, though it 
> > does seem that RFC-7230 specifically prohibits whitespace in that location. 
> > I wonder if a deprecation cycle is advisable for this change?
> > 
> > Also, it's not immediately clear to me: are we using http-parser in 
> > "strict" mode?
> 
> Vinod Kone wrote:
> Good point. Though I would be surprised if typical clients use colons 
> after header names. IIIUC (zhiwei can correct me if i'm wrong) 2.6.1 
> disallowed spaces before colon irrespective of strict mode (which was a bug). 
> I think they fixed the behavior in 2.6.2 so that it rejects spaces iff the 
> mode is strict. If that's the case, we should probably just upgrade to 2.6.2 
> instead of 2.6.1?

Yes, I think I need to create another ticket to upgrade to 2.6.2.


- Zhiwei


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


On March 30, 2016, 1:35 p.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45471/
> ---
> 
> (Updated March 30, 2016, 1:35 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-5063
> https://issues.apache.org/jira/browse/MESOS-5063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix a test case bug due to upgrade http-parser to 2.6.1.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 53a0ed03226030104af43a45c6ccfbfc0f4b7e9f 
> 
> Diff: https://reviews.apache.org/r/45471/diff/
> 
> 
> Testing
> ---
> 
> ../configure --enable-libevent --enable-ssl
> 
> sudo make dist check
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45513: Fixed mount point check in port mapping isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (lines 1901 - 
1909)


Thanks for the patch!

Actually, I am working on a patch already. There are more issues here. We 
need to make sure /var/run/netns (or /run/netns) is a shared mount in its own 
mount peer group so that we don't create extra reference to the mount when 
launching a container (which will clone a new mount namespace, thus adding a 
reference to the mount). That'll make sure the rmdir of the mount points will 
succeeds after umount (othewise, we might get EBUSY).


- Jie Yu


On March 31, 2016, 12:04 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45513/
> ---
> 
> (Updated March 31, 2016, 12:04 a.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On Fedora, /var/run is a symlink to /run, which causes the mount point check 
> code fail to detect it therefore keeps mounting it each time when we restart 
> the slave.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 323c84a3d960a196d8ba87f753814e9d43a07957 
> 
> Diff: https://reviews.apache.org/r/45513/diff/
> 
> 
> Testing
> ---
> 
> Manually start mesos slave and verify there is no more leak in /proc/mounts.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 45434: Removed unused credentials.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On March 30, 2016, 1:20 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45434/
> ---
> 
> (Updated March 30, 2016, 1:20 a.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/files_tests.cpp 2f0f84cd78e7f382b70b97c8711f71cea64ce0a5 
> 
> Diff: https://reviews.apache.org/r/45434/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 45434: Removed unused credentials.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On March 30, 2016, 8:20 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45434/
> ---
> 
> (Updated March 30, 2016, 8:20 a.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/files_tests.cpp 2f0f84cd78e7f382b70b97c8711f71cea64ce0a5 
> 
> Diff: https://reviews.apache.org/r/45434/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 45513: Fixed mount point check in port mapping isolator.

[Cong Wang]

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

Review request for mesos, Ian Downes and Jie Yu.


Repository: mesos


Description
---

On Fedora, /var/run is a symlink to /run, which causes the mount point check 
code fail to detect it therefore keeps mounting it each time when we restart 
the slave.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
323c84a3d960a196d8ba87f753814e9d43a07957 

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


Testing
---

Manually start mesos slave and verify there is no more leak in /proc/mounts.


Thanks,

Cong Wang

Re: Review Request 44950: Add XFS disk isolator documentation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44945, 44946, 44947, 44948, 44949, 44950]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 30, 2016, 10:18 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44950/
> ---
> 
> (Updated March 30, 2016, 10:18 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOs-4828
> https://issues.apache.org/jira/browse/MESOs-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add XFS disk isolator documentation.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 75c9a0ae7d63ff8b4337778aa7bb28676a1b9f56 
>   docs/mesos-containerizer.md 15fb5bdbe74e059614b8948108f32cd04b623305 
> 
> Diff: https://reviews.apache.org/r/44950/diff/
> 
> 
> Testing
> ---
> 
> Make check. Source inspection.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45314: Updated `network::connect` to use the typeful `Try` error state.

[Daniel Pravat]

> On March 26, 2016, 8:05 p.m., Daniel Pravat wrote:
> > 3rdparty/libprocess/include/process/network.hpp, line 75
> > 
> >
> > We tried to avoid axecuting too much code between ::connect 
> > WSAGetLastError() calls. 
> > Constructing the parameter for WindowsSocketError may overwrite the 
> > last error. It should be better to pass in the return from 
> > WSAGetLastError() as a parameter to the constructor, that is evaluate 
> > before the std::string parameter.
> 
> Michael Park wrote:
> Gave this some thought. I'm inclined to keep this as is.
> 
> Even if you're suggesting that we provide `::WSAGetLastError()` on every 
> callsite like:
> 
> ```
> int result = ::connect(...);
> 
> // BE AWARE!
> 
> // Even ignoring `::WSAGetLastError()` being Windows-specific.
> return ConnectError(::WSAGetLastError(), "Failed to connect to " + 
> stringify(address));
> ```
> 
> The order of evaluation of the parameters is unspecified.
> 
> Even if it were to be specified (left-to-right), we still need to be 
> mindful of the code between
> `::connect` and `ConnectError`. We would run into the same sequence of 
> code execution if someone
> were to pull out the expression and constructed a variable.
> 
> ```
> int result = ::connect(...);
> 
> // BE AWARE!
> 
> std::string message = "Failed to connect to " + stringify(address);  // 
> BE AWARE!
> return ConnectError(::WSAGetLastError(), message);
> ```
> 
> The only way we would completely solve this is if we were to say either: 
> `::WSAGetLastError()` must be
> called immediately after `::connect`, or ensure that nothing between 
> `::connect` and the call to
> `::WSAGetLastError()` can overwrite the error code. The latter is what we 
> currently do, and I don't think
> that reordering just the `::WSAGetLastError` call and the argument to 
> `ConnectError` is a big win.
> 
> Please share your ideas and solutions!

Exposing a constructor taking both the error code and the message should be 
suficient. 

The convenience provided by the parameters reordering is not adding a lot of 
value given the small number of instance where the error code is interpreted.


- Daniel


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


On March 24, 2016, 8:23 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45314/
> ---
> 
> (Updated March 24, 2016, 8:23 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/network.hpp 
> 9976257d2d13316062bc95a22ab564ca0df165e5 
>   3rdparty/libprocess/src/poll_socket.cpp 
> 6e6634b4b352e3723096521843546cf56ec6dd8b 
> 
> Diff: https://reviews.apache.org/r/45314/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 44950: Add XFS disk isolator documentation.

[James Peach]

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

(Updated March 30, 2016, 10:18 p.m.)


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


Changes
---

Rebased.


Bugs: MESOs-4828
https://issues.apache.org/jira/browse/MESOs-4828


Repository: mesos


Description
---

Add XFS disk isolator documentation.


Diffs (updated)
-

  docs/configuration.md 75c9a0ae7d63ff8b4337778aa7bb28676a1b9f56 
  docs/mesos-containerizer.md 15fb5bdbe74e059614b8948108f32cd04b623305 

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


Testing
---

Make check. Source inspection.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

[James Peach]

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

(Updated March 30, 2016, 10:19 p.m.)


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


Changes
---

Rebased.


Bugs: MESOs-4828
https://issues.apache.org/jira/browse/MESOs-4828


Repository: mesos


Description
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp 345a2256c8dfcd94ce8233f270b450364b27cf37 
  src/slave/flags.cpp 8868e1ec2db93fa8587e095617548fb923db1fb0 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44947: Add tests for XFS project quota utilities.

[James Peach]

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

(Updated March 30, 2016, 10:18 p.m.)


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add tests for XFS project quota utilities.


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/tests/cluster.cpp 2da0bd7612d571277e76d0a95ad8e776434af323 
  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 
  src/tests/environment.cpp 7617e43587cb81104786d06f753f08565a6c2d0a 

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


Testing
---

Make check. Manual testing. These tests.


Thanks,

James Peach

Re: Review Request 44946: Add utility functions to manipulate XFS project quotas.

[James Peach]

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

(Updated March 30, 2016, 10:18 p.m.)


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add utility functions to manipulate XFS project quotas.


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 

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


Testing
---

Make check. Manual verification. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44945: Add autoconf tests for XFS project quotas.

[James Peach]

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

(Updated March 30, 2016, 10:18 p.m.)


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


Changes
---

Rebased and updates with review comments.


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


Repository: mesos


Description
---

Add autoconf tests for XFS project quotas.


Diffs (updated)
-

  configure.ac 812c92aa13c0cc01c4907669000d050221cb62fd 

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


Testing
---

Make check. Manual verification.


Thanks,

James Peach

Re: Review Request 44136: Libprocess: [1/2] Conditioned out Windows-incompatible includes.

[Michael Park]

> On March 22, 2016, 9:16 p.m., Michael Park wrote:
> > 3rdparty/libprocess/src/pid.cpp, lines 13-23
> > 
> >
> > Can we maybe organize it like this?
> > 
> > ```
> > #ifndef __WINDOWS__
> > #include 
> > #include 
> > #endif // __WINDOWS__
> > 
> > #include 
> > #include 
> > #include 
> > #include 
> > ```
> 
> Daniel Pravat wrote:
> Thanks! I splited the change in two commits: 
> https://reviews.apache.org/r/44136/
> https://reviews.apache.org/r/45194/
> 
> Gilbert Song wrote:
> @MPark, just fly by, a little formatting question. Do we already decide 
> to gather conditioned headers in this way? Understand that it is more 
> convenient, but seems like we have all the other `#ifdef __linux__` headers 
> separated.

@Gilbert: I don't have a strong preference about this. If someone else does or 
if there's clear consistent, precedence elsewhere, I'm happy to reformat this.
I've committed this however, so let me know if you want to submit a separate 
patch!


- Michael


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


On March 23, 2016, 2:49 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44136/
> ---
> 
> (Updated March 23, 2016, 2:49 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Alex 
> Clemmer, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Libprocess: [1/2] Conditioned out Windows-incompatible includes.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/http.cpp 3ca0cfd37ac58f6d2bf5341dc88e8abed05fe994 
>   3rdparty/libprocess/src/pid.cpp 9387f59a3834af368bf37f8cc2e85102f0bb34f6 
> 
> Diff: https://reviews.apache.org/r/44136/diff/
> 
> 
> Testing
> ---
> 
> OSX: make
> Windows: build/run
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 45496: Added documentation of LIBPROCESS_NUM_WORKER_THREADS.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On March 30, 2016, 5:34 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45496/
> ---
> 
> (Updated March 30, 2016, 5:34 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation of LIBPROCESS_NUM_WORKER_THREADS. Documentation related 
> to https://reviews.apache.org/r/43144/ for 
> https://issues.apache.org/jira/browse/MESOS-4353.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 75c9a0ae7d63ff8b4337778aa7bb28676a1b9f56 
> 
> Diff: https://reviews.apache.org/r/45496/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

[Michael Park]

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


Fix it, then Ship it!





src/cli/execute.cpp (line 483)


It looks like this patch needs to be rebased now, sorry about the delay, 
but I'll get this committed once it's rebased!


- Michael Park


On March 28, 2016, 1:11 p.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 28, 2016, 1:11 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 45381: Migrate /monitor/statistics and /monitor/statistics.json to slave.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45381]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 30, 2016, 5:30 p.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45381/
> ---
> 
> (Updated March 30, 2016, 5:30 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These two endpoints and their underlying logics are moved from
> ResourceMonitorProcess to slave process. ResourceMonitor is removed.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/slave/http.cpp 7a872c6e21b115500c5291b7cb9fb240ec9dc8ed 
>   src/slave/monitor.hpp 70a7c88fefd4577f53f85e3eccdd6b69ab6f3acd 
>   src/slave/monitor.cpp 5c1dd354595e67e5eb603c5d49850f84b84b 
>   src/slave/slave.hpp 3ba335fcd31a92af9609023daae328b7f4bf5e59 
>   src/slave/slave.cpp fc77f594d16a9fb2ca001e089d74e2c0ffeb5baa 
>   src/tests/monitor_tests.cpp 5dcb2481ff2f1a7caf54036bc3e60c78feb982b1 
>   src/tests/oversubscription_tests.cpp 
> ba036810758d99a6fb0034c5e2bc7829e2343a44 
>   src/tests/slave_tests.cpp 57fc50360eae85819ae6ce714b0c3c4c1867b2b8 
> 
> Diff: https://reviews.apache.org/r/45381/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> https://issues.apache.org/jira/browse/MESOS-4891
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 45314: Updated `network::connect` to use the typeful `Try` error state.

[Michael Park]

> On March 26, 2016, 8:05 p.m., Daniel Pravat wrote:
> > 3rdparty/libprocess/include/process/network.hpp, line 75
> > 
> >
> > We tried to avoid axecuting too much code between ::connect 
> > WSAGetLastError() calls. 
> > Constructing the parameter for WindowsSocketError may overwrite the 
> > last error. It should be better to pass in the return from 
> > WSAGetLastError() as a parameter to the constructor, that is evaluate 
> > before the std::string parameter.

Gave this some thought. I'm inclined to keep this as is.

Even if you're suggesting that we provide `::WSAGetLastError()` on every 
callsite like:

```
int result = ::connect(...);

// BE AWARE!

// Even ignoring `::WSAGetLastError()` being Windows-specific.
return ConnectError(::WSAGetLastError(), "Failed to connect to " + 
stringify(address));
```

The order of evaluation of the parameters is unspecified.

Even if it were to be specified (left-to-right), we still need to be mindful of 
the code between
`::connect` and `ConnectError`. We would run into the same sequence of code 
execution if someone
were to pull out the expression and constructed a variable.

```
int result = ::connect(...);

// BE AWARE!

std::string message = "Failed to connect to " + stringify(address);  // BE 
AWARE!
return ConnectError(::WSAGetLastError(), message);
```

The only way we would completely solve this is if we were to say either: 
`::WSAGetLastError()` must be
called immediately after `::connect`, or ensure that nothing between 
`::connect` and the call to
`::WSAGetLastError()` can overwrite the error code. The latter is what we 
currently do, and I don't think
that reordering just the `::WSAGetLastError` call and the argument to 
`ConnectError` is a big win.

Please share your ideas and solutions!


- Michael


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


On March 24, 2016, 8:23 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45314/
> ---
> 
> (Updated March 24, 2016, 8:23 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/network.hpp 
> 9976257d2d13316062bc95a22ab564ca0df165e5 
>   3rdparty/libprocess/src/poll_socket.cpp 
> 6e6634b4b352e3723096521843546cf56ec6dd8b 
> 
> Diff: https://reviews.apache.org/r/45314/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 45482: Replace NULL with nullptr.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [45482]

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

Error:
2016-03-30 19:47:18 URL:https://reviews.apache.org/r/45482/diff/raw/ 
[241389/241389] -> "45482.patch" [1]
error: patch failed: src/sched/sched.cpp:1808
error: src/sched/sched.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/1/console

- Mesos ReviewBot


On March 30, 2016, 11:07 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45482/
> ---
> 
> (Updated March 30, 2016, 11:07 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3243
> https://issues.apache.org/jira/browse/MESOS-3243
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> RUN: find . -name "*.[hc]pp" | xargs -P 4 sed -i 's/\bNULL\b/nullptr/g'
> 
> 
> Diffs
> -
> 
>   include/mesos/module.hpp 30a9674686221d25132368e6c10664daa9cd6dc0 
>   src/authentication/cram_md5/authenticatee.cpp 
> 7d4994333428e5d8aa9ce6b85e0a7f42a0ddc4c8 
>   src/authentication/cram_md5/authenticator.cpp 
> 027eba4f433fff328d68c41005bc59c41c7ae668 
>   src/authentication/cram_md5/auxprop.cpp 
> d82d2d2f7793859772d89cc91bee09240624c613 
>   src/authorizer/local/authorizer.cpp 
> 0f0d9276337858984f0b19a82ffca74ee84dc650 
>   src/examples/balloon_executor.cpp 399218478a596387814d123290eeeb33102fad7a 
>   src/examples/example_module_impl.cpp 
> aeffdc0e95fa7badc8cfa09e2b57462e427fec55 
>   src/examples/load_generator_framework.cpp 
> b22a09e1749d4b523addacad99858d7b6bde3403 
>   src/examples/test_allocator_module.cpp 
> 1255a4a22a9d1a5724bef78dfc4dee498ed50fff 
>   src/examples/test_anonymous_module.cpp 
> dd291cff3b5d47337e371cd2c1082fd6716af3fc 
>   src/examples/test_authorizer_module.cpp 
> 19ec7cd114562f74c660b83b39235127d25001ee 
>   src/examples/test_container_logger_module.cpp 
> 76dd494fa4c32514ba14b1f4498b588ac9051b4d 
>   src/examples/test_hook_module.cpp abd132b3f39265683542a9d1533d2a31bd81769a 
>   src/examples/test_http_authenticator_module.cpp 
> 8affdb60ae6336a2be5218c7bd04a4d5efeeb3f4 
>   src/examples/test_isolator_module.cpp 
> a4a2103b1e449837b95948c8b5c25e05c5d13860 
>   src/examples/test_qos_controller_module.cpp 
> f382fc443fa0a4545ba78dc28016e4d0a0dd16b1 
>   src/examples/test_resource_estimator_module.cpp 
> 229b2931e9129c194850f04051060d33bfa06570 
>   src/exec/exec.cpp 8f672602daf090dec032d2b684e407e5d043af9c 
>   src/files/files.hpp 90acb3406c46c164108deb559af71fb109a5773b 
>   src/java/jni/construct.cpp 0bfe6291e94a69a787c965fa3a3a90d2ebae8d72 
>   src/java/jni/convert.cpp f1a486d8d2f6d43efd89407c928734bd9715e591 
>   src/java/jni/org_apache_mesos_Log.cpp 
> 140b950136417eed7cba363a89537ed2f33832ff 
>   src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp 
> 9a92ade5d54bd814353acb40170d7aa4d8fe4a77 
>   src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
> c723e2d7bce98e6ac5ab587ce8c42fa9ba8b47e4 
>   src/java/jni/org_apache_mesos_state_AbstractState.cpp 
> 4fd43ca31c6917e81ea1b331b8507ca42a2249cf 
>   src/java/jni/org_apache_mesos_state_Variable.cpp 
> 7a69b3d878e6eaa87bfaf56c0500e11efe4ac44d 
>   src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp 
> 23a89c3e5c5309a8a9b45c168fb61b12d89db1ec 
>   src/jvm/jvm.hpp d5023e4050d109ca97cedb05f85e1c24202ac3b0 
>   src/jvm/jvm.cpp 779f8b987e2673ca5ab924caf00eea09a0a79af5 
>   src/jvm/org/apache/zookeeper.hpp 7e9c3aaa9f0eed44994004f9e32ce7ce3cf6d335 
>   src/launcher/executor.cpp 4e9b4d9820f7c2f4cb1b3e16e2f4c8c13500693f 
>   src/linux/cgroups.cpp b7420c682970c4838e84973198ac4fe7af5f68f9 
>   src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
>   src/linux/fs.cpp dbf94759b6382bfafd3f3b8b4c2047af36a53ad5 
>   src/linux/perf.cpp 749e676aaf2ce639dd976f2b23e323300c6114c5 
>   src/linux/routing/diagnosis/diagnosis.cpp 
> 8b86a8864c08b078425dcc242d323763d6ec15dd 
>   src/linux/routing/filter/internal.hpp 
> 8690bf56472c318e85afb53d35494790ae4da27b 
>   src/linux/routing/internal.hpp 8f68119819f7c79ece1a13ac1894b1802ddc8e19 
>   src/linux/routing/link/internal.hpp 
> 8f05e5f513208c8f966bc324c9fe994a5807b051 
>   src/linux/routing/queueing/internal.hpp 
> 768ed325f9b259e150779eb3ad74f4e5d4bcc7a2 
>   src/linux/routing/route.cpp 4b33350dc3761e19121c29c0d090cfdae544e4ef 
>   src/linux/systemd.cpp 9f6e06cfcf6f5b38971ff75eb85326d043140b4b 
>   src/local/local.hpp f4ae285edc30a0fb1c960d50dfb1a859b2eae166 
>   src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa 
>   src/log/leveldb.cpp ba2f62bc97e4a0eaa8a54824551686467db4b9e4 
>   src/log/log.cpp a37676068dae14b1adc61ef75e2742c16e7a6e42 
>   

Re: Review Request 45489: Replaced reinterpret_cast with static_cast in libprocess.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45318, 45319, 45488, 45489]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 30, 2016, 4:16 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45489/
> ---
> 
> (Updated March 30, 2016, 4:16 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Casting from a virtual base class to a child class should not be
> done with reinterpret_cast. We could use dynamic_cast as well, but
> that has a performance cost; in this case, we know exactly which
> child class the pointer points to, so the performance cost and
> additional safety offered by dynamic_cast is not necessary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> feaffa4334422ec3964f8d079f570061eaf390d2 
> 
> Diff: https://reviews.apache.org/r/45489/diff/
> 
> 
> Testing
> ---
> 
> make check with GCC 5.3 and recent apple-clang.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45500: Fix Mesos-1104, linux/fs.hpp remove mesos::internal:: should be enough.

[Deshi Xiao]

> On 三月 30, 2016, 6:40 p.m., Cong Wang wrote:
> > You must be joking when you say you tested this Linux change on your osx...
> 
> Deshi Xiao wrote:
> Oops. let me testing it again on centos.

Wang Cong, are u a ex-redhatter? if u have alternative linux, please help 
testing with the changes. thanks a lot.


- Deshi


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


On 三月 30, 2016, 6:47 p.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45500/
> ---
> 
> (Updated 三月 30, 2016, 6:47 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Cong Wang.
> 
> 
> Bugs: MESOS-1104
> https://issues.apache.org/jira/browse/MESOS-1104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> other exists code under src/linux/, e.g. perf.hpp, ns.hpp
> I think remove mesos::internal:: should be enough.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
> 
> Diff: https://reviews.apache.org/r/45500/diff/
> 
> 
> Testing
> ---
> 
> make check on localhost(centos)
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 45408: Introduced a `reconnect` method on the scheduler library.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 30, 2016, 6:44 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45408/
> ---
> 
> (Updated March 30, 2016, 6:44 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4950
> https://issues.apache.org/jira/browse/MESOS-4950
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a `reconnect` method that allows schedulers
> to force a reconnection with the master. This is useful when
> the scheduler detects that there is a one-way partition with
> the master (e.g., lack of `HEARTBEAT` events) and wants to
> trigger a reconnection rather than relying on the `disconnected`
> callback.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp 765935e97b6c1686ab464a5cf1cf2dfd816f51f1 
>   src/scheduler/scheduler.cpp 56476c3fc99b954b146dc28251541d9760a5e917 
> 
> Diff: https://reviews.apache.org/r/45408/diff/
> 
> 
> Testing
> ---
> 
> make check (added a test later in the review chain)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45409: Added test for `reconnect` functionality.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 30, 2016, 6:44 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45409/
> ---
> 
> (Updated March 30, 2016, 6:44 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4950
> https://issues.apache.org/jira/browse/MESOS-4950
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a trivial test for testing the `reconnect`
> method on the scheduler library.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 917058f4dcf32ddaaeda8a3ff21898571f4829dd 
> 
> Diff: https://reviews.apache.org/r/45409/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45409: Added test for `reconnect` functionality.

[Anand Mazumdar]

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

(Updated March 30, 2016, 6:44 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

This change adds a trivial test for testing the `reconnect`
method on the scheduler library.


Diffs (updated)
-

  src/tests/scheduler_tests.cpp 917058f4dcf32ddaaeda8a3ff21898571f4829dd 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 45410: Documented when to invoke `send` using the scheduler library.

[Anand Mazumdar]

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

(Updated March 30, 2016, 6:45 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Added review comments from r45408 here.


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


Repository: mesos


Description
---

Trivial change to add a comment to invoke `send` only after
receiving the `connected` callback. If not, all calls would
be dropped while disconnected.


Diffs (updated)
-

  include/mesos/v1/scheduler.hpp 765935e97b6c1686ab464a5cf1cf2dfd816f51f1 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 45408: Introduced a `reconnect` method on the scheduler library.

[Anand Mazumdar]

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

(Updated March 30, 2016, 6:44 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

This change adds a `reconnect` method that allows schedulers
to force a reconnection with the master. This is useful when
the scheduler detects that there is a one-way partition with
the master (e.g., lack of `HEARTBEAT` events) and wants to
trigger a reconnection rather than relying on the `disconnected`
callback.


Diffs (updated)
-

  include/mesos/v1/scheduler.hpp 765935e97b6c1686ab464a5cf1cf2dfd816f51f1 
  src/scheduler/scheduler.cpp 56476c3fc99b954b146dc28251541d9760a5e917 

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


Testing
---

make check (added a test later in the review chain)


Thanks,

Anand Mazumdar

Re: Review Request 45500: Fix Mesos-1104, linux/fs.hpp remove mesos::internal:: should be enough.

[Deshi Xiao]

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

(Updated 三月 30, 2016, 6:44 p.m.)


Review request for mesos and haosdent huang.


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


Repository: mesos


Description
---

other exists code under src/linux/, e.g. perf.hpp, ns.hpp
I think remove mesos::internal:: should be enough.


Diffs
-

  src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 

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


Testing (updated)
---

make check on localhost(centos)


Thanks,

Deshi Xiao

Re: Review Request 45500: Fix Mesos-1104, linux/fs.hpp remove mesos::internal:: should be enough.

[Deshi Xiao]

> On 三月 30, 2016, 6:40 p.m., Cong Wang wrote:
> > You must be joking when you say you tested this Linux change on your osx...

Oops. let me testing it again on centos.


- Deshi


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


On 三月 30, 2016, 6:34 p.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45500/
> ---
> 
> (Updated 三月 30, 2016, 6:34 p.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Bugs: MESOS-1104
> https://issues.apache.org/jira/browse/MESOS-1104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> other exists code under src/linux/, e.g. perf.hpp, ns.hpp
> I think remove mesos::internal:: should be enough.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
> 
> Diff: https://reviews.apache.org/r/45500/diff/
> 
> 
> Testing
> ---
> 
> make check on mac_osx 11
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 45500: Fix Mesos-1104, linux/fs.hpp remove mesos::internal:: should be enough.

[Cong Wang]

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



You must be joking when you say you tested this Linux change on your osx...

- Cong Wang


On March 30, 2016, 6:34 p.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45500/
> ---
> 
> (Updated March 30, 2016, 6:34 p.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Bugs: MESOS-1104
> https://issues.apache.org/jira/browse/MESOS-1104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> other exists code under src/linux/, e.g. perf.hpp, ns.hpp
> I think remove mesos::internal:: should be enough.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
> 
> Diff: https://reviews.apache.org/r/45500/diff/
> 
> 
> Testing
> ---
> 
> make check on mac_osx 11
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Review Request 45500: Fix Mesos-1104, linux/fs.hpp remove mesos::internal:: should be enough.

[Deshi Xiao]

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

Review request for mesos and haosdent huang.


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


Repository: mesos


Description
---

other exists code under src/linux/, e.g. perf.hpp, ns.hpp
I think remove mesos::internal:: should be enough.


Diffs
-

  src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 

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


Testing
---

make check on mac_osx 11


Thanks,

Deshi Xiao

Re: Review Request 45480: Replace NULL with nullptr.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45480]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 30, 2016, 11:07 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45480/
> ---
> 
> (Updated March 30, 2016, 11:07 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3243
> https://issues.apache.org/jira/browse/MESOS-3243
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> RUN: `run-clang-tidy.py src -fix -checks="-*,modernize-use-nullptr" -p build 
> -fix`
> 
> 
> Diffs
> -
> 
>   src/authentication/cram_md5/authenticatee.cpp 
> 7d4994333428e5d8aa9ce6b85e0a7f42a0ddc4c8 
>   src/authentication/cram_md5/auxprop.cpp 
> d82d2d2f7793859772d89cc91bee09240624c613 
>   src/authorizer/local/authorizer.cpp 
> 0f0d9276337858984f0b19a82ffca74ee84dc650 
>   src/examples/balloon_executor.cpp 399218478a596387814d123290eeeb33102fad7a 
>   src/examples/example_module_impl.cpp 
> aeffdc0e95fa7badc8cfa09e2b57462e427fec55 
>   src/examples/load_generator_framework.cpp 
> b22a09e1749d4b523addacad99858d7b6bde3403 
>   src/examples/test_allocator_module.cpp 
> 1255a4a22a9d1a5724bef78dfc4dee498ed50fff 
>   src/examples/test_anonymous_module.cpp 
> dd291cff3b5d47337e371cd2c1082fd6716af3fc 
>   src/examples/test_authorizer_module.cpp 
> 19ec7cd114562f74c660b83b39235127d25001ee 
>   src/examples/test_container_logger_module.cpp 
> 76dd494fa4c32514ba14b1f4498b588ac9051b4d 
>   src/examples/test_http_authenticator_module.cpp 
> 8affdb60ae6336a2be5218c7bd04a4d5efeeb3f4 
>   src/examples/test_isolator_module.cpp 
> a4a2103b1e449837b95948c8b5c25e05c5d13860 
>   src/examples/test_qos_controller_module.cpp 
> f382fc443fa0a4545ba78dc28016e4d0a0dd16b1 
>   src/examples/test_resource_estimator_module.cpp 
> 229b2931e9129c194850f04051060d33bfa06570 
>   src/java/jni/convert.cpp f1a486d8d2f6d43efd89407c928734bd9715e591 
>   src/java/jni/org_apache_mesos_Log.cpp 
> 140b950136417eed7cba363a89537ed2f33832ff 
>   src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp 
> 9a92ade5d54bd814353acb40170d7aa4d8fe4a77 
>   src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
> c723e2d7bce98e6ac5ab587ce8c42fa9ba8b47e4 
>   src/java/jni/org_apache_mesos_state_AbstractState.cpp 
> 4fd43ca31c6917e81ea1b331b8507ca42a2249cf 
>   src/java/jni/org_apache_mesos_state_Variable.cpp 
> 7a69b3d878e6eaa87bfaf56c0500e11efe4ac44d 
>   src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp 
> 23a89c3e5c5309a8a9b45c168fb61b12d89db1ec 
>   src/jvm/jvm.cpp 779f8b987e2673ca5ab924caf00eea09a0a79af5 
>   src/launcher/executor.cpp 4e9b4d9820f7c2f4cb1b3e16e2f4c8c13500693f 
>   src/linux/fs.cpp dbf94759b6382bfafd3f3b8b4c2047af36a53ad5 
>   src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa 
>   src/log/leveldb.cpp ba2f62bc97e4a0eaa8a54824551686467db4b9e4 
>   src/log/log.cpp a37676068dae14b1adc61ef75e2742c16e7a6e42 
>   src/log/tool/benchmark.cpp 770c6d85fec76826ca8369b2afc721c07899e32f 
>   src/log/tool/initialize.cpp bd1e9ef1922ae972a5999b6e7412e08eac92c1ac 
>   src/log/tool/read.cpp b9e90e44c8cd7351767e523af338d8c662e0848c 
>   src/log/tool/replica.cpp 49415821a32960c78192b89f9a0f2067b9157a63 
>   src/master/contender.cpp 846f516edcc63affd05158e9e5235e6d97f67f15 
>   src/master/main.cpp 58561cffa440aaf1293e9ffe19b5685e6d2f1952 
>   src/master/validation.cpp 9c9e42283baa6e49d86af2ce7222131ce53ccaff 
>   src/slave/container_loggers/logrotate.cpp 
> 7d36c052ff7a180b45ca265fb7ff4c6900d98d64 
>   src/slave/qos_controllers/load.cpp dd44f9209ad283bfea95f16a8c1017e309757f23 
>   src/slave/resource_estimators/fixed.cpp 
> c858a48bc137185d1e1e24a20f6b75b0dd7912ff 
>   src/state/leveldb.cpp f925a73856075f08c4af2af0fcaae8e81358667e 
>   src/tests/containerizer/memory_test_helper.cpp 
> 92579a28336b3c15ba7fdbb4a9f769ac7182aeb6 
>   src/tests/environment.cpp 7617e43587cb81104786d06f753f08565a6c2d0a 
>   src/tests/mesos.cpp 77d49cc65e08f040b0d2010cd083928e4ff8b7cd 
>   src/tests/registrar_tests.cpp 39caf9bb950c0b229a66becb039c7a830a18f6bc 
>   src/tests/state_tests.cpp 0b5a6abadc24d16aa0e5e5f2f4b0c9f524c399ec 
>   src/tests/zookeeper_test_server.cpp 
> 0dc041fef8973d35114b9f76a6a4002853884670 
>   src/zookeeper/group.cpp ca5e99d09825bf5ace7ba59c95266c51e246974f 
>   src/zookeeper/zookeeper.cpp 02fa158a37ddaf045bdf6403c134d03a02c74833 
> 
> Diff: https://reviews.apache.org/r/45480/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 44945: Add autoconf tests for XFS project quotas.

[Jiang Yan Xu]

> On March 25, 2016, 11:43 p.m., Jiang Yan Xu wrote:
> > configure.ac, lines 957-960
> > 
> >
> > Can we put AC_MSG_CHECKING before `AC_CHECK_HEADERS` and inside `AS_IF` 
> > above?
> > 
> > i.e., AC_MSG_CHECKING/AC_MSG_CHECKING/AC_MSG_ERROR inform user what 
> > they don't already know (whether the system dependencies are met), not what 
> > they already know (the fact that they provided the 
> > `--enable_xfs_disk_isolator` flag).
> > 
> > And this check is conditional: we only print `AC_MSG_CHECKING` and do 
> > these checks when `test "x$enable_xfs_disk_isolator" = "xyes"`; we print 
> > `AC_MSG_ERROR([...])` when such checks fail and `AC_MSG_RESULT([yes])` when 
> > all checks succeed.
> > 
> > Would this work?
> > 
> > If we do this, then the message could be `AC_MSG_CHECKING([whether we 
> > can enable the XFS disk isolator])`
> 
> James Peach wrote:
> No, this message should be outside the check so that the builder always 
> knows what happened, independent of what configure flags they passed. 
> Typically, people start by not passing any flags, and it is helpful to 
> explicitly log that a feature was not enabled. Even if you pass the --enable 
> flags, it is comforting to get an explicit confirmation that the feature you 
> asked for is enabled.
> 
> Jiang Yan Xu wrote:
> I agree that explicitly confirming that the user has enabled something is 
> better. 
> 
> I still don't think we should print it after we've done all the 
> dependency checks for it and have potentially aborted configure with error 
> messages but without telling the error is due to "whether to enable the XFS 
> disk isolator..."
> 
> i.e., the following is better.
> 
> ```
> checking whether to enable the XFS disk isolator... yes
> 
> OR 
> 
> checking whether to enable the XFS disk isolator... missing libblkid 
> headers
> ---
> Please install the libblkid development package.
> ---
> 
> OR
> 
> checking whether to enable the XFS disk isolator... no / checking whether 
> to enable the XFS disk isolator... not enabled by user
> 
> ```
> 
> Agreed?
> 
> James Peach wrote:
> If you do it this way, you get the headers, etc check output intermingled 
> in the middle of your nice clean feature check message.

I see. Seems like technology is really lacking in this! In the absence of a 
perfect solution, IMO this is better:

checking whether we are asked to enable the XFS disk isolator... yes
checking for xfs/xfs.h... yes
// Move on to other headers

OR

checking whether we are asked to enable the XFS disk isolator... yes
checking for xfs/xfs.h... missing XFS quota headers
---
Please install the Linux kernel headers and xfsprogs development
packages for XFS disk isolator support.
---

OR 
checking whether we are asked to enable the XFS disk isolator... no
// Move on to other feature checking


TBH I haven't seen this style of using AC_MSG_CHECKING to check a user supplied 
flag so I am not convinced it's idiomatic but I do agree with the comforting 
factor. So let's at least make the message honest about what it's doing.

The above style still doesn't print anything when it has finished checking all 
of the dependencies for XFS but it's not like autoconf has ever done a good job 
of this: this is lack of information.

At least we are not printing all these header checking messages and potentially 
fail before we print "checking whether to enable the XFS disk isolator", which 
implies could be misleading.


- Jiang Yan


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


On March 29, 2016, 1:55 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44945/
> ---
> 
> (Updated March 29, 2016, 1:55 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
> https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add autoconf tests for XFS project quotas.
> 
> 
> Diffs
> -
> 
>   configure.ac 9ec4bc1cff3b0b46dd2e7ece2c1f2d19ffb8 
> 
> Diff: https://reviews.apache.org/r/44945/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual verification.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45471: Fix a test case bug due to upgrade http-parser to 2.6.1.

[Vinod Kone]

> On March 30, 2016, 6:10 p.m., Greg Mann wrote:
> > This could break existing clients which include extra whitespace, though it 
> > does seem that RFC-7230 specifically prohibits whitespace in that location. 
> > I wonder if a deprecation cycle is advisable for this change?
> > 
> > Also, it's not immediately clear to me: are we using http-parser in 
> > "strict" mode?

Good point. Though I would be surprised if typical clients use colons after 
header names. IIIUC (zhiwei can correct me if i'm wrong) 2.6.1 disallowed 
spaces before colon irrespective of strict mode (which was a bug). I think they 
fixed the behavior in 2.6.2 so that it rejects spaces iff the mode is strict. 
If that's the case, we should probably just upgrade to 2.6.2 instead of 2.6.1?


- Vinod


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


On March 30, 2016, 5:35 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45471/
> ---
> 
> (Updated March 30, 2016, 5:35 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-5063
> https://issues.apache.org/jira/browse/MESOS-5063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix a test case bug due to upgrade http-parser to 2.6.1.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 53a0ed03226030104af43a45c6ccfbfc0f4b7e9f 
> 
> Diff: https://reviews.apache.org/r/45471/diff/
> 
> 
> Testing
> ---
> 
> ../configure --enable-libevent --enable-ssl
> 
> sudo make dist check
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45383: Implemented recover() method of "network/cni" isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 322)


`s/recoverInfo/_recover/`

We usually move `_xxx` right below `xxx` for readability. So please move 
the function to the correct place.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 324)


I think this is not needed because if recover fails, slave will restart.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 333)


Ditto.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 350)


Ditto.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 840)


OK, now I realized that `getNetworkInfoDir` is a little confusing with 
`getNetworkDir`. Can we `s/getNetworkInfoDir/getContainerDir/` in a subsequent 
patch?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 858 - 859)


Please align the parameter properly.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 862)


`networkDirs->empty()`



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 863 - 866)


Or agent crashes right before removing the containerDir?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 867 - 884)


This basically duplicates what's in the cleanup. I am wondering if we 
should make cleanup more robust to tolerate a container dir without network 
name dirs. In other words, remove the logic here, change cleanup so that it can 
handle empty network dirs case.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 889)


instead of that, can you instead
```
s/getNetworkDirs/getNetworkNames/
```

In other words, return a list of names, rather than  dirs.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 894)


Ditto.

s/getInterfaceDirs/getInterfaces/


- Jie Yu


On March 30, 2016, 1:39 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45383/
> ---
> 
> (Updated March 30, 2016, 1:39 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 873e0c52475f4868e611bd24a6782ad5eb261a99 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 1c8e231813c0579b79681c5d18b1f799a727ead7 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
> f627ec9499a34ca104d2c1a4d28e1d2f4b849f64 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
> 611f3869402b9033081b7f9ecc1bdf006f61918b 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
> 6a3c33645bab73edaf5af4d298a671852ea59c46 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
> 5b5f904def9ef6dcc4462a03a2d024ad4eb3d946 
> 
> Diff: https://reviews.apache.org/r/45383/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45466: Removed the redundant `NULL` check when deleting `Credential`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 30, 2016, 2:23 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45466/
> ---
> 
> (Updated March 30, 2016, 2:23 a.m.)
> 
> 
> Review request for mesos, Klaus Ma, Neil Conway, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary. Also moved all `delete` statements together in the order they 
> were declared in `.hpp` file.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 701e024ac57fc6e2f96d7d83e65cb68252d41654 
> 
> Diff: https://reviews.apache.org/r/45466/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45471: Fix a test case bug due to upgrade http-parser to 2.6.1.

[Greg Mann]

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



This could break existing clients which include extra whitespace, though it 
does seem that RFC-7230 specifically prohibits whitespace in that location. I 
wonder if a deprecation cycle is advisable for this change?

Also, it's not immediately clear to me: are we using http-parser in "strict" 
mode?

- Greg Mann


On March 30, 2016, 5:35 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45471/
> ---
> 
> (Updated March 30, 2016, 5:35 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-5063
> https://issues.apache.org/jira/browse/MESOS-5063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix a test case bug due to upgrade http-parser to 2.6.1.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 53a0ed03226030104af43a45c6ccfbfc0f4b7e9f 
> 
> Diff: https://reviews.apache.org/r/45471/diff/
> 
> 
> Testing
> ---
> 
> ../configure --enable-libevent --enable-ssl
> 
> sudo make dist check
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45015: Windows: Fixed bug causing `os::exists` to report invalid paths exist.

[Michael Park]

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




3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp (lines 144 - 
166)


I thought we concluded that actions such as `os::mkdir` and `os::touch` 
should be `ASSERT_*`, and the things that are being actually being tested here, 
i.e. `os::exists` should be `EXPECT_*`?


- Michael Park


On March 24, 2016, 7:58 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45015/
> ---
> 
> (Updated March 24, 2016, 7:58 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently on Windows, `os::exists` will return true if a component of a
> path does not exist. For example if you have `a/fancy/path`, and you ask
> `os::exists("a/fake/path")`, the result currently reports `true`. In
> other words, the Windows code path only checks for the error that a file
> does not exist, and ignores the error that says the path is not valid.
> 
> This commit will fix this, and also add a test that will verify we don't
> regress.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp 
> 9211851e4562e04045276421b359c3c78cdae7f1 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> 4c30189bb8261ccfc699da0f31b8b1fd3e9b3c83 
> 
> Diff: https://reviews.apache.org/r/45015/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 45471: Fix a test case bug due to upgrade http-parser to 2.6.1.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 30, 2016, 5:35 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45471/
> ---
> 
> (Updated March 30, 2016, 5:35 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-5063
> https://issues.apache.org/jira/browse/MESOS-5063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix a test case bug due to upgrade http-parser to 2.6.1.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 53a0ed03226030104af43a45c6ccfbfc0f4b7e9f 
> 
> Diff: https://reviews.apache.org/r/45471/diff/
> 
> 
> Testing
> ---
> 
> ../configure --enable-libevent --enable-ssl
> 
> sudo make dist check
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45484: Made CHANGELOG formatting more consistent.

[Vinod Kone]

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


Ship it!




Thanks for the cleanup!

- Vinod Kone


On March 30, 2016, 12:36 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45484/
> ---
> 
> (Updated March 30, 2016, 12:36 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We follow a new format starting from 0.28. To avoid confusion
> and remove precedents of alternate styles, this cleans up
> formatting for previous releases.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 3cbbf9099948d043d68cd3db5e63c25403fa00f9 
> 
> Diff: https://reviews.apache.org/r/45484/diff/
> 
> 
> Testing
> ---
> 
> None: Not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 45496: Added documentation of LIBPROCESS_NUM_WORKER_THREADS.

[Maged Michael]

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

Review request for mesos.


Repository: mesos


Description
---

Added documentation of LIBPROCESS_NUM_WORKER_THREADS. Documentation related to 
https://reviews.apache.org/r/43144/ for 
https://issues.apache.org/jira/browse/MESOS-4353.


Diffs
-

  docs/configuration.md 75c9a0ae7d63ff8b4337778aa7bb28676a1b9f56 

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


Testing
---


Thanks,

Maged Michael

Re: Review Request 45381: Migrate /monitor/statistics and /monitor/statistics.json to slave.

[Jay Guo]

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

(Updated March 30, 2016, 5:30 p.m.)


Review request for mesos and Jie Yu.


Changes
---

As requested, `ResourceStatistics` collection logic is moved to slave and 
`ResourceMonitor` is deleted.


Summary (updated)
-

Migrate /monitor/statistics and /monitor/statistics.json to slave.


Repository: mesos


Description (updated)
---

These two endpoints and their underlying logics are moved from
ResourceMonitorProcess to slave process. ResourceMonitor is removed.


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/slave/http.cpp 7a872c6e21b115500c5291b7cb9fb240ec9dc8ed 
  src/slave/monitor.hpp 70a7c88fefd4577f53f85e3eccdd6b69ab6f3acd 
  src/slave/monitor.cpp 5c1dd354595e67e5eb603c5d49850f84b84b 
  src/slave/slave.hpp 3ba335fcd31a92af9609023daae328b7f4bf5e59 
  src/slave/slave.cpp fc77f594d16a9fb2ca001e089d74e2c0ffeb5baa 
  src/tests/monitor_tests.cpp 5dcb2481ff2f1a7caf54036bc3e60c78feb982b1 
  src/tests/oversubscription_tests.cpp ba036810758d99a6fb0034c5e2bc7829e2343a44 
  src/tests/slave_tests.cpp 57fc50360eae85819ae6ce714b0c3c4c1867b2b8 

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


Testing
---

make check

https://issues.apache.org/jira/browse/MESOS-4891


Thanks,

Jay Guo

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

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

(Updated March 30, 2016, 5:12 p.m.)


Review request for mesos, Guangya Liu, Joris Van Remoortere, Klaus Ma, and Qian 
Zhang.


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


Repository: mesos


Description
---

Added env var to set upper bound on number of libprocess worker threads.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp feaffa4 

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


Testing
---

Passed make check on x86_64 Ubuntu 14.04.

Invocations of mesos-master, mesos-slave, and mesos-tests with the 
LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
"-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
The last 3 tests generated warnings.


Thanks,

Maged Michael

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

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

(Updated March 30, 2016, 5:10 p.m.)


Review request for mesos, Guangya Liu, Joris Van Remoortere, Klaus Ma, and Qian 
Zhang.


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


Repository: mesos


Description
---

Added env var to set upper bound on number of libprocess worker threads.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp feaffa4 
  docs/configuration.md 9ad0c2a 

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


Testing
---

Passed make check on x86_64 Ubuntu 14.04.

Invocations of mesos-master, mesos-slave, and mesos-tests with the 
LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
"-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
The last 3 tests generated warnings.


Thanks,

Maged Michael

Re: Review Request 45082: Implemented cleanup() method of "network/cni" isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 625)


space after `foreachkey`. Please fix all occurances.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 629)


Should we use 'await' here to make sure all plugin DEL finishes/failed 
before returnning result? You can do:
```
return await(futures)
  .then(_cleanup);
```

No need for onAny here since await is guaranteed to have a suceeded future.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 631)


Please assign the parameters here. You're off by 1 space.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 685)


You can use `s->status()` and `s->out().get()` here.

ALso, no need for process:: namespace prefix.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 739)


I think in `_cleanup`, I think if any of the plugin DEL succeeds, we should 
remove it's corresponding directory so that we don't retry DEL again. 
Otherwise, we should not so that during recover, we can retry it's deletion.

I think we should not umount unless all plugin DEL succeeds.


- Jie Yu


On March 30, 2016, 8:36 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45082/
> ---
> 
> (Updated March 30, 2016, 8:36 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented cleanup() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 873e0c52475f4868e611bd24a6782ad5eb261a99 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 1c8e231813c0579b79681c5d18b1f799a727ead7 
> 
> Diff: https://reviews.apache.org/r/45082/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45419: Cleaned up ModuleManager.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45419]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 29, 2016, 3:41 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45419/
> ---
> 
> (Updated March 29, 2016, 3:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removes unused includes and usings from the ModuleManager.
> 
> 
> Diffs
> -
> 
>   src/module/manager.hpp 9944af0 
>   src/module/manager.cpp 8c9aaf7 
> 
> Diff: https://reviews.apache.org/r/45419/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 45489: Replaced reinterpret_cast with static_cast in libprocess.

[Neil Conway]

> On March 30, 2016, 3:03 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/src/process.cpp, line 1531
> > 
> >
> > Here and below: Either explicitly assert that the `dynamic_cast` does 
> > not return `NULL`, or use `static_cast`.
> > 
> > Since `Encoder` seems to implement its own idea of polymorphism it 
> > seems `static_cast` might fit better.
> 
> Neil Conway wrote:
> Hmmm -- I thought `static_cast` was only safe for downcasts when the 
> parent class is non-virtual?
> 
> Benjamin Bannier wrote:
> It is fine for most cases, but cannot be used e.g., in the presence of 
> virtual *inheritance* (which isn't used here).

Double-checked with mpark -- sounds good. Thanks!


- Neil


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


On March 30, 2016, 4:16 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45489/
> ---
> 
> (Updated March 30, 2016, 4:16 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Casting from a virtual base class to a child class should not be
> done with reinterpret_cast. We could use dynamic_cast as well, but
> that has a performance cost; in this case, we know exactly which
> child class the pointer points to, so the performance cost and
> additional safety offered by dynamic_cast is not necessary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> feaffa4334422ec3964f8d079f570061eaf390d2 
> 
> Diff: https://reviews.apache.org/r/45489/diff/
> 
> 
> Testing
> ---
> 
> make check with GCC 5.3 and recent apple-clang.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45489: Replaced reinterpret_cast with static_cast in libprocess.

[Neil Conway]

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

(Updated March 30, 2016, 4:16 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Use static_cast instead of dynamic_cast.


Summary (updated)
-

Replaced reinterpret_cast with static_cast in libprocess.


Repository: mesos


Description (updated)
---

Casting from a virtual base class to a child class should not be
done with reinterpret_cast. We could use dynamic_cast as well, but
that has a performance cost; in this case, we know exactly which
child class the pointer points to, so the performance cost and
additional safety offered by dynamic_cast is not necessary.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp feaffa4334422ec3964f8d079f570061eaf390d2 

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


Testing (updated)
---

make check with GCC 5.3 and recent apple-clang.


Thanks,

Neil Conway

Re: Review Request 45489: Replaced reinterpret_cast with dynamic_cast in libprocess.

[Benjamin Bannier]

> On March 30, 2016, 5:03 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/src/process.cpp, line 1531
> > 
> >
> > Here and below: Either explicitly assert that the `dynamic_cast` does 
> > not return `NULL`, or use `static_cast`.
> > 
> > Since `Encoder` seems to implement its own idea of polymorphism it 
> > seems `static_cast` might fit better.
> 
> Neil Conway wrote:
> Hmmm -- I thought `static_cast` was only safe for downcasts when the 
> parent class is non-virtual?

It is fine for most cases, but cannot be used e.g., in the presence of virtual 
*inheritance* (which isn't used here).


- Benjamin


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


On March 30, 2016, 4:49 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45489/
> ---
> 
> (Updated March 30, 2016, 4:49 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Casting from a parent class to a child class should not
> be done with reinterpret_cast.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> feaffa4334422ec3964f8d079f570061eaf390d2 
> 
> Diff: https://reviews.apache.org/r/45489/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45474: MESOS-1739: Allow slave reconfiguration on restart, Phase 1.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45474]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 30, 2016, 8:13 a.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45474/
> ---
> 
> (Updated March 30, 2016, 8:13 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, haosdent huang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1739
> https://issues.apache.org/jira/browse/MESOS-1739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Phase 1
> Make SlaveInfo mutable throughout the stack, and allow for expansion of 
> resources and attributes only (Which allows testing to make sure it 
> propagates to the allocator, shows up in offers, etc). Ensure there is 
> unified checking for incompatibilities in both the slave and master (the 
> slave should validate the config, the master should validate that all 
> operations the slave takes are legal).
> 
> it derived from another PR(https://reviews.apache.org/r/25525/)
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1f1a31020096efa5db698e86ac74e61dfdb4b94a 
> 
> Diff: https://reviews.apache.org/r/45474/diff/
> 
> 
> Testing
> ---
> 
> make check on localhost
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 45489: Replaced reinterpret_cast with dynamic_cast in libprocess.

[Neil Conway]

> On March 30, 2016, 3:03 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/src/process.cpp, line 1531
> > 
> >
> > Here and below: Either explicitly assert that the `dynamic_cast` does 
> > not return `NULL`, or use `static_cast`.
> > 
> > Since `Encoder` seems to implement its own idea of polymorphism it 
> > seems `static_cast` might fit better.

Hmmm -- I thought `static_cast` was only safe for downcasts when the parent 
class is non-virtual?


- Neil


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


On March 30, 2016, 2:49 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45489/
> ---
> 
> (Updated March 30, 2016, 2:49 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Casting from a parent class to a child class should not
> be done with reinterpret_cast.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> feaffa4334422ec3964f8d079f570061eaf390d2 
> 
> Diff: https://reviews.apache.org/r/45489/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45270: Added spec protobut for external mount.

[Guangya Liu]

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

(Updated 三月 30, 2016, 3:17 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added spec protobut for external mount.


Diffs (updated)
-

  src/CMakeLists.txt 366a7ae8f6ef1d55202699df0502a30f15a35e1f 
  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/slave/containerizer/mesos/isolators/docker/dvd/spec.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/dvd/spec.proto PRE-CREATION 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 45489: Replaced reinterpret_cast with dynamic_cast in libprocess.

[Benjamin Bannier]

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




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


Here and below: Either explicitly assert that the `dynamic_cast` does not 
return `NULL`, or use `static_cast`.

Since `Encoder` seems to implement its own idea of polymorphism it seems 
`static_cast` might fit better.


- Benjamin Bannier


On March 30, 2016, 4:49 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45489/
> ---
> 
> (Updated March 30, 2016, 4:49 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Casting from a parent class to a child class should not
> be done with reinterpret_cast.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> feaffa4334422ec3964f8d079f570061eaf390d2 
> 
> Diff: https://reviews.apache.org/r/45489/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45319: Cleaned up various code in libprocess.

[Neil Conway]

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

(Updated March 30, 2016, 2:49 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Cleaned up various code in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp feaffa4334422ec3964f8d079f570061eaf390d2 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
e8bbc55f0f9aeabe7612a2ced5299cc01202b1f6 
  3rdparty/libprocess/src/tests/process_tests.cpp 
6b3aa1bcf20466cdf8f8249988b8b06dec27e5cd 

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


Testing
---

make check


Thanks,

Neil Conway

Review Request 45488: Removed an unnecessary `memset` from libprocess.

[Neil Conway]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Zero'ing the input buffer before receiving data is unnecessary.
Moreover, keeping the `memset` around is confusing, because it makes
the API contract of Socket.recv() less clear.


Diffs
-

  3rdparty/libprocess/src/process.cpp feaffa4334422ec3964f8d079f570061eaf390d2 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 45318: Added comments to some libprocess functions.

[Neil Conway]

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

(Updated March 30, 2016, 2:48 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Added another comment.


Summary (updated)
-

Added comments to some libprocess functions.


Repository: mesos


Description (updated)
---

Added comments to some libprocess functions.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp feaffa4334422ec3964f8d079f570061eaf390d2 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 45466: Removed the redundant `NULL` check when deleting `Credential`.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On March 30, 2016, 2:23 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45466/
> ---
> 
> (Updated March 30, 2016, 2:23 a.m.)
> 
> 
> Review request for mesos, Klaus Ma, Neil Conway, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary. Also moved all `delete` statements together in the order they 
> were declared in `.hpp` file.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 701e024ac57fc6e2f96d7d83e65cb68252d41654 
> 
> Diff: https://reviews.apache.org/r/45466/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45383: Implemented recover() method of "network/cni" isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44004, 44200, 44549, 44555, 44622, 44514, 44706, 45082, 45383]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 30, 2016, 1:39 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45383/
> ---
> 
> (Updated March 30, 2016, 1:39 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 873e0c52475f4868e611bd24a6782ad5eb261a99 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 1c8e231813c0579b79681c5d18b1f799a727ead7 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
> f627ec9499a34ca104d2c1a4d28e1d2f4b849f64 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
> 611f3869402b9033081b7f9ecc1bdf006f61918b 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
> 6a3c33645bab73edaf5af4d298a671852ea59c46 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
> 5b5f904def9ef6dcc4462a03a2d024ad4eb3d946 
> 
> Diff: https://reviews.apache.org/r/45383/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45383: Implemented recover() method of "network/cni" isolator.

[Qian Zhang]

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

(Updated March 30, 2016, 9:39 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


Repository: mesos


Description
---

Implemented recover() method of "network/cni" isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
873e0c52475f4868e611bd24a6782ad5eb261a99 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
1c8e231813c0579b79681c5d18b1f799a727ead7 
  src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
f627ec9499a34ca104d2c1a4d28e1d2f4b849f64 
  src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
611f3869402b9033081b7f9ecc1bdf006f61918b 
  src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
6a3c33645bab73edaf5af4d298a671852ea59c46 
  src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
5b5f904def9ef6dcc4462a03a2d024ad4eb3d946 

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Joris Van Remoortere]

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



Can you please split the review between the code update and the docs? The git 
commit hooks prevent committing accross projects (in this case Libprocess, 
Mesos).
I would do it for you, but since this is your first commit I don't have an 
author to reference for committing on your behalf ;-)


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


s/system defined/default



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


/system defined/default



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


New line after closing brace `}`.
space between operands `num_worker_threads + 1` (I know it was incorrect 
before)


- Joris Van Remoortere


On March 30, 2016, 12:07 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated March 30, 2016, 12:07 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Joris Van Remoortere, Klaus Ma, and 
> Qian Zhang.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp feaffa4 
>   docs/configuration.md 9ad0c2a 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 45434: Removed unused credentials.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45434]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 30, 2016, 8:20 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45434/
> ---
> 
> (Updated March 30, 2016, 8:20 a.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/files_tests.cpp 2f0f84cd78e7f382b70b97c8711f71cea64ce0a5 
> 
> Diff: https://reviews.apache.org/r/45434/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 45484: Made CHANGELOG formatting more consistent.

[Alexander Rukletsov]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

We follow a new format starting from 0.28. To avoid confusion
and remove precedents of alternate styles, this cleans up
formatting for previous releases.


Diffs
-

  CHANGELOG 3cbbf9099948d043d68cd3db5e63c25403fa00f9 

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


Testing
---

None: Not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 45202: Add test for rescinding offer trriggered by updating weights.

[Adam B]

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


Fix it, then Ship it!




Some minor style suggestions, and then we can commit this too.


src/tests/dynamic_weights_tests.cpp (line 467)


s/UPDATED_WEIGHTS/UPDATED_WEIGHTS1/



src/tests/dynamic_weights_tests.cpp (line 498)


s/UPDATED_WEIGHTS/UPDATED_WEIGHTS1/



src/tests/dynamic_weights_tests.cpp (line 550)


UPDATED_WEIGHTS1



src/tests/dynamic_weights_tests.cpp (line 590)


UPDATED_WEIGHTS1



src/tests/dynamic_weights_tests.cpp (line 622)


UPDATED_WEIGHTS1



src/tests/master_allocator_tests.cpp (line 1519)


Don't you at least know that one of these parameters is the master's pid?



src/tests/master_allocator_tests.cpp (line 1524)


You should be able to assign the string directly with `flags.resources = 
agentResources;` and the Option constructor will handle conversion



src/tests/master_allocator_tests.cpp (lines 1593 - 1595)


Set `using` statements at the beginning of the file so we don't need to 
have the entire namespace here for each class


- Adam B


On March 25, 2016, 2:13 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45202/
> ---
> 
> (Updated March 25, 2016, 2:13 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4881
> https://issues.apache.org/jira/browse/MESOS-4881
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add test for rescinding offer trriggered by updating weights.
> 
> 
> Diffs
> -
> 
>   src/tests/dynamic_weights_tests.cpp 
> 6357cf42ec59c1388e95d6d808978918d5cd4a78 
>   src/tests/master_allocator_tests.cpp 
> b41ba2bda4d680f6fc42f525719973d56c11fe31 
>   src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
> 
> Diff: https://reviews.apache.org/r/45202/diff/
> 
> 
> Testing
> ---
> 
> make && make check.
> 
> $ ./src/mesos-tests 
> --gtest_filter=MasterAllocatorTest/0.RebalancedForUpdatedWeights
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from MasterAllocatorTest/0, where TypeParam = 
> mesos::internal::master::allocator::MesosAllocator  mesos::internal::master::allocator::DRFSorter> >
> [ RUN  ] MasterAllocatorTest/0.RebalancedForUpdatedWeights
> [   OK ] MasterAllocatorTest/0.RebalancedForUpdatedWeights (1059 ms)
> [--] 1 test from MasterAllocatorTest/0 (1059 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (1070 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Review Request 45480: Replace NULL with nullptr.

[Tomasz Janiszewski]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

RUN: `run-clang-tidy.py src -fix -checks="-*,modernize-use-nullptr" -p build 
-fix`


Diffs
-

  src/authentication/cram_md5/authenticatee.cpp 
7d4994333428e5d8aa9ce6b85e0a7f42a0ddc4c8 
  src/authentication/cram_md5/auxprop.cpp 
d82d2d2f7793859772d89cc91bee09240624c613 
  src/authorizer/local/authorizer.cpp 0f0d9276337858984f0b19a82ffca74ee84dc650 
  src/examples/balloon_executor.cpp 399218478a596387814d123290eeeb33102fad7a 
  src/examples/example_module_impl.cpp aeffdc0e95fa7badc8cfa09e2b57462e427fec55 
  src/examples/load_generator_framework.cpp 
b22a09e1749d4b523addacad99858d7b6bde3403 
  src/examples/test_allocator_module.cpp 
1255a4a22a9d1a5724bef78dfc4dee498ed50fff 
  src/examples/test_anonymous_module.cpp 
dd291cff3b5d47337e371cd2c1082fd6716af3fc 
  src/examples/test_authorizer_module.cpp 
19ec7cd114562f74c660b83b39235127d25001ee 
  src/examples/test_container_logger_module.cpp 
76dd494fa4c32514ba14b1f4498b588ac9051b4d 
  src/examples/test_http_authenticator_module.cpp 
8affdb60ae6336a2be5218c7bd04a4d5efeeb3f4 
  src/examples/test_isolator_module.cpp 
a4a2103b1e449837b95948c8b5c25e05c5d13860 
  src/examples/test_qos_controller_module.cpp 
f382fc443fa0a4545ba78dc28016e4d0a0dd16b1 
  src/examples/test_resource_estimator_module.cpp 
229b2931e9129c194850f04051060d33bfa06570 
  src/java/jni/convert.cpp f1a486d8d2f6d43efd89407c928734bd9715e591 
  src/java/jni/org_apache_mesos_Log.cpp 
140b950136417eed7cba363a89537ed2f33832ff 
  src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp 
9a92ade5d54bd814353acb40170d7aa4d8fe4a77 
  src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
c723e2d7bce98e6ac5ab587ce8c42fa9ba8b47e4 
  src/java/jni/org_apache_mesos_state_AbstractState.cpp 
4fd43ca31c6917e81ea1b331b8507ca42a2249cf 
  src/java/jni/org_apache_mesos_state_Variable.cpp 
7a69b3d878e6eaa87bfaf56c0500e11efe4ac44d 
  src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp 
23a89c3e5c5309a8a9b45c168fb61b12d89db1ec 
  src/jvm/jvm.cpp 779f8b987e2673ca5ab924caf00eea09a0a79af5 
  src/launcher/executor.cpp 4e9b4d9820f7c2f4cb1b3e16e2f4c8c13500693f 
  src/linux/fs.cpp dbf94759b6382bfafd3f3b8b4c2047af36a53ad5 
  src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa 
  src/log/leveldb.cpp ba2f62bc97e4a0eaa8a54824551686467db4b9e4 
  src/log/log.cpp a37676068dae14b1adc61ef75e2742c16e7a6e42 
  src/log/tool/benchmark.cpp 770c6d85fec76826ca8369b2afc721c07899e32f 
  src/log/tool/initialize.cpp bd1e9ef1922ae972a5999b6e7412e08eac92c1ac 
  src/log/tool/read.cpp b9e90e44c8cd7351767e523af338d8c662e0848c 
  src/log/tool/replica.cpp 49415821a32960c78192b89f9a0f2067b9157a63 
  src/master/contender.cpp 846f516edcc63affd05158e9e5235e6d97f67f15 
  src/master/main.cpp 58561cffa440aaf1293e9ffe19b5685e6d2f1952 
  src/master/validation.cpp 9c9e42283baa6e49d86af2ce7222131ce53ccaff 
  src/slave/container_loggers/logrotate.cpp 
7d36c052ff7a180b45ca265fb7ff4c6900d98d64 
  src/slave/qos_controllers/load.cpp dd44f9209ad283bfea95f16a8c1017e309757f23 
  src/slave/resource_estimators/fixed.cpp 
c858a48bc137185d1e1e24a20f6b75b0dd7912ff 
  src/state/leveldb.cpp f925a73856075f08c4af2af0fcaae8e81358667e 
  src/tests/containerizer/memory_test_helper.cpp 
92579a28336b3c15ba7fdbb4a9f769ac7182aeb6 
  src/tests/environment.cpp 7617e43587cb81104786d06f753f08565a6c2d0a 
  src/tests/mesos.cpp 77d49cc65e08f040b0d2010cd083928e4ff8b7cd 
  src/tests/registrar_tests.cpp 39caf9bb950c0b229a66becb039c7a830a18f6bc 
  src/tests/state_tests.cpp 0b5a6abadc24d16aa0e5e5f2f4b0c9f524c399ec 
  src/tests/zookeeper_test_server.cpp 0dc041fef8973d35114b9f76a6a4002853884670 
  src/zookeeper/group.cpp ca5e99d09825bf5ace7ba59c95266c51e246974f 
  src/zookeeper/zookeeper.cpp 02fa158a37ddaf045bdf6403c134d03a02c74833 

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


Testing
---


Thanks,

Tomasz Janiszewski

Review Request 45482: Replace NULL with nullptr.

[Tomasz Janiszewski]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

RUN: find . -name "*.[hc]pp" | xargs -P 4 sed -i 's/\bNULL\b/nullptr/g'


Diffs
-

  include/mesos/module.hpp 30a9674686221d25132368e6c10664daa9cd6dc0 
  src/authentication/cram_md5/authenticatee.cpp 
7d4994333428e5d8aa9ce6b85e0a7f42a0ddc4c8 
  src/authentication/cram_md5/authenticator.cpp 
027eba4f433fff328d68c41005bc59c41c7ae668 
  src/authentication/cram_md5/auxprop.cpp 
d82d2d2f7793859772d89cc91bee09240624c613 
  src/authorizer/local/authorizer.cpp 0f0d9276337858984f0b19a82ffca74ee84dc650 
  src/examples/balloon_executor.cpp 399218478a596387814d123290eeeb33102fad7a 
  src/examples/example_module_impl.cpp aeffdc0e95fa7badc8cfa09e2b57462e427fec55 
  src/examples/load_generator_framework.cpp 
b22a09e1749d4b523addacad99858d7b6bde3403 
  src/examples/test_allocator_module.cpp 
1255a4a22a9d1a5724bef78dfc4dee498ed50fff 
  src/examples/test_anonymous_module.cpp 
dd291cff3b5d47337e371cd2c1082fd6716af3fc 
  src/examples/test_authorizer_module.cpp 
19ec7cd114562f74c660b83b39235127d25001ee 
  src/examples/test_container_logger_module.cpp 
76dd494fa4c32514ba14b1f4498b588ac9051b4d 
  src/examples/test_hook_module.cpp abd132b3f39265683542a9d1533d2a31bd81769a 
  src/examples/test_http_authenticator_module.cpp 
8affdb60ae6336a2be5218c7bd04a4d5efeeb3f4 
  src/examples/test_isolator_module.cpp 
a4a2103b1e449837b95948c8b5c25e05c5d13860 
  src/examples/test_qos_controller_module.cpp 
f382fc443fa0a4545ba78dc28016e4d0a0dd16b1 
  src/examples/test_resource_estimator_module.cpp 
229b2931e9129c194850f04051060d33bfa06570 
  src/exec/exec.cpp 8f672602daf090dec032d2b684e407e5d043af9c 
  src/files/files.hpp 90acb3406c46c164108deb559af71fb109a5773b 
  src/java/jni/construct.cpp 0bfe6291e94a69a787c965fa3a3a90d2ebae8d72 
  src/java/jni/convert.cpp f1a486d8d2f6d43efd89407c928734bd9715e591 
  src/java/jni/org_apache_mesos_Log.cpp 
140b950136417eed7cba363a89537ed2f33832ff 
  src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp 
9a92ade5d54bd814353acb40170d7aa4d8fe4a77 
  src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
c723e2d7bce98e6ac5ab587ce8c42fa9ba8b47e4 
  src/java/jni/org_apache_mesos_state_AbstractState.cpp 
4fd43ca31c6917e81ea1b331b8507ca42a2249cf 
  src/java/jni/org_apache_mesos_state_Variable.cpp 
7a69b3d878e6eaa87bfaf56c0500e11efe4ac44d 
  src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp 
23a89c3e5c5309a8a9b45c168fb61b12d89db1ec 
  src/jvm/jvm.hpp d5023e4050d109ca97cedb05f85e1c24202ac3b0 
  src/jvm/jvm.cpp 779f8b987e2673ca5ab924caf00eea09a0a79af5 
  src/jvm/org/apache/zookeeper.hpp 7e9c3aaa9f0eed44994004f9e32ce7ce3cf6d335 
  src/launcher/executor.cpp 4e9b4d9820f7c2f4cb1b3e16e2f4c8c13500693f 
  src/linux/cgroups.cpp b7420c682970c4838e84973198ac4fe7af5f68f9 
  src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
  src/linux/fs.cpp dbf94759b6382bfafd3f3b8b4c2047af36a53ad5 
  src/linux/perf.cpp 749e676aaf2ce639dd976f2b23e323300c6114c5 
  src/linux/routing/diagnosis/diagnosis.cpp 
8b86a8864c08b078425dcc242d323763d6ec15dd 
  src/linux/routing/filter/internal.hpp 
8690bf56472c318e85afb53d35494790ae4da27b 
  src/linux/routing/internal.hpp 8f68119819f7c79ece1a13ac1894b1802ddc8e19 
  src/linux/routing/link/internal.hpp 8f05e5f513208c8f966bc324c9fe994a5807b051 
  src/linux/routing/queueing/internal.hpp 
768ed325f9b259e150779eb3ad74f4e5d4bcc7a2 
  src/linux/routing/route.cpp 4b33350dc3761e19121c29c0d090cfdae544e4ef 
  src/linux/systemd.cpp 9f6e06cfcf6f5b38971ff75eb85326d043140b4b 
  src/local/local.hpp f4ae285edc30a0fb1c960d50dfb1a859b2eae166 
  src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa 
  src/log/leveldb.cpp ba2f62bc97e4a0eaa8a54824551686467db4b9e4 
  src/log/log.cpp a37676068dae14b1adc61ef75e2742c16e7a6e42 
  src/log/tool/benchmark.hpp 3051b6ff025252d2c770b0a70d6a66f4f34d7e69 
  src/log/tool/benchmark.cpp 770c6d85fec76826ca8369b2afc721c07899e32f 
  src/log/tool/initialize.hpp 99c80061442c6379978f907c80a7e9d54a015f04 
  src/log/tool/initialize.cpp bd1e9ef1922ae972a5999b6e7412e08eac92c1ac 
  src/log/tool/read.hpp 937ae8f15f5e2227247ab43aa24c887c13c3cf2c 
  src/log/tool/read.cpp b9e90e44c8cd7351767e523af338d8c662e0848c 
  src/log/tool/replica.hpp c25bb09abc4391614c7a35b9ca91d678a745ffdd 
  src/log/tool/replica.cpp 49415821a32960c78192b89f9a0f2067b9157a63 
  src/logging/logging.cpp 20d2f6341bd39fc5d056f1046d258d006fc602e4 
  src/master/allocator/mesos/hierarchical.hpp 
e979fdf60da1409d1c2d08f0e9f03cef067506dd 
  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/contender.cpp 846f516edcc63affd05158e9e5235e6d97f67f15 
  src/master/http.cpp 03d4ebabfba1bf711fc0897801a989ac3c72f9f1 
  

Re: Review Request 42516: Add support for user-defined networks.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42516]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 30, 2016, 7:52 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 30, 2016, 7:52 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 45419: Cleaned up ModuleManager.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/module/manager.hpp (line 35)


Could you also include `stout/nothing.hpp` here?



src/module/manager.cpp (line 35)


My bad, `stout/stringify.hpp` was actually used here.


- Benjamin Bannier


On March 29, 2016, 5:41 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45419/
> ---
> 
> (Updated March 29, 2016, 5:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removes unused includes and usings from the ModuleManager.
> 
> 
> Diffs
> -
> 
>   src/module/manager.hpp 9944af0 
>   src/module/manager.cpp 8c9aaf7 
> 
> Diff: https://reviews.apache.org/r/45419/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 45473: Migrate test cases for `net_cls` subsystem to cgroups unified isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45083, 45084, 45350, 45085, 45086, 45087, 45351, 45352, 
45353, 45354, 45362, 45363, 45364, 45472, 45473]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 30, 2016, 6:27 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45473/
> ---
> 
> (Updated March 30, 2016, 6:27 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5046
> https://issues.apache.org/jira/browse/MESOS-5046
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Migrate test cases for `net_cls` subsystem to cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> c4e467c8227f9e4129b05d173812592f39a04e06 
> 
> Diff: https://reviews.apache.org/r/45473/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45423: Added note about preventing resource autodetecting to documentation.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On March 29, 2016, 2:02 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45423/
> ---
> 
> (Updated March 29, 2016, 2:02 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When manually specifying disk resources as explained in the multiple-disk
> documentation, Mesos stops autodetecting the Root disk. This can be
> unexpected behavior for users.
> 
> 
> Diffs
> -
> 
>   docs/multiple-disk.md 5c15b64b8b7737159cfba21223a84bdfcac75a0b 
> 
> Diff: https://reviews.apache.org/r/45423/diff/
> 
> 
> Testing
> ---
> 
> Viewed as gist: https://gist.github.com/joerg84/ee8a6b5a92e71d6bf704
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45474: MESOS-1739: Allow slave reconfiguration on restart, Phase 1.

[Deshi Xiao]

> On 三月 30, 2016, 9:08 a.m., Adam B wrote:
> > src/tests/slave_tests.cpp, lines 3541-3545
> > 
> >
> > Here you shutdown the slave and wait (you'll probably want to advance 
> > the clock rather than wait for 90s) for the slave to be declared 
> > SLAVE_LOST. Once this occurs, the master will no longer allow the slave to 
> > reregister with the same slaveId, and the slave will be told to kill all 
> > running tasks. The slave will do so and then restart and register as a new 
> > slaveId. 
> > This is what is meant by the quote from the design doc: "Currently this 
> > can only be handled by stopping / draining a mesos slave entirely (Killing 
> > all of its running jobs), removing it from the cluster, then bringing it 
> > back up as a brand new slave."
> > 
> > To truly observe this behavior, you should start a task on the slave 
> > before you shut it down. Then you will see a TASK_LOST and the task will be 
> > killed.

Thanks Adam, i will udpate the test case.


- Deshi


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


On 三月 30, 2016, 8:13 a.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45474/
> ---
> 
> (Updated 三月 30, 2016, 8:13 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, haosdent huang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1739
> https://issues.apache.org/jira/browse/MESOS-1739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Phase 1
> Make SlaveInfo mutable throughout the stack, and allow for expansion of 
> resources and attributes only (Which allows testing to make sure it 
> propagates to the allocator, shows up in offers, etc). Ensure there is 
> unified checking for incompatibilities in both the slave and master (the 
> slave should validate the config, the master should validate that all 
> operations the slave takes are legal).
> 
> it derived from another PR(https://reviews.apache.org/r/25525/)
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1f1a31020096efa5db698e86ac74e61dfdb4b94a 
> 
> Diff: https://reviews.apache.org/r/45474/diff/
> 
> 
> Testing
> ---
> 
> make check on localhost
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 45383: Implemented recover() method of "network/cni" isolator.

[Qian Zhang]

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

(Updated March 30, 2016, 5:12 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


Repository: mesos


Description
---

Implemented recover() method of "network/cni" isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
873e0c52475f4868e611bd24a6782ad5eb261a99 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
1c8e231813c0579b79681c5d18b1f799a727ead7 
  src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
f627ec9499a34ca104d2c1a4d28e1d2f4b849f64 
  src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
611f3869402b9033081b7f9ecc1bdf006f61918b 
  src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
6a3c33645bab73edaf5af4d298a671852ea59c46 
  src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
5b5f904def9ef6dcc4462a03a2d024ad4eb3d946 

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 45265: Plugged in dvd isolator into agent.

[Guangya Liu]

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

(Updated 三月 30, 2016, 9:12 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebase


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


Repository: mesos


Description
---

Plugged in dvd isolator into agent.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 45471: Fix a test case bug due to upgrade http-parser to 2.6.1.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45471]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 30, 2016, 5:35 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45471/
> ---
> 
> (Updated March 30, 2016, 5:35 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-5063
> https://issues.apache.org/jira/browse/MESOS-5063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix a test case bug due to upgrade http-parser to 2.6.1.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 53a0ed03226030104af43a45c6ccfbfc0f4b7e9f 
> 
> Diff: https://reviews.apache.org/r/45471/diff/
> 
> 
> Testing
> ---
> 
> ../configure --enable-libevent --enable-ssl
> 
> sudo make dist check
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45474: MESOS-1739: Allow slave reconfiguration on restart, Phase 1.

[Adam B]

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




src/tests/slave_tests.cpp (lines 3541 - 3545)


Here you shutdown the slave and wait (you'll probably want to advance the 
clock rather than wait for 90s) for the slave to be declared SLAVE_LOST. Once 
this occurs, the master will no longer allow the slave to reregister with the 
same slaveId, and the slave will be told to kill all running tasks. The slave 
will do so and then restart and register as a new slaveId. 
This is what is meant by the quote from the design doc: "Currently this can 
only be handled by stopping / draining a mesos slave entirely (Killing all of 
its running jobs), removing it from the cluster, then bringing it back up as a 
brand new slave."

To truly observe this behavior, you should start a task on the slave before 
you shut it down. Then you will see a TASK_LOST and the task will be killed.


- Adam B


On March 30, 2016, 1:13 a.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45474/
> ---
> 
> (Updated March 30, 2016, 1:13 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, haosdent huang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1739
> https://issues.apache.org/jira/browse/MESOS-1739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Phase 1
> Make SlaveInfo mutable throughout the stack, and allow for expansion of 
> resources and attributes only (Which allows testing to make sure it 
> propagates to the allocator, shows up in offers, etc). Ensure there is 
> unified checking for incompatibilities in both the slave and master (the 
> slave should validate the config, the master should validate that all 
> operations the slave takes are legal).
> 
> it derived from another PR(https://reviews.apache.org/r/25525/)
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1f1a31020096efa5db698e86ac74e61dfdb4b94a 
> 
> Diff: https://reviews.apache.org/r/45474/diff/
> 
> 
> Testing
> ---
> 
> make check on localhost
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 45383: Implemented recover() method of "network/cni" isolator.

[Qian Zhang]

> On March 30, 2016, 1:35 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, line 103
> > 
> >
> > Why do we need to use a hashmap over here? Why not a vector or a list? 
> > Hashmap's are expensive in terms of memory. Usually a container won't be 
> > associated with more than one network, so seems pretty inefficient?

This was suggested by Jie when I wrote the `prepare()` patch, you can check 
https://reviews.apache.org/r/44514/diff/5?file=1304896#file1304896line82 for 
details.


> On March 30, 2016, 1:35 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, line 139
> > 
> >
> > Maybe s/recoverInfo/_recover/

I'd like to name this method more meaningful, `recoverInfo` implies that this 
method will recover/rebuild the `Info` struct for the container.


> On March 30, 2016, 1:35 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, line 151
> > 
> >
> > Should have pointed in the earlier patches, why do we need `Info` to be 
> > a smart pointer. We can have it as an object. We are not going to share 
> > this information with any other class or thread.

I think `Owned` is the suggested way, please see:
https://github.com/apache/mesos/blob/0.28.0/src/slave/containerizer/mesos/isolators/cgroups/cpushare.hpp#L110
https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/cgroups/mem.hpp#L129
https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp#L115


> On March 30, 2016, 1:35 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 136
> > 
> >
> > Why do we have this change as part of this patch? Can we move this out 
> > to a different patch?

Because when I wrote this patch, I found I need a new method to parse 
`NetworkInfo`, but I can not name it as `parse()` since there is already a 
`parse()` method for parsing `NetworkConfig`, so I have to rename that one as 
`parseNetworkConfig()`, and introduce the new one as `parseNetworkInfo()`.


> On March 30, 2016, 1:35 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 288-290
> > 
> >
> > Do we need this loop? `infos.clear` below should destroy the hashmap 
> > which in turn would destroy any of the residing objects (`networkinfos`)

Yes, I agree, and I will fix this in `_cleanup()` as well.


> On March 30, 2016, 1:35 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 817
> > 
> >
> > Isn't this a problem? We are returning `Nothing` here, which implies 
> > that in the `recover` method the container information is assumed to have 
> > been stored correctly. However, there is NO container informationt that has 
> > been stored!! I thought we should never run into this issue, since we 
> > should be creating the directories before calling the CNI plugin?

> which implies that in the recover method the container information is assumed 
> to have been stored correctly.

It does not imply the container info has been stored correctly, actually it 
implies that for this kind of container (which has no networkInfoDir created 
yet), we do not need to store anything for it (actually we have nothing to 
restore from since there is no networkInfoDir for the container).


> On March 30, 2016, 1:35 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 878
> > 
> >
> > This error doesn't make sense. Maybe:
> > "Found multiple interfaces for a given CNI network. This should not be 
> > allowed" ?

But it will miss the case that interfaces.get().size() == 0.


> On March 30, 2016, 1:35 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/spec.cpp, line 53
> > 
> >
> > Why not just return `parse` ?

Either way should be OK? I see 
https://github.com/apache/mesos/blob/0.28.0/src/docker/spec.cpp#L110:L124 does 
the similar.


> On March 30, 2016, 1:35 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 832-835
> > 
> >
> > What if the isolator dies right after creating the directories, 

Re: Review Request 45475: Fixed typos in isolator test comments.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On March 30, 2016, 7:24 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45475/
> ---
> 
> (Updated March 30, 2016, 7:24 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typos in isolator test comments.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 4dde7292831dd6eab5da1c511a55a34e3bb5a96f 
> 
> Diff: https://reviews.apache.org/r/45475/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45477: Fixed typo in stout documentation.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On March 30, 2016, 8:21 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45477/
> ---
> 
> (Updated March 30, 2016, 8:21 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
> a6e676766be527af48846a7ae6842c5b1465e501 
> 
> Diff: https://reviews.apache.org/r/45477/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45476: Fixed common typo in libprocess documentation.

[Alexander Rukletsov]

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


Ship it!




There is one more in "help.cpp", I'll fix it for you.

- Alexander Rukletsov


On March 30, 2016, 8:19 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45476/
> ---
> 
> (Updated March 30, 2016, 8:19 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed common typo in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/io.hpp 
> 52b7ee58ec6ae602e11222b72e84221d4550eea9 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> b1d867bc18490ef36e4012c152f8bed46d9c00f2 
>   3rdparty/libprocess/src/io.cpp dd77793b0a289ebffecfdddf2cd881b4659d637c 
> 
> Diff: https://reviews.apache.org/r/45476/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45082: Implemented cleanup() method of "network/cni" isolator.

[Qian Zhang]

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

(Updated March 30, 2016, 4:36 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Implemented cleanup() method of "network/cni" isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
873e0c52475f4868e611bd24a6782ad5eb261a99 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
1c8e231813c0579b79681c5d18b1f799a727ead7 

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


Testing
---

make check


Thanks,

Qian Zhang

Review Request 45477: Fixed typo in stout documentation.

[Joerg Schad]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

Fixed typo in stout.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
a6e676766be527af48846a7ae6842c5b1465e501 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 45434: Removed unused credentials.

[Jan Schlicht]

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

(Updated March 30, 2016, 10:20 a.m.)


Review request for mesos, Adam B and Greg Mann.


Summary (updated)
-

Removed unused credentials.


Repository: mesos


Description (updated)
---

See summary.


Diffs (updated)
-

  src/tests/files_tests.cpp 2f0f84cd78e7f382b70b97c8711f71cea64ce0a5 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 42516: Add support for user-defined networks.

[haosdent huang]

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


Fix it, then Ship it!




Ship It!


src/docker/docker.cpp (line 556)


I think could remove `this->`


- haosdent huang


On March 30, 2016, 7:52 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 30, 2016, 7:52 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 45434: Added test cases for '/files' endpoint authentication.

[Jan Schlicht]

> On March 30, 2016, 2:38 a.m., Greg Mann wrote:
> > BenM and I discussed this while I was working on the previous patches: 
> > since the code paths for the authenticated and unauthenticated cases are 
> > the same once the handler is reached, it shouldn't be necessary to 
> > explicitly test the full functioning of the endpoints in both cases. We 
> > test the full functionality of the endpoints with authentication disabled, 
> > then we test that requests that don't authenticate properly are refused 
> > when authentication is on. This should test all of the code paths through 
> > the handlers, since we don't actually do anything with the principal 
> > currently. What do you think?

Yes, I agree. The authenticated cases are already covered implicitly and the 
authenticator differs between "bad credentials" vs "missing credentials" -- 
which is already covered elsewhere. I'd then probably only remove the 
`badCredentials` as they're unused.


- Jan


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


On March 29, 2016, 6:36 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45434/
> ---
> 
> (Updated March 29, 2016, 6:36 p.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current authentication tests of the `/files/` endpoint only test the case 
> of missing credentials. This commit adds the additional case of wrong 
> credentials.
> 
> 
> Diffs
> -
> 
>   src/tests/files_tests.cpp 2f0f84cd78e7f382b70b97c8711f71cea64ce0a5 
> 
> Diff: https://reviews.apache.org/r/45434/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Guangya Liu]

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


Fix it, then Ship it!




Ship It!


src/docker/docker.cpp (lines 568 - 569)


I think this can be simplified as:

return Failure("Only a single network can be defined in Docker run");

No need to add period to the end.


- Guangya Liu


On 三月 30, 2016, 7:52 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 三月 30, 2016, 7:52 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 45466: Removed the redundant `NULL` check when deleting `Credential`.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45466]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 30, 2016, 2:23 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45466/
> ---
> 
> (Updated March 30, 2016, 2:23 a.m.)
> 
> 
> Review request for mesos, Klaus Ma, Neil Conway, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary. Also moved all `delete` statements together in the order they 
> were declared in `.hpp` file.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 701e024ac57fc6e2f96d7d83e65cb68252d41654 
> 
> Diff: https://reviews.apache.org/r/45466/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

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

(Updated March 30, 2016, 7:52 a.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Changes
---

Fixed the  error message in case multiple networks are specified during "run"


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
  include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
  src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera

Re: Review Request 45082: Implemented cleanup() method of "network/cni" isolator.

[Qian Zhang]

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

(Updated March 30, 2016, 3:45 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Implemented cleanup() method of "network/cni" isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
873e0c52475f4868e611bd24a6782ad5eb261a99 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
1c8e231813c0579b79681c5d18b1f799a727ead7 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 45475: Fixed typos in isolator test comments.

[haosdent huang]

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


Ship it!




Thank you for your quick reply!

- haosdent huang


On March 30, 2016, 7:24 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45475/
> ---
> 
> (Updated March 30, 2016, 7:24 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typos in isolator test comments.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 4dde7292831dd6eab5da1c511a55a34e3bb5a96f 
> 
> Diff: https://reviews.apache.org/r/45475/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45231: Removed the use of subprocess setup functions [2/7].

[Joerg Schad]

> On March 30, 2016, 3:39 a.m., haosdent huang wrote:
> > src/tests/containerizer/isolator_tests.cpp, line 634
> > 
> >
> > `testwill` should be `test will`

THX! Fixed with https://reviews.apache.org/r/45475/.


- Joerg


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


On March 28, 2016, 4:51 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45231/
> ---
> 
> (Updated March 28, 2016, 4:51 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review follows the previous one and removes most
> (see following reviews) usages setup functions throughout
> the code.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
>   src/health-check/main.cpp 36955dd5f77a056213762d92c39be4ca7dd5780c 
>   src/slave/container_loggers/lib_logrotate.cpp 
> cf5f238eb3b6217b848cf23f8c9cd1848bf9a9f0 
>   src/slave/containerizer/docker.cpp 0133628f5b37dc0d3aa12f210c61dfc07deac18b 
>   src/slave/containerizer/external_containerizer.cpp 
> fe368dd52644b89e47ffd4d947de290b3998fb1a 
>   src/slave/containerizer/fetcher.cpp 
> 2b1e022c8dcd9734663f20fe963a8de0d6233715 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e7f7e7fd1304e14dbfaab8b53cea16efc0417911 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> b38d83fbb29f46552ffbda7b17cbc85af15550e1 
>   src/slave/containerizer/mesos/launcher.hpp 
> 8afac20c5a4093162f764412dbf16c4230dcb141 
>   src/slave/containerizer/mesos/launcher.cpp 
> ba012b1d577a770948a1bfcad5ad2d4977f81b2f 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 1763d8d48614e0a1e187505d90920e97c8e476fb 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 550c58263df257989568debe2858ef599e5f96e8 
>   src/tests/containerizer/isolator_tests.cpp 
> df506fc793e480e825b476e43c683ef8bcf676b2 
>   src/tests/containerizer/launch_tests.cpp 
> e0f934cf93f51316b35f34d34f3d0c5f004565d9 
>   src/tests/containerizer/launcher.hpp 
> 36405a97893f352147af4794fb39b6e621d95b58 
>   src/tests/containerizer/launcher.cpp 
> a92d9890f0931425d69ef8ce0896d081b8722079 
>   src/tests/containerizer/ns_tests.cpp 
> 4bf45e970bede713fa4ffce205627b149232fe2b 
>   src/tests/slave_tests.cpp 1f1a31020096efa5db698e86ac74e61dfdb4b94a 
> 
> Diff: https://reviews.apache.org/r/45231/diff/
> 
> 
> Testing
> ---
> 
> tested complete chain (see https://reviews.apache.org/r/45236/).
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 45475: Fixed typos in isolator test comments.

[Joerg Schad]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

Fixed typos in isolator test comments.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
4dde7292831dd6eab5da1c511a55a34e3bb5a96f 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 45474: MESOS-1739: Allow slave reconfiguration on restart, Phase 1.

[haosdent huang]

> On March 30, 2016, 6:52 a.m., haosdent huang wrote:
> > I think the better place to put this test case is 
> > `slave_recovery_tests.cpp`.
> 
> Deshi Xiao wrote:
> i don't think so. the test aim to focus on reconfiguration from restart 
> slave. it doesn't care the recovery state.

Got it. You reason make sense.


- haosdent


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


On March 30, 2016, 7:03 a.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45474/
> ---
> 
> (Updated March 30, 2016, 7:03 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Bugs: MESOS-1739
> https://issues.apache.org/jira/browse/MESOS-1739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Phase 1
> Make SlaveInfo mutable throughout the stack, and allow for expansion of 
> resources and attributes only (Which allows testing to make sure it 
> propagates to the allocator, shows up in offers, etc). Ensure there is 
> unified checking for incompatibilities in both the slave and master (the 
> slave should validate the config, the master should validate that all 
> operations the slave takes are legal).
> 
> it derived from another PR(https://reviews.apache.org/r/25525/)
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1f1a31020096efa5db698e86ac74e61dfdb4b94a 
> 
> Diff: https://reviews.apache.org/r/45474/diff/
> 
> 
> Testing
> ---
> 
> make check on localhost
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 45474: MESOS-1739: Allow slave reconfiguration on restart, Phase 1.

[Deshi Xiao]

> On 三月 30, 2016, 6:52 a.m., haosdent huang wrote:
> > I think the better place to put this test case is 
> > `slave_recovery_tests.cpp`.

i don't think so. the test aim to focus on reconfiguration from restart slave. 
it doesn't care the recovery state.


- Deshi


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


On 三月 30, 2016, 7:03 a.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45474/
> ---
> 
> (Updated 三月 30, 2016, 7:03 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Bugs: MESOS-1739
> https://issues.apache.org/jira/browse/MESOS-1739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Phase 1
> Make SlaveInfo mutable throughout the stack, and allow for expansion of 
> resources and attributes only (Which allows testing to make sure it 
> propagates to the allocator, shows up in offers, etc). Ensure there is 
> unified checking for incompatibilities in both the slave and master (the 
> slave should validate the config, the master should validate that all 
> operations the slave takes are legal).
> 
> it derived from another PR(https://reviews.apache.org/r/25525/)
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1f1a31020096efa5db698e86ac74e61dfdb4b94a 
> 
> Diff: https://reviews.apache.org/r/45474/diff/
> 
> 
> Testing
> ---
> 
> make check on localhost
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 45474: MESOS-1739: Allow slave reconfiguration on restart, Phase 1.

[haosdent huang]

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




src/tests/slave_tests.cpp (line 3558)


`Wait for offers.`


- haosdent huang


On March 30, 2016, 6:52 a.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45474/
> ---
> 
> (Updated March 30, 2016, 6:52 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1739
> https://issues.apache.org/jira/browse/MESOS-1739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Phase 1
> Make SlaveInfo mutable throughout the stack, and allow for expansion of 
> resources and attributes only (Which allows testing to make sure it 
> propagates to the allocator, shows up in offers, etc). Ensure there is 
> unified checking for incompatibilities in both the slave and master (the 
> slave should validate the config, the master should validate that all 
> operations the slave takes are legal).
> 
> it derived from another PR(https://reviews.apache.org/r/25525/)
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1f1a31020096efa5db698e86ac74e61dfdb4b94a 
> 
> Diff: https://reviews.apache.org/r/45474/diff/
> 
> 
> Testing
> ---
> 
> make check on localhost
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>