Re: Review Request 70870: Updated `namespaces-ipc.md` for configurable IPC namespace and /dev/shm.

2019-06-17 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [70773, 70774, 70775, 70798, 70844, 70820, 70845, 70849, 
70852, 70857, 70859, 70860, 70870]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On June 18, 2019, 2:42 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70870/
> ---
> 
> (Updated June 18, 2019, 2:42 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9828
> https://issues.apache.org/jira/browse/MESOS-9828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `namespaces-ipc.md` for configurable IPC namespace and /dev/shm.
> 
> 
> Diffs
> -
> 
>   docs/isolators/namespaces-ipc.md e550e752ef0ca4bc363fd3c5c190c44f5dd4187a 
> 
> 
> Diff: https://reviews.apache.org/r/70870/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Review Request 70870: Updated `namespaces-ipc.md` for configurable IPC namespace and /dev/shm.

2019-06-17 Thread Qian Zhang

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

Review request for mesos, Andrei Budnik and Gilbert Song.


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


Repository: mesos


Description
---

Updated `namespaces-ipc.md` for configurable IPC namespace and /dev/shm.


Diffs
-

  docs/isolators/namespaces-ipc.md e550e752ef0ca4bc363fd3c5c190c44f5dd4187a 


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


Testing
---


Thanks,

Qian Zhang



Re: Review Request 70862: Update `EXPECT` to `ASSERT` in blkio tests.

2019-06-17 Thread Jiang Yan Xu

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


Ship it!




Ship It!

- Jiang Yan Xu


On June 16, 2019, 7:13 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70862/
> ---
> 
> (Updated June 16, 2019, 7:13 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the blkio cgroup tests immediately require the result of
> `blkio_statistics()`, we should test for the presence of statistics
> with `ASSERT_TRUE` rather than `EXPECT_TRUE`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> eef8eb28ea57ef0a2a3c626ac9aee202eb7231d9 
> 
> 
> Diff: https://reviews.apache.org/r/70862/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 30)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 70868: Clarified ERROR event as first event in the scheduler HTTP API docs.

2019-06-17 Thread Benjamin Mahler

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




docs/scheduler-http-api.md
Lines 644 (patched)


Actually, we made the `UPDATE_FRAMEWORK` call return an appropriate 
response code so remove this example.


- Benjamin Mahler


On June 17, 2019, 10:49 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70868/
> ---
> 
> (Updated June 17, 2019, 10:49 p.m.)
> 
> 
> Review request for mesos, Greg Mann, James DeFelice, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The ERROR event is also possible as the first event in the streaming
> response from the master, but this is not clear in the documentation
> as pointed out by James DeFelice (james.defel...@gmail.com).
> 
> 
> Diffs
> -
> 
>   docs/scheduler-http-api.md a5327c229142267836f327f9c382ef50b7e334db 
> 
> 
> Diff: https://reviews.apache.org/r/70868/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 70868: Clarified ERROR event as first event in the scheduler HTTP API docs.

2019-06-17 Thread James DeFelice

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


Ship it!




Ship It!

- James DeFelice


On June 17, 2019, 10:49 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70868/
> ---
> 
> (Updated June 17, 2019, 10:49 p.m.)
> 
> 
> Review request for mesos, Greg Mann, James DeFelice, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The ERROR event is also possible as the first event in the streaming
> response from the master, but this is not clear in the documentation
> as pointed out by James DeFelice (james.defel...@gmail.com).
> 
> 
> Diffs
> -
> 
>   docs/scheduler-http-api.md a5327c229142267836f327f9c382ef50b7e334db 
> 
> 
> Diff: https://reviews.apache.org/r/70868/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Review Request 70868: Clarified ERROR event as first event in the scheduler HTTP API docs.

2019-06-17 Thread Benjamin Mahler

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

Review request for mesos, Greg Mann, James DeFelice, and Vinod Kone.


Repository: mesos


Description
---

The ERROR event is also possible as the first event in the streaming
response from the master, but this is not clear in the documentation
as pointed out by James DeFelice (james.defel...@gmail.com).


Diffs
-

  docs/scheduler-http-api.md a5327c229142267836f327f9c382ef50b7e334db 


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


Testing
---

N/A


Thanks,

Benjamin Mahler



Re: Review Request 70854: Made scheduler driver's updateFramework() require FrameworkInfo with ID.

2019-06-17 Thread Andrei Sekretenko


> On June 15, 2019, 5:34 p.m., Benjamin Mahler wrote:
> > Thanks! One thing I'm wondering is whether it makes sense to just populate 
> > `FrameworkInfo.user` and `FrameworkInfo.hostname` in 
> > `SchedulerProcess::updateFramework` to the previous values (if not set), 
> > rather than calling the system calls again.

I'm not sure either.

This approach has two upsides:
- a bit simpler
- calling updateFramework() is consistent with re-creating the SchedulerDriver

and two downsides:
- user/hostname can change during the SchedulerDriver's lifetime (weird)
- these syscalls might block for an enormous length of time (at least the ones 
behind `net::hostname()`, which calls `getaddrinfo()`, which might perform DNS 
lookups)


- Andrei


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


On June 14, 2019, 12:32 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70854/
> ---
> 
> (Updated June 14, 2019, 12:32 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes it mandatory for the caller to fill the 'id' field of
> the FrameworkInfo passed into MesosSchedulerDriver::updateFramework().
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp c5e61d4fd7aae5067d8c7d8dec878dc82c86e4ec 
>   src/sched/sched.cpp 281236bb53e1c6ed77b69bf954e27705595ffb2a 
> 
> 
> Diff: https://reviews.apache.org/r/70854/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>



Re: Review Request 70863: Assign cgroup processes after configuring the subsystem.

2019-06-17 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [70863]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On June 17, 2019, 2:47 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70863/
> ---
> 
> (Updated June 17, 2019, 2:47 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang.
> 
> 
> Bugs: MESOS-9805
> https://issues.apache.org/jira/browse/MESOS-9805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, the PID targeted by the cgroups isolator is moved into
> the cgroup before the subsystem runs to apply any type-specific
> cgroup configuration. We should reverse the order of this so that
> the PID is only moved once the cgroup is fully configured by the
> subsystem.
> 
> A specific case that can happen is where a PID is assigned to a net_cls
> cgroup before that cgroup has its class ID set.  This intermediate
> process state can be observed by system monitoring process, causing
> confusion that is hard to debug.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 4a1871b3b06b54a02dfe09289f7fb304a3f7f24c 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> e7819d732172bdbd215106e3b781588c1f78b2ec 
> 
> 
> Diff: https://reviews.apache.org/r/70863/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 30)
> 
> 
> Thanks,
> 
> James Peach
> 
>