Re: Review Request 43819: Added Scheduler-Driver API to app-framework-development-guide.md.

[Adam B]

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



Great work. Still missing a bit of context about what a SchedulerDriver is 
though


docs/app-framework-development-guide.md (lines 131 - 133)


Let's be very explicit that they'll need to instantiate a 
MesosSchedulerDriver and call these methods on it. Probably also want to 
document the MesosSchedulerDriver constructors so they know how to instantiate 
one.



docs/app-framework-development-guide.md (line 138)


Get rid of the `virtual` and `= 0` since that's not a part of the API that 
their scheduler will call. They're not implementing a new SchedulerDriver, 
they're just instantiating one and calling its methods.



docs/app-framework-development-guide.md (lines 187 - 191)


Remove this from the guide since it's deprecated.


- Adam B


On Feb. 21, 2016, 7 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> ---
> 
> (Updated Feb. 21, 2016, 7 p.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Scheduler-Driver API to app-framework-development-guide.md.
> 
> 
> Diffs
> -
> 
>   docs/app-framework-development-guide.md 
> e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d 
> 
> Diff: https://reviews.apache.org/r/43819/diff/
> 
> 
> Testing
> ---
> 
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and 
> via docker website container.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43916: Modified testcase to test `--cgroups_net_cls_secondary_handles` flag.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43913, 43914, 43915, 43916]

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 Feb. 23, 2016, 11:25 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43916/
> ---
> 
> (Updated Feb. 23, 2016, 11:25 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4700
> https://issues.apache.org/jira/browse/MESOS-4700
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modified testcase to test `--cgroups_net_cls_secondary_handles` flag.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 653b037c489072f43e53dec01a811a9249dcd660 
> 
> Diff: https://reviews.apache.org/r/43916/diff/
> 
> 
> Testing
> ---
> 
> make check and sudo make check (for NetClsIsolator)
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

[Kevin Klues]

> On Feb. 17, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 108-112
> > 
> >
> > As we close the review, we should also post a comment to reviewboard 
> > with the commit message we actually pushed to master.
> > 
> > You can use the --description flag:
> > 
> > https://www.reviewboard.org/docs/rbtools/dev/rbt/commands/close/
> 
> Vinod Kone wrote:
> I'll make a TODO for now as committers don't currently/always set the 
> commit message on RB.
> 
> Kevin Klues wrote:
> I think we should add the option as a command line flag now, so that 
> people who like to include the commit message when closing can still take 
> advantage of this script.  If we later decide that *everyone* should always 
> include the commit message, we can remove the flag and force it to be set.
> 
> Vinod Kone wrote:
> I am suggesting a TODO because it needs some non-trivial code change to 
> get it to work. `get_reviews()` currently read log line by line.
> 
> Also, note that people can still put a commit message in the description 
> by directly going to RB after running this script. So I see this script as a 
> strict improvement to the current workflow.

OK.


- Kevin


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


On Feb. 22, 2016, 4:52 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> ---
> 
> (Updated Feb. 22, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
> https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> ---
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

[Kevin Klues]

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




support/push-reviews.py (lines 123 - 125)


Ah, I see it now. There is another call to `if not options['dry_run']` 
inside the `close_reviews()` call which protects it from actually closing it on 
rb. Seems like this could be consolidated somehow, since it's a little 
confusing at first glance, but may not be worth the effort.


- Kevin Klues


On Feb. 22, 2016, 4:52 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> ---
> 
> (Updated Feb. 22, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
> https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> ---
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 43911: Updated `/state` agent endpoint to use jsonify.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43816, 43822, 43823, 43817, 43910, 43911]

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 Feb. 23, 2016, 10:29 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43911/
> ---
> 
> (Updated Feb. 23, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `/state` agent endpoint to use jsonify.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e 
> 
> Diff: https://reviews.apache.org/r/43911/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

[Vinod Kone]

> On Feb. 23, 2016, 12:44 a.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 14-15
> > 
> >
> > I would at least pass this as a flag right now rather than jsut adding 
> > a TODO. Otherwise people that prefer to close with the commit message won't 
> > be able to use your script at all.

Not sure what you mean by pass this as a flag? Do you mean add a command line 
argument to the script (--say post_to_JIRA) that is a no-op? Or do you want me 
to write code that speaks JIRA API? Also I don't understand why people who 
prefer to close with a commit message can't use this script. See my comment in 
the previous reply to your comment.


> On Feb. 23, 2016, 12:44 a.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 123-125
> > 
> >
> > Shouldn't this also be protected by the `if not options['dry_run']` 
> > check?

The only reason was to print the 'Closing review...' irrespective of the dry 
run option.


- Vinod


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


On Feb. 22, 2016, 4:52 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> ---
> 
> (Updated Feb. 22, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
> https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> ---
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 43552: Added a support/push-reviews.py script to push reviews upstream.

[Vinod Kone]

> On Feb. 17, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/push-reviews.py, lines 108-112
> > 
> >
> > As we close the review, we should also post a comment to reviewboard 
> > with the commit message we actually pushed to master.
> > 
> > You can use the --description flag:
> > 
> > https://www.reviewboard.org/docs/rbtools/dev/rbt/commands/close/
> 
> Vinod Kone wrote:
> I'll make a TODO for now as committers don't currently/always set the 
> commit message on RB.
> 
> Kevin Klues wrote:
> I think we should add the option as a command line flag now, so that 
> people who like to include the commit message when closing can still take 
> advantage of this script.  If we later decide that *everyone* should always 
> include the commit message, we can remove the flag and force it to be set.

I am suggesting a TODO because it needs some non-trivial code change to get it 
to work. `get_reviews()` currently read log line by line.

Also, note that people can still put a commit message in the description by 
directly going to RB after running this script. So I see this script as a 
strict improvement to the current workflow.


- Vinod


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


On Feb. 22, 2016, 4:52 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43552/
> ---
> 
> (Updated Feb. 22, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Michael Park.
> 
> 
> Bugs: MESOS-3929
> https://issues.apache.org/jira/browse/MESOS-3929
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This script allows committers to push locally applied review chain to the ASF
> git repo and mark the reviews as submitted.
> 
> 
> Diffs
> -
> 
>   support/push-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43552/diff/
> 
> 
> Testing
> ---
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 43798: Added overview section to upgrades.md.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43792, 43798]

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 Feb. 23, 2016, 9:39 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43798/
> ---
> 
> (Updated Feb. 23, 2016, 9:39 p.m.)
> 
> 
> Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4381
> https://issues.apache.org/jira/browse/MESOS-4381
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added overview section to upgrades.md.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 4f30d725c6ed28c09a1c5528fd4193c3f06b2d93 
> 
> Diff: https://reviews.apache.org/r/43798/diff/
> 
> 
> Testing
> ---
> 
> Viewed via gist (https://gist.github.com/joerg84/eddbc0302a5a4b291e81) and 
> docker website container.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 43920: Added a helper function to stout : os/which.hpp.

[Disha Singh]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

Added a helper function to stout : os/which.hpp.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/which.hpp PRE-CREATION 

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


Testing
---


Thanks,

Disha  Singh

Re: Review Request 43868: Add a getter for master::Flags.

[Jay Guo]

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

(Updated Feb. 24, 2016, 3:42 a.m.)


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


Changes
---

Add blank line.


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


Repository: mesos


Description
---

MESOS-3481 Add const accessor to Master flags.


Diffs (updated)
-

  src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
  src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 

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


Testing
---

Added one new unit test MasterTest::Flags

`make check` passed


Thanks,

Jay Guo

Re: Review Request 43864: Fix typo of roles doc.

[Klaus Ma]

> On Feb. 24, 2016, 2:06 a.m., Neil Conway wrote:
> > The right place to make this fix is in `src/master/http.cpp`, and then 
> > rerun `support/generate-endpoint-help.py`. Seems like 
> > `Master::Http::ROLES_HELP` is missing commas in `DESCRIPTION`.

You're right, we missed commas in `Master::Http::ROLES_HELP` `DESCRIPTION`. 
Update the patches to address this comments :).


- Klaus


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


On Feb. 24, 2016, 11:34 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43864/
> ---
> 
> (Updated Feb. 24, 2016, 11:34 a.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix typo of roles doc.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/roles.json.md 
> d67779c246cceae2209f2611f32ada4493ae6f83 
>   docs/endpoints/master/roles.md 976a9b7891a17652289126ec7e7ee73cea0c2e35 
>   src/master/http.cpp 939fab21a2240de7214ef809a194ffb3837a9f1b 
> 
> Diff: https://reviews.apache.org/r/43864/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43864: Fix typo of roles doc.

[Klaus Ma]

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

(Updated Feb. 24, 2016, 11:34 a.m.)


Review request for mesos and Neil Conway.


Changes
---

Address comments.


Repository: mesos


Description
---

Fix typo of roles doc.


Diffs (updated)
-

  docs/endpoints/master/roles.json.md d67779c246cceae2209f2611f32ada4493ae6f83 
  docs/endpoints/master/roles.md 976a9b7891a17652289126ec7e7ee73cea0c2e35 
  src/master/http.cpp 939fab21a2240de7214ef809a194ffb3837a9f1b 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 43868: Add a getter for master::Flags.

[Jay Guo]

> On Feb. 24, 2016, 1:38 a.m., Qian Zhang wrote:
> > src/master/master.hpp, line 545
> > 
> >
> > Need to return "const Flags&"?

Return reference to flags causes problem:
```
../../3rdparty/libprocess/include/process/future.hpp:147:10: error: 
'operator->' declared as a pointer to a reference of type 'const 
mesos::internal::master::Flags &'
  const T* operator->() const;
 ^
```

method `getFlags()` is dispatched to master process and we believe pointer is 
created during intermediate procedures pointing to returned object.


- Jay


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


On Feb. 23, 2016, 5:23 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43868/
> ---
> 
> (Updated Feb. 23, 2016, 5:23 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Joris Van Remoortere, and Qian Zhang.
> 
> 
> Bugs: MESOS-3481
> https://issues.apache.org/jira/browse/MESOS-3481
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3481 Add const accessor to Master flags.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
> 
> Diff: https://reviews.apache.org/r/43868/diff/
> 
> 
> Testing
> ---
> 
> Added one new unit test MasterTest::Flags
> 
> `make check` passed
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 43868: Add a getter for master::Flags.

[Jay Guo]

> On Feb. 23, 2016, 12:45 p.m., Klaus Ma wrote:
> > src/master/master.hpp, line 545
> > 
> >
> > `getFlags() const`
> 
> Jay Guo wrote:
> OK

We found that *constant member funtion* here requires a new `dispatch()` 
template:
```cpp
template 
Future dispatch(const PID& pid, R (T::*method)() const)
```

original:
```cpp
template 
Future dispatch(const PID& pid, R (T::*method)())
```

notice *const* following `R (T::*method)()` in arguments.

Otherwise compiler complains. Should we add it?


- Jay


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


On Feb. 23, 2016, 5:23 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43868/
> ---
> 
> (Updated Feb. 23, 2016, 5:23 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Joris Van Remoortere, and Qian Zhang.
> 
> 
> Bugs: MESOS-3481
> https://issues.apache.org/jira/browse/MESOS-3481
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3481 Add const accessor to Master flags.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
> 
> Diff: https://reviews.apache.org/r/43868/diff/
> 
> 
> Testing
> ---
> 
> Added one new unit test MasterTest::Flags
> 
> `make check` passed
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 43662: Added support for pipelining calls to the scheduler library.

[Vinod Kone]

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




src/scheduler/scheduler.cpp (lines 117 - 119)


This comment is old. Can you update?

s/receiving messages (eventually events/HTTP messages/

s/sending messages (via calls)/HTTP messages/



src/scheduler/scheduler.cpp (line 207)


I think for both scheduler::Mesos and executor::Mesos we need to add more 
comments around send() semantics. 

For example, we should mention that clients should send() only after 
connected() callback has been called.



src/scheduler/scheduler.cpp (lines 237 - 242)


Shouldn't we do the same for executor library?

More importnatly, what if a previous subscribe is in progress but 
`subscribed` is not set?



src/scheduler/scheduler.cpp (line 264)


CHECK(state == DISCONNECTED) ?



src/scheduler/scheduler.cpp (lines 278 - 280)


s/i.e./e.g.,/ ?

s/we failed over to a new master/master failed over/

Also, not sure what you wanted to say in this comment? Are you saying it is 
OK even if a new master is enqueued because?



src/scheduler/scheduler.cpp (line 297)


This is the current master as detected by the detector. What is the 
guarantee that the connection was made with this master and not an old master? 
IOW, should this method take `master` as an argument?

Can we guarantee that the master hasn't changed because a new master 
detected causes these futures to be discarded synchronously? Looking at 
detected() that doesn't seem to be the case.

Do we want to enforce the invariant that there is ever only one connection 
attempt in progress?



src/scheduler/scheduler.cpp (line 370)


is it possible for this callback to be invoked on the scheduler after they 
get a connected callback from this new connection attempt?



src/scheduler/scheduler.cpp (line 400)


What happens if a previous connection attempt is in progress?



src/scheduler/scheduler.cpp (line 457)


For executor library, we did a close() here of the previous reader. Why was 
it required there and not here?


- Vinod Kone


On Feb. 22, 2016, 8:19 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43662/
> ---
> 
> (Updated Feb. 22, 2016, 8:19 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3570
> https://issues.apache.org/jira/browse/MESOS-3570
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the scheduler library used to chain calls on previous call 
> responses. This was inherently slow. This change adds support for pipelining 
> all calls to the master on a single connection via the `http::Connection` 
> abstraction in libprocess.
> 
> This change also adds support for handling various error scenarios when we 
> notice a disconnection instead of just relying on the master detector for 
> invoking the `disconnected` callback.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 99a7d0dfff7b0c61decc9ff6d9e6d46ef13a7e75 
> 
> Diff: https://reviews.apache.org/r/43662/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 43868: Add a getter for master::Flags.

[Jay Guo]

> On Feb. 24, 2016, 1:44 a.m., Qian Zhang wrote:
> > With this patch, do we still need to explicitly call `CreateMasterFlags()` 
> > in the test cases (e.g., 
> > https://github.com/apache/mesos/blob/2876b8c918814347dd56f6f87d461e414a90650a/src/tests/master_maintenance_tests.cpp#L1231-L1235)?
> >  I see `CreateMasterFlags()` will be internally called by `StartMaster()`, 
> > so for all the test cases which calls `CreateMasterFlags()` first and then 
> > `StartMaster()`, should we change them to call `StartMaster()` and then 
> > `getFlags()` that you added in this patch?

Yes we believe this is exactly the intention of adding getter method. And we 
have actually experimented modifying original test to use `StartMaster()` and 
`getFlags()`. **However**, there are approx 50 instances in current code base 
using `StartMaster(flags)`. We fear *changing all of them is beyond the scope 
of this task*. Thoughts?


- Jay


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


On Feb. 23, 2016, 5:23 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43868/
> ---
> 
> (Updated Feb. 23, 2016, 5:23 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Joris Van Remoortere, and Qian Zhang.
> 
> 
> Bugs: MESOS-3481
> https://issues.apache.org/jira/browse/MESOS-3481
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3481 Add const accessor to Master flags.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
> 
> Diff: https://reviews.apache.org/r/43868/diff/
> 
> 
> Testing
> ---
> 
> Added one new unit test MasterTest::Flags
> 
> `make check` passed
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 43893: Made sure that spawned processes terminated before leaving test.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43893]

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 Feb. 23, 2016, 7:50 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43893/
> ---
> 
> (Updated Feb. 23, 2016, 7:50 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4747
> https://issues.apache.org/jira/browse/MESOS-4747
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made sure that spawned processes terminated before leaving tests.
> 
> 
> Diffs
> -
> 
>   src/tests/container_logger_tests.cpp 
> 6e2b8c1b80d4b49a1042c53945c223785d0d4a3f 
> 
> Diff: https://reviews.apache.org/r/43893/diff/
> 
> 
> Testing
> ---
> 
> make check
> make check GTEST_FILTER=ContainerLoggerTest.MesosContainerizerRecover
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43868: Add a getter for master::Flags.

[Qian Zhang]

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




src/master/master.hpp (line 545)


Need to return "const Flags&"?


- Qian Zhang


On Feb. 23, 2016, 1:23 p.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43868/
> ---
> 
> (Updated Feb. 23, 2016, 1:23 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Joris Van Remoortere, and Qian Zhang.
> 
> 
> Bugs: MESOS-3481
> https://issues.apache.org/jira/browse/MESOS-3481
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3481 Add const accessor to Master flags.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
> 
> Diff: https://reviews.apache.org/r/43868/diff/
> 
> 
> Testing
> ---
> 
> Added one new unit test MasterTest::Flags
> 
> `make check` passed
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 43615: Update test suite to use the reworked MesosTest helpers.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43613, 43614, 43629, 43630, 43615]

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 Feb. 23, 2016, 7:29 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43615/
> ---
> 
> (Updated Feb. 23, 2016, 7:29 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Includes the following changes:
> 
> * Added the `` header where appropriate.
> * Added the namespace `using process::Owned;` where appropriate.
> * Generally replaced `Try` with `Owned`.  And 
> `Try` with `Owned`.
> * Added the (now required) `MasterDetector` argument to all slaves.  Before, 
> this was fetched from the first master in `Cluster`.
> * Removed `Shutdown();` from all tests.
> * Replaced `Stop(...)` with the appropriate master/slave destruction calls.
> * Wrap various slave objects in `Owned` (i.e. containerizers, isolators, 
> launchers, etc).
> * Replace `CHECK` in tests with `ASSERT`.
> 
> 
> Diffs
> -
> 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/command_executor_tests.cpp 
> 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 
>   src/tests/container_logger_tests.cpp 
> 6e2b8c1b80d4b49a1042c53945c223785d0d4a3f 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 6a60962b4593b3521c182c7320331743ccffd4ba 
>   src/tests/containerizer/isolator_tests.cpp 
> 653b037c489072f43e53dec01a811a9249dcd660 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 4a03af2c9c0643d964b1d76e2096341b59bf5dce 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 983a6be160aefe5a32acb6111bb3c85230ec 
>   src/tests/credentials_tests.cpp 7edcc857e0f6f8e80e265deeec59d6349d392224 
>   src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
>   src/tests/exception_tests.cpp 6b71316d545e97f14a45daa14d0fd95204befd3b 
>   src/tests/executor_http_api_tests.cpp 
> 36a042ed103271ca873450236f39a8152fbbf07e 
>   src/tests/fault_tolerance_tests.cpp 
> 982468f851cd9d95eb6cde7c57f2d737d46a827c 
>   src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/hook_tests.cpp 59a00ea722a17bbc82b14c69bda826f68cbac6e9 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/master_contender_detector_tests.cpp 
> 255ab8119a04b55bb4f1b61dee19c4be64499376 
>   src/tests/master_maintenance_tests.cpp 
> b98eedec388813ee795dd83ccc5ff27338209475 
>   src/tests/master_quota_tests.cpp 8357ec911b2a158632a708ae3adff6eabc536697 
>   src/tests/master_slave_reconciliation_tests.cpp 
> d41178eb41df519073fc0890c5716bbc9fed6ad2 
>   src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
>   src/tests/master_validation_tests.cpp 
> ab2df22f73052f6bd77653e56e7b460b17e7b0be 
>   src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
>   src/tests/monitor_tests.cpp 869c9e032817e8859a968232d4a61556a3d53d45 
>   src/tests/oversubscription_tests.cpp 
> d4ae81972fd218c58a413d1968a4e9acbee52fd3 
>   src/tests/partition_tests.cpp c5badbe90e302793bfbf3f16373efe241decb7d5 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 6069ca1e9ed278459c5182e438417e95955b1924 
>   src/tests/persistent_volume_tests.cpp 
> e169e1b141a38dc389eefd42c11a078c413123d5 
>   src/tests/rate_limiting_tests.cpp caced732ded05a334861a53488ef6391885b2263 
>   src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
>   src/tests/registrar_zookeeper_tests.cpp 
> 3df9779ee5d076e16f6a538326693a36f986b6d0 
>   src/tests/repair_tests.cpp bb104562659e135492f9857e5b452c8a0a9e97da 
>   src/tests/reservation_endpoints_tests.cpp 
> 32b2af4115211b58a5127a14dd19152c2eca120c 
>   src/tests/reservation_tests.cpp b8878d51767ac0d95e346c44c0a4d5c060e565ef 
>   

Re: Review Request 43881: Added allocator metric for the number of allocations to a framework.

[Guangya Liu]

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




src/master/allocator/mesos/hierarchical.cpp (line 1197)


The allocator do not know what is offer, does it make sense to rename this 
metrics to `allocator/framework_allocations`



src/master/allocator/mesos/hierarchical.cpp (line 1354)


The counter may count twice for some cases, one framework may get resource 
allocation in both allocation stage1(Quota) and stage2(wDRF).



src/tests/hierarchical_allocator_tests.cpp (lines 2359 - 2360)


4 spaces



src/tests/hierarchical_allocator_tests.cpp (lines 2369 - 2370)


What about following format?

allocator->setQuota(
framework1.role(),
createQuota(framework1.role(),
"disk:10"));



src/tests/hierarchical_allocator_tests.cpp (line 2371)


s/Setting/Setting a



src/tests/hierarchical_allocator_tests.cpp (lines 2390 - 2395)


4 spaces



src/tests/hierarchical_allocator_tests.cpp (lines 2400 - 2403)


4 spaces



src/tests/hierarchical_allocator_tests.cpp (lines 2416 - 2418)


4 spaces


- Guangya Liu


On 二月 23, 2016, 5:09 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43881/
> ---
> 
> (Updated 二月 23, 2016, 5:09 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4719
> https://issues.apache.org/jira/browse/MESOS-4719
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metric for the number of allocations to a framework.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 0d39d3f3b5f4ff7f62f9de7200d062845c71818a 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 1af5c9870430eb05ca0b19ece0ca2957a05093ff 
> 
> Diff: https://reviews.apache.org/r/43881/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43880: Added allocator metrics for total and allocated scalar resources.

[Guangya Liu]

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




src/master/allocator/mesos/hierarchical.cpp (lines 432 - 444)


The allocatd may keeps changing due the resource offer and resource 
recovery, I think that this logic should be in `allocate(slaveIds)`?



src/tests/hierarchical_allocator_tests.cpp (line 2353)


s/Course/Coarse-grained ?

ditto for the following



src/tests/hierarchical_allocator_tests.cpp (line 2384)


move this above `removeSlave`

ditto for the following


- Guangya Liu


On 二月 23, 2016, 5:09 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43880/
> ---
> 
> (Updated 二月 23, 2016, 5:09 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4720
> https://issues.apache.org/jira/browse/MESOS-4720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for total and allocated scalar resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 0d39d3f3b5f4ff7f62f9de7200d062845c71818a 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 1af5c9870430eb05ca0b19ece0ca2957a05093ff 
> 
> Diff: https://reviews.apache.org/r/43880/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43854: Fixed chdir to an empty directory error.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 22, 2016, 10:52 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43854/
> ---
> 
> (Updated Feb. 22, 2016, 10:52 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed chdir to an empty directory error.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> 1bb5e28c4aefe70f9ebfb76b0a65973aaf7273f6 
> 
> Diff: https://reviews.apache.org/r/43854/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> Tested with /mesos-execute --docker_image=busybox
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 43854: Fixed chdir to an empty directory error.

[Jie Yu]

> On Feb. 23, 2016, 6:40 a.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/isolators/docker/runtime.cpp, line 354
> > 
> >
> > Do we have tests to test the docker runtime isolator?

Yeah, I think he's adding one. Need a few test abstractions to do that.


- Jie


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


On Feb. 22, 2016, 10:52 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43854/
> ---
> 
> (Updated Feb. 22, 2016, 10:52 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed chdir to an empty directory error.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> 1bb5e28c4aefe70f9ebfb76b0a65973aaf7273f6 
> 
> Diff: https://reviews.apache.org/r/43854/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> Tested with /mesos-execute --docker_image=busybox
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 43799: WIP: Removed race condition from libevent based poll implementation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43799]

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 Feb. 23, 2016, 7:08 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43799/
> ---
> 
> (Updated Feb. 23, 2016, 7:08 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Bugs: MESOS-3271 and MESOS-4711
> https://issues.apache.org/jira/browse/MESOS-3271
> https://issues.apache.org/jira/browse/MESOS-4711
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Patch not ready for commit.
> 
> Under certains circumstances, the future returned by poll is discarded right
> after the event is triggered, this causes the event callback to be called
> before the discard callback which results in an abort signal being raised
> by the libevent library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_poll.cpp 
> 461624ca003e97be5ea9cf956d86fc294e6f1bc1 
> 
> Diff: https://reviews.apache.org/r/43799/diff/
> 
> 
> Testing
> ---
> 
> ```bash
> # On CentOS 6.7 running in virtualbox
> ../configure --enable-ssl --enable-libevent
> make -j4 check
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery" 
> --gtest_repeat=1000
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Review Request 43916: Modified testcase to test `--cgroups_net_cls_secondary_handles` flag.

[Avinash sridharan]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Modified testcase to test `--cgroups_net_cls_secondary_handles` flag.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
653b037c489072f43e53dec01a811a9249dcd660 

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


Testing
---

make check and sudo make check (for NetClsIsolator)


Thanks,

Avinash sridharan

Re: Review Request 43915: Introduced the `--cgroups_net_cls_secondary_handles` flag in the agent.

[Avinash sridharan]

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

(Updated Feb. 23, 2016, 11:21 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Added JIRA.


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


Repository: mesos


Description
---

Introduced the `--cgroups_net_cls_secondary_handles` flag in the agent.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
f05cd55a9e080f44566ec7dcef22f11950496d6f 
  src/slave/flags.hpp 54c1a69d8777f417cdd8f73ce638447d9951ab61 
  src/slave/flags.cpp 855812e9f7cb4b96d4297f4bd5ac5de7f1d3c39a 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 43914: Modified the `NetClsHandleManager` methods to work with ranges.

[Avinash sridharan]

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

(Updated Feb. 23, 2016, 11:18 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Added JIRA.


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


Repository: mesos


Description
---

The `alloc`, `free` and `reserve` methods of `NetClsHandleManager` can
now work with a specified range of secondary handles, instead of
assuming that the entire 64K space of secondary handles is available
for use.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
f05cd55a9e080f44566ec7dcef22f11950496d6f 

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


Testing
---

make and make check.

Also ran sudo make check for the "NetClsIsolatoTest"


Thanks,

Avinash sridharan

Re: Review Request 43661: Added check to not Subscribe if we are already subscribed.

[Anand Mazumdar]

> On Feb. 23, 2016, 10:17 p.m., Vinod Kone wrote:
> > src/scheduler/scheduler.cpp, line 216
> > 
> >
> > Subscribe contains FrameworkInfo. So this check means that a framework 
> > cannot update its FrameworkInfo unless it gets a disconnected callback? Is 
> > there a way for the scheduler force the disconnection?

As per our offline discussion:

The driver also only allowed to update the `FrameworkInfo` after a scheduler 
failover. I would add a comment about this behavior explicitly for the library 
in a subsequent review. Does this sound reasonable?

Feel free to reopen the issue if this does not suffice.


- Anand


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


On Feb. 17, 2016, 7:22 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43661/
> ---
> 
> (Updated Feb. 17, 2016, 7:22 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3570
> https://issues.apache.org/jira/browse/MESOS-3570
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a check to ensure that we don't `Subscribe` again in case we 
> already have a `SubscribedResponse` stream set. With pipelining, we won't be 
> able to send more then 1 subscribe calls on the `Subscribe` connection.
> 
> In case, the scheduler sends two subscribe calls simultaneously before the 
> subscribed response stream could be established. We would invoke the 
> `disconnected`/`connected` callback, making the scheduler subscribe again.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 99a7d0dfff7b0c61decc9ff6d9e6d46ef13a7e75 
> 
> Diff: https://reviews.apache.org/r/43661/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 43913: Modified `NetClsHandleManager` to take a range of secondary handles.

[Avinash sridharan]

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

(Updated Feb. 23, 2016, 11:16 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Added JIRA.


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


Repository: mesos


Description
---

Modified `NetClsHandleManager` to take a range of secondary handles.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
acd61e615951e18dd51a2ce1e746ba733e9159ae 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 43912: Fixed linux fs create dev symlinks.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 23, 2016, 10:52 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43912/
> ---
> 
> (Updated Feb. 23, 2016, 10:52 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Ian Downes, Jie Yu, and Timothy 
> Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed linux fs create dev symlinks.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e 
> 
> Diff: https://reviews.apache.org/r/43912/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang3.6)
> 
> tested with mesos-execute --docker_image=nginx
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 43915: Introduced the `--cgroups_net_cls_secondary_handles` flag in the agent.

[Avinash sridharan]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Introduced the `--cgroups_net_cls_secondary_handles` flag in the agent.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
f05cd55a9e080f44566ec7dcef22f11950496d6f 
  src/slave/flags.hpp 54c1a69d8777f417cdd8f73ce638447d9951ab61 
  src/slave/flags.cpp 855812e9f7cb4b96d4297f4bd5ac5de7f1d3c39a 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Review Request 43914: Modified the `NetClsHandleManager` methods to work with ranges.

[Avinash sridharan]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

The `alloc`, `free` and `reserve` methods of `NetClsHandleManager` can
now work with a specified range of secondary handles, instead of
assuming that the entire 64K space of secondary handles is available
for use.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
f05cd55a9e080f44566ec7dcef22f11950496d6f 

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


Testing
---

make and make check.

Also ran sudo make check for the "NetClsIsolatoTest"


Thanks,

Avinash sridharan

Re: Review Request 43860: Used uri::Fetcher to pull docker images in docker registry puller.

[Gilbert Song]

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


Ship it!




Tested with mesos-execute --docker_image=library/busybox.

- Gilbert Song


On Feb. 23, 2016, 12:12 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43860/
> ---
> 
> (Updated Feb. 23, 2016, 12:12 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-4261 and MESOS-4499
> https://issues.apache.org/jira/browse/MESOS-4261
> https://issues.apache.org/jira/browse/MESOS-4499
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the second step of cleaning/simplying the docker registry puller code.
> 
> Used uri::Fetcher to pull docker images in docker registry puller.
> 
> The new code uses the uri::Fetcher to download docker manifest and blobs 
> (instead of writting our own http logic which requires enabling of SSL for 
> Mesos).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 5b2d72c22fcbcc379b4901607cf3eb682de66206 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> a239b97557ad20353c67050dbc89ef16da898330 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> bccbac3357cf942446604e6cf5d16c3d594b 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 3fcf1471a035e35a2cac22442655ad65a84a9793 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 2f1d3e002140f34c646aab445a419c9c3d712f99 
>   src/slave/flags.hpp 54c1a69d8777f417cdd8f73ce638447d9951ab61 
>   src/slave/flags.cpp 855812e9f7cb4b96d4297f4bd5ac5de7f1d3c39a 
>   src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 4db6793a21abbb7ea4de0d0fca0431237d38d013 
> 
> Diff: https://reviews.apache.org/r/43860/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 43860: Used uri::Fetcher to pull docker images in docker registry puller.

[Jie Yu]

- Jie


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


On Feb. 23, 2016, 8:12 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43860/
> ---
> 
> (Updated Feb. 23, 2016, 8:12 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-4261 and MESOS-4499
> https://issues.apache.org/jira/browse/MESOS-4261
> https://issues.apache.org/jira/browse/MESOS-4499
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the second step of cleaning/simplying the docker registry puller code.
> 
> Used uri::Fetcher to pull docker images in docker registry puller.
> 
> The new code uses the uri::Fetcher to download docker manifest and blobs 
> (instead of writting our own http logic which requires enabling of SSL for 
> Mesos).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 5b2d72c22fcbcc379b4901607cf3eb682de66206 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> a239b97557ad20353c67050dbc89ef16da898330 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> bccbac3357cf942446604e6cf5d16c3d594b 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 3fcf1471a035e35a2cac22442655ad65a84a9793 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 2f1d3e002140f34c646aab445a419c9c3d712f99 
>   src/slave/flags.hpp 54c1a69d8777f417cdd8f73ce638447d9951ab61 
>   src/slave/flags.cpp 855812e9f7cb4b96d4297f4bd5ac5de7f1d3c39a 
>   src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 4db6793a21abbb7ea4de0d0fca0431237d38d013 
> 
> Diff: https://reviews.apache.org/r/43860/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 43860: Used uri::Fetcher to pull docker images in docker registry puller.

[Jie Yu]

> On Feb. 23, 2016, 7:41 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp, line 
> > 258
> > 
> >
> > Not yours but do we need a CHECK here or a graceful  failure.

I think that's an impossible case. So assertion here sounds fine to me.


- Jie


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


On Feb. 23, 2016, 8:12 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43860/
> ---
> 
> (Updated Feb. 23, 2016, 8:12 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-4261 and MESOS-4499
> https://issues.apache.org/jira/browse/MESOS-4261
> https://issues.apache.org/jira/browse/MESOS-4499
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the second step of cleaning/simplying the docker registry puller code.
> 
> Used uri::Fetcher to pull docker images in docker registry puller.
> 
> The new code uses the uri::Fetcher to download docker manifest and blobs 
> (instead of writting our own http logic which requires enabling of SSL for 
> Mesos).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 5b2d72c22fcbcc379b4901607cf3eb682de66206 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> a239b97557ad20353c67050dbc89ef16da898330 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> bccbac3357cf942446604e6cf5d16c3d594b 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 3fcf1471a035e35a2cac22442655ad65a84a9793 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 2f1d3e002140f34c646aab445a419c9c3d712f99 
>   src/slave/flags.hpp 54c1a69d8777f417cdd8f73ce638447d9951ab61 
>   src/slave/flags.cpp 855812e9f7cb4b96d4297f4bd5ac5de7f1d3c39a 
>   src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 4db6793a21abbb7ea4de0d0fca0431237d38d013 
> 
> Diff: https://reviews.apache.org/r/43860/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 43860: Used uri::Fetcher to pull docker images in docker registry puller.

[Jie Yu]

> On Feb. 23, 2016, 7:45 a.m., Guangya Liu wrote:
> > Some doc may also need to be changed by removing the ssl related 
> > configurations, such as 
> > 
> > https://github.com/apache/mesos/blob/master/docs/endpoints/slave/state.json.md
> > https://github.com/apache/mesos/blob/master/docs/endpoints/slave/state.md
> > https://github.com/apache/mesos/blob/master/docs/configuration.md

Done.


> On Feb. 23, 2016, 7:45 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp, line 
> > 180
> > 
> >
> > what about highlighting the words with `` but not ''?

Have we agreed on that? I use '' all the time.


- Jie


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


On Feb. 23, 2016, 8:12 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43860/
> ---
> 
> (Updated Feb. 23, 2016, 8:12 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-4261 and MESOS-4499
> https://issues.apache.org/jira/browse/MESOS-4261
> https://issues.apache.org/jira/browse/MESOS-4499
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the second step of cleaning/simplying the docker registry puller code.
> 
> Used uri::Fetcher to pull docker images in docker registry puller.
> 
> The new code uses the uri::Fetcher to download docker manifest and blobs 
> (instead of writting our own http logic which requires enabling of SSL for 
> Mesos).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 5b2d72c22fcbcc379b4901607cf3eb682de66206 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> a239b97557ad20353c67050dbc89ef16da898330 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> bccbac3357cf942446604e6cf5d16c3d594b 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 3fcf1471a035e35a2cac22442655ad65a84a9793 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 2f1d3e002140f34c646aab445a419c9c3d712f99 
>   src/slave/flags.hpp 54c1a69d8777f417cdd8f73ce638447d9951ab61 
>   src/slave/flags.cpp 855812e9f7cb4b96d4297f4bd5ac5de7f1d3c39a 
>   src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 4db6793a21abbb7ea4de0d0fca0431237d38d013 
> 
> Diff: https://reviews.apache.org/r/43860/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 43913: Modified `NetClsHandleManager` to take a range of secondary handles.

[Avinash sridharan]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Modified `NetClsHandleManager` to take a range of secondary handles.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
acd61e615951e18dd51a2ce1e746ba733e9159ae 

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


Testing
---

make


Thanks,

Avinash sridharan

Review Request 43912: Fixed linux fs create dev symlinks.

[Gilbert Song]

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

Review request for mesos, Artem Harutyunyan, Ian Downes, Jie Yu, and Timothy 
Chen.


Summary (updated)
-

Fixed linux fs create dev symlinks.


Repository: mesos


Description (updated)
---

Fixed linux fs create dev symlinks.


Diffs (updated)
-

  src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e 

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


Testing (updated)
---

make check (ubuntu14.04 + clang3.6)

tested with mesos-execute --docker_image=nginx


Thanks,

Gilbert Song

Re: Review Request 43407: CMake: Force GMock and libevent to build and link statically.

[Alex Clemmer]

> On Feb. 11, 2016, 9:31 p.m., M Lawindi wrote:
> > 3rdparty/libprocess/3rdparty/CMakeLists.txt, line 182
> > 
> >
> > Fix spaces/tabs

Lawindi and I talked on Slack, and concluded there isn't a tabs issue after 
all. But, I will break this up so it fits in 80 character lines.


- Alex


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


On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43407/
> ---
> 
> (Updated Feb. 18, 2016, 12:59 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Force GMock to build and link statically.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 9b61376ea6aad304607c20c9823d9ef19013eca0 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
> 
> Diff: https://reviews.apache.org/r/43407/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43822: Updated `/slaves` master endpoint to use jsonify.

[Neil Conway]

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

(Updated Feb. 23, 2016, 10:29 p.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


Repository: mesos


Description
---

Updated `/slaves` master endpoint to use jsonify.


Diffs (updated)
-

  src/master/http.cpp 939fab21a2240de7214ef809a194ffb3837a9f1b 

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


Testing
---

make check. Also verified that this endpoint is covered by the unit tests.


Thanks,

Neil Conway

Re: Review Request 43817: Removed no-longer-used model functions.

[Neil Conway]

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

(Updated Feb. 23, 2016, 10:29 p.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


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


Repository: mesos


Description
---

These `model()` variants provided functionality that is now
implemented via jsonify; no more call-sites of the old functions
remain.


Diffs (updated)
-

  src/master/http.cpp 939fab21a2240de7214ef809a194ffb3837a9f1b 

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


Testing
---

make check

Also updated a few comments to not refer to `summarize`.


Thanks,

Neil Conway

Re: Review Request 43823: Updated `/tasks` master endpoint to use jsonify.

[Neil Conway]

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

(Updated Feb. 23, 2016, 10:29 p.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


Repository: mesos


Description
---

Updated `/tasks` master endpoint to use jsonify.


Diffs (updated)
-

  src/master/http.cpp 939fab21a2240de7214ef809a194ffb3837a9f1b 

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


Testing
---

make check. Also verified that this endpoint is covered by the unit tests.


Thanks,

Neil Conway

Review Request 43910: Enhanced a test case for the `/state` agent endpoint.

[Neil Conway]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Enhanced a test case for the `/state` agent endpoint.


Diffs
-

  src/tests/slave_tests.cpp c7f5a701eff2c2f9aa3df5722583a131bf2c072a 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 43816: Updated `/frameworks` master endpoint to use jsonify.

[Neil Conway]

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

(Updated Feb. 23, 2016, 10:29 p.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Updated `/frameworks` master endpoint to use jsonify.


Diffs (updated)
-

  src/master/http.cpp 939fab21a2240de7214ef809a194ffb3837a9f1b 

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


Testing
---

1. make check
2. Verified that after introducing a bug into the jsonify version of 
`frameworks()`, `make check` fails (i.e., the test suite covers the 
`/frameworks` endpoint).
3. Compared output of `/frameworks` with old and new implementation on a test 
Mesos installation to try to gauge correctness visually.


Thanks,

Neil Conway

Review Request 43911: Updated `/state` agent endpoint to use jsonify.

[Neil Conway]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Updated `/state` agent endpoint to use jsonify.


Diffs
-

  src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 43661: Added check to not Subscribe if we are already subscribed.

[Vinod Kone]

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




src/scheduler/scheduler.cpp (line 216)


Subscribe contains FrameworkInfo. So this check means that a framework 
cannot update its FrameworkInfo unless it gets a disconnected callback? Is 
there a way for the scheduler force the disconnection?


- Vinod Kone


On Feb. 17, 2016, 7:22 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43661/
> ---
> 
> (Updated Feb. 17, 2016, 7:22 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3570
> https://issues.apache.org/jira/browse/MESOS-3570
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a check to ensure that we don't `Subscribe` again in case we 
> already have a `SubscribedResponse` stream set. With pipelining, we won't be 
> able to send more then 1 subscribe calls on the `Subscribe` connection.
> 
> In case, the scheduler sends two subscribe calls simultaneously before the 
> subscribed response stream could be established. We would invoke the 
> `disconnected`/`connected` callback, making the scheduler subscribe again.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 99a7d0dfff7b0c61decc9ff6d9e6d46ef13a7e75 
> 
> Diff: https://reviews.apache.org/r/43661/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 43660: Renamed Connection to SubscribedResponse.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Feb. 17, 2016, 7:22 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43660/
> ---
> 
> (Updated Feb. 17, 2016, 7:22 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3570
> https://issues.apache.org/jira/browse/MESOS-3570
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change renames the `Connection` object that existed previously to 
> `SubscribedResponse`. This would be needed later in the review chain when we 
> implement pipelining.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 99a7d0dfff7b0c61decc9ff6d9e6d46ef13a7e75 
> 
> Diff: https://reviews.apache.org/r/43660/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 43659: Cleaned up the previous queueing Calls logic.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Feb. 17, 2016, 7:22 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43659/
> ---
> 
> (Updated Feb. 17, 2016, 7:22 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3570
> https://issues.apache.org/jira/browse/MESOS-3570
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change cleans up the previous queueing calls logic in favor of HTTP 
> connection pipelining in the scheduler library. This review clears up the 
> noise of the pipelining diff in subsequent reviews.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 99a7d0dfff7b0c61decc9ff6d9e6d46ef13a7e75 
> 
> Diff: https://reviews.apache.org/r/43659/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 43798: Added overview section to upgrades.md.

[Joerg Schad]

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

(Updated Feb. 23, 2016, 9:39 p.m.)


Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.


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


Repository: mesos


Description
---

Added overview section to upgrades.md.


Diffs (updated)
-

  docs/upgrades.md 4f30d725c6ed28c09a1c5528fd4193c3f06b2d93 

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


Testing
---

Viewed via gist (https://gist.github.com/joerg84/eddbc0302a5a4b291e81) and 
docker website container.


Thanks,

Joerg Schad

Re: Review Request 43801: Refactored and simplified the docker puller interfaces.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Feb. 23, 2016, 12:11 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43801/
> ---
> 
> (Updated Feb. 23, 2016, 12:11 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Timothy Chen.
> 
> 
> Bugs: MESOS-4499
> https://issues.apache.org/jira/browse/MESOS-4499
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the first step of cleaning up the docker registry puller/client code 
> base. I did the following simplications:
> 1) Simplify the paths functions. The getImageArchiveXXX functions are 
> redundant.
> 2) Simplify the Puller::pull interface. We just need to return layer ids and 
> no need to return path.
> 3) Used untar from command utils, and kill the same function in puller.cpp.
> 4) A lot of consistency fixes, including using the same type and name.
> 5) A lot of style fixes.
>   
> The next step will be to cleanup and refactor registry_puller to use uri 
> Fetcher (docker plugin) to download manifest and blobs (and kill registry 
> client).
> 
> Reviewers are recommended to look at the end version, instead of the diff.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 811c24b43f7aec9db406dd521770c6cd82097c92 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f3e7c042f2c9980f6274c8185ee8bb89a8b02002 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
> d2b0cf95e5ed7664071484a27a2a1f9bafff70d6 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
> 82d92a2a9cce181eb283395a32e93cbb1586703b 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 5b2d72c22fcbcc379b4901607cf3eb682de66206 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> a239b97557ad20353c67050dbc89ef16da898330 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> bccbac3357cf942446604e6cf5d16c3d594b 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 3fcf1471a035e35a2cac22442655ad65a84a9793 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> 880e216e33dd178d0baa47d3958c84cda4d9e25e 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 2f1d3e002140f34c646aab445a419c9c3d712f99 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 4db6793a21abbb7ea4de0d0fca0431237d38d013 
> 
> Diff: https://reviews.apache.org/r/43801/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 43889: CMAKE: Add leveldb library to 3rdparty external builds

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [43889]

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

Error:
2016-02-23 21:11:52 URL:https://reviews.apache.org/r/43889/diff/raw/ 
[8229/8229] -> "43889.patch" [1]
43889.patch:137: trailing whitespace.
 
warning: 1 line adds whitespace errors.
3rdparty/libprocess/cmake/macros/External.cmake:57: trailing whitespace.
+ 

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

- Mesos ReviewBot


On Feb. 23, 2016, 6:13 p.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43889/
> ---
> 
> (Updated Feb. 23, 2016, 6:13 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-4647
> https://issues.apache.org/jira/browse/MESOS-4647
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMAKE: Add leveldb library to 3rdparty external builds
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
> 0c80fb8d799ea1252492cd98ac0780f1228aadcd 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 3a2e0999722007475c023ade75719093e35cfc80 
>   3rdparty/libprocess/cmake/macros/External.cmake 
> e3901b67048f1c028216ae8323ee1c318a46f3cc 
> 
> Diff: https://reviews.apache.org/r/43889/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 43893: Made sure that spawned processes terminated before leaving test.

[Alexander Rojas]

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


Ship it!




I feel there are a bunch of tests around that do not verify certain events 
actually happen before continuing. We should probably go and look into that.

- Alexander Rojas


On Feb. 23, 2016, 8:50 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43893/
> ---
> 
> (Updated Feb. 23, 2016, 8:50 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4747
> https://issues.apache.org/jira/browse/MESOS-4747
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made sure that spawned processes terminated before leaving tests.
> 
> 
> Diffs
> -
> 
>   src/tests/container_logger_tests.cpp 
> 6e2b8c1b80d4b49a1042c53945c223785d0d4a3f 
> 
> Diff: https://reviews.apache.org/r/43893/diff/
> 
> 
> Testing
> ---
> 
> make check
> make check GTEST_FILTER=ContainerLoggerTest.MesosContainerizerRecover
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43884: Added allocator metrics for satisfied quotas.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43879, 43880, 43881, 43882, 43883, 43884]

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 Feb. 23, 2016, 5:08 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43884/
> ---
> 
> (Updated Feb. 23, 2016, 5:08 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4723
> https://issues.apache.org/jira/browse/MESOS-4723
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for satisfied quotas.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 0d39d3f3b5f4ff7f62f9de7200d062845c71818a 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 1af5c9870430eb05ca0b19ece0ca2957a05093ff 
> 
> Diff: https://reviews.apache.org/r/43884/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43860: Used uri::Fetcher to pull docker images in docker registry puller.

[Jie Yu]

> On Feb. 23, 2016, 7:27 p.m., Gilbert Song wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp, line 1267
> > 
> >
> > Could we do `EXPECT_EQ(2u, imageInfo->layers.size())` ?

We don't know that for sure, right:)


- Jie


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


On Feb. 23, 2016, 8:12 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43860/
> ---
> 
> (Updated Feb. 23, 2016, 8:12 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-4261 and MESOS-4499
> https://issues.apache.org/jira/browse/MESOS-4261
> https://issues.apache.org/jira/browse/MESOS-4499
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the second step of cleaning/simplying the docker registry puller code.
> 
> Used uri::Fetcher to pull docker images in docker registry puller.
> 
> The new code uses the uri::Fetcher to download docker manifest and blobs 
> (instead of writting our own http logic which requires enabling of SSL for 
> Mesos).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 5b2d72c22fcbcc379b4901607cf3eb682de66206 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> a239b97557ad20353c67050dbc89ef16da898330 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> bccbac3357cf942446604e6cf5d16c3d594b 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 3fcf1471a035e35a2cac22442655ad65a84a9793 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 2f1d3e002140f34c646aab445a419c9c3d712f99 
>   src/slave/flags.hpp 54c1a69d8777f417cdd8f73ce638447d9951ab61 
>   src/slave/flags.cpp 855812e9f7cb4b96d4297f4bd5ac5de7f1d3c39a 
>   src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 4db6793a21abbb7ea4de0d0fca0431237d38d013 
> 
> Diff: https://reviews.apache.org/r/43860/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 43896: Removed the restriction that /tmp needs to be writable in new rootfs.

[Jie Yu]

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

Review request for mesos, Gilbert Song, Ian Downes, Jojy Varghese, and Timothy 
Chen.


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


Repository: mesos


Description
---

Removed the restriction that /tmp needs to be writable in new rootfs.


Diffs
-

  src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 43798: Added overview section to upgrades.md.

[Neil Conway]

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




docs/upgrades.md (line 13)


Seems like this isn't a table any more? Either a table or a bulleted list 
would be good.


- Neil Conway


On Feb. 23, 2016, 7:31 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43798/
> ---
> 
> (Updated Feb. 23, 2016, 7:31 p.m.)
> 
> 
> Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4381
> https://issues.apache.org/jira/browse/MESOS-4381
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added overview section to upgrades.md.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 4f30d725c6ed28c09a1c5528fd4193c3f06b2d93 
> 
> Diff: https://reviews.apache.org/r/43798/diff/
> 
> 
> Testing
> ---
> 
> Viewed via gist (https://gist.github.com/joerg84/eddbc0302a5a4b291e81) and 
> docker website container.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43860: Used uri::Fetcher to pull docker images in docker registry puller.

[Jie Yu]

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

(Updated Feb. 23, 2016, 8:12 p.m.)


Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.


Bugs: MESOS-4261 and MESOS-4499
https://issues.apache.org/jira/browse/MESOS-4261
https://issues.apache.org/jira/browse/MESOS-4499


Repository: mesos


Description (updated)
---

This is the second step of cleaning/simplying the docker registry puller code.

Used uri::Fetcher to pull docker images in docker registry puller.

The new code uses the uri::Fetcher to download docker manifest and blobs 
(instead of writting our own http logic which requires enabling of SSL for 
Mesos).


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
5b2d72c22fcbcc379b4901607cf3eb682de66206 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
a239b97557ad20353c67050dbc89ef16da898330 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
bccbac3357cf942446604e6cf5d16c3d594b 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
3fcf1471a035e35a2cac22442655ad65a84a9793 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
2f1d3e002140f34c646aab445a419c9c3d712f99 
  src/slave/flags.hpp 54c1a69d8777f417cdd8f73ce638447d9951ab61 
  src/slave/flags.cpp 855812e9f7cb4b96d4297f4bd5ac5de7f1d3c39a 
  src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e 
  src/tests/containerizer/provisioner_docker_tests.cpp 
4db6793a21abbb7ea4de0d0fca0431237d38d013 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 43860: Used uri::Fetcher to pull docker images in docker registry puller.

[Jie Yu]

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

(Updated Feb. 23, 2016, 8:11 p.m.)


Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.


Changes
---

Rebased.


Bugs: MESOS-4261 and MESOS-4499
https://issues.apache.org/jira/browse/MESOS-4261
https://issues.apache.org/jira/browse/MESOS-4499


Repository: mesos


Description (updated)
---

Used uri::Fetcher to pull docker images in docker registry puller.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
5b2d72c22fcbcc379b4901607cf3eb682de66206 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
a239b97557ad20353c67050dbc89ef16da898330 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
bccbac3357cf942446604e6cf5d16c3d594b 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
3fcf1471a035e35a2cac22442655ad65a84a9793 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
2f1d3e002140f34c646aab445a419c9c3d712f99 
  src/slave/flags.hpp 54c1a69d8777f417cdd8f73ce638447d9951ab61 
  src/slave/flags.cpp 855812e9f7cb4b96d4297f4bd5ac5de7f1d3c39a 
  src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e 
  src/tests/containerizer/provisioner_docker_tests.cpp 
4db6793a21abbb7ea4de0d0fca0431237d38d013 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 43801: Refactored and simplified the docker puller interfaces.

[Jie Yu]

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

(Updated Feb. 23, 2016, 8:11 p.m.)


Review request for mesos, Gilbert Song and Timothy Chen.


Changes
---

Rebased.


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


Repository: mesos


Description (updated)
---

This is the first step of cleaning up the docker registry puller/client code 
base. I did the following simplications:
1) Simplify the paths functions. The getImageArchiveXXX functions are redundant.
2) Simplify the Puller::pull interface. We just need to return layer ids and no 
need to return path.
3) Used untar from command utils, and kill the same function in puller.cpp.
4) A lot of consistency fixes, including using the same type and name.
5) A lot of style fixes.

The next step will be to cleanup and refactor registry_puller to use uri 
Fetcher (docker plugin) to download manifest and blobs (and kill registry 
client).

Reviewers are recommended to look at the end version, instead of the diff.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
811c24b43f7aec9db406dd521770c6cd82097c92 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
f3e7c042f2c9980f6274c8185ee8bb89a8b02002 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
d2b0cf95e5ed7664071484a27a2a1f9bafff70d6 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
82d92a2a9cce181eb283395a32e93cbb1586703b 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
5b2d72c22fcbcc379b4901607cf3eb682de66206 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
a239b97557ad20353c67050dbc89ef16da898330 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
bccbac3357cf942446604e6cf5d16c3d594b 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
3fcf1471a035e35a2cac22442655ad65a84a9793 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 
880e216e33dd178d0baa47d3958c84cda4d9e25e 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
2f1d3e002140f34c646aab445a419c9c3d712f99 
  src/tests/containerizer/provisioner_docker_tests.cpp 
4db6793a21abbb7ea4de0d0fca0431237d38d013 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 43801: Refactored and simplified the docker puller interfaces.

[Jie Yu]

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

(Updated Feb. 23, 2016, 8:10 p.m.)


Review request for mesos, Gilbert Song and Timothy Chen.


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


Repository: mesos


Description (updated)
---

Refactored and simplified the docker puller interfaces.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
811c24b43f7aec9db406dd521770c6cd82097c92 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
f3e7c042f2c9980f6274c8185ee8bb89a8b02002 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
d2b0cf95e5ed7664071484a27a2a1f9bafff70d6 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
82d92a2a9cce181eb283395a32e93cbb1586703b 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
5b2d72c22fcbcc379b4901607cf3eb682de66206 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
a239b97557ad20353c67050dbc89ef16da898330 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
bccbac3357cf942446604e6cf5d16c3d594b 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
3fcf1471a035e35a2cac22442655ad65a84a9793 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 
880e216e33dd178d0baa47d3958c84cda4d9e25e 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
2f1d3e002140f34c646aab445a419c9c3d712f99 
  src/tests/containerizer/provisioner_docker_tests.cpp 
4db6793a21abbb7ea4de0d0fca0431237d38d013 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 43801: Refactored and simplified the docker puller interfaces.

[Jie Yu]

> On Feb. 23, 2016, 7:26 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/docker/paths.hpp, line 35
> > 
> >
> > Why removing this? I think that this still exist as a temp directory 
> > when pulling docker image?

This is not in the staging root. Also, there is no such thing if registry 
puller is used.


- Jie


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


On Feb. 20, 2016, 6:09 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43801/
> ---
> 
> (Updated Feb. 20, 2016, 6:09 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Timothy Chen.
> 
> 
> Bugs: MESOS-4499
> https://issues.apache.org/jira/browse/MESOS-4499
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the first step of cleaning up the docker registry puller/client code 
> base. I did the following simplications:
> 
> 1) Simplify the paths functions. The getImageArchiveXXX functions are 
> redundant.
> 2) Simplify the Puller::pull interface. We just need to return layer ids and 
> no need to return path.
> 3) Used untar from command utils, and kill the same function in puller.cpp.
> 4) A lot of consistency fixes, including using the same type and name.
> 5) A lot of style fixes.
> 
> The next step will be to cleanup and refactor registry_puller to use uri 
> Fetcher (docker plugin) to download manifest and blobs (and kill registry 
> client).
> 
> Reviewers are recommended to look at the end version, instead of the diff.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 811c24b43f7aec9db406dd521770c6cd82097c92 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f3e7c042f2c9980f6274c8185ee8bb89a8b02002 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
> d2b0cf95e5ed7664071484a27a2a1f9bafff70d6 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
> 82d92a2a9cce181eb283395a32e93cbb1586703b 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 5b2d72c22fcbcc379b4901607cf3eb682de66206 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> a239b97557ad20353c67050dbc89ef16da898330 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> bccbac3357cf942446604e6cf5d16c3d594b 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 3fcf1471a035e35a2cac22442655ad65a84a9793 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> 880e216e33dd178d0baa47d3958c84cda4d9e25e 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 2f1d3e002140f34c646aab445a419c9c3d712f99 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 4db6793a21abbb7ea4de0d0fca0431237d38d013 
> 
> Diff: https://reviews.apache.org/r/43801/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 43801: Refactored and simplified the docker puller interfaces.

[Jie Yu]

> On Feb. 23, 2016, 7:26 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/docker/paths.hpp, lines 38-40
> > 
> >
> > Do we need to add layer.tar here?

That's a impl. detail. We would likely to remove it later. So I won't mention 
it here.


- Jie


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


On Feb. 20, 2016, 6:09 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43801/
> ---
> 
> (Updated Feb. 20, 2016, 6:09 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Timothy Chen.
> 
> 
> Bugs: MESOS-4499
> https://issues.apache.org/jira/browse/MESOS-4499
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the first step of cleaning up the docker registry puller/client code 
> base. I did the following simplications:
> 
> 1) Simplify the paths functions. The getImageArchiveXXX functions are 
> redundant.
> 2) Simplify the Puller::pull interface. We just need to return layer ids and 
> no need to return path.
> 3) Used untar from command utils, and kill the same function in puller.cpp.
> 4) A lot of consistency fixes, including using the same type and name.
> 5) A lot of style fixes.
> 
> The next step will be to cleanup and refactor registry_puller to use uri 
> Fetcher (docker plugin) to download manifest and blobs (and kill registry 
> client).
> 
> Reviewers are recommended to look at the end version, instead of the diff.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 811c24b43f7aec9db406dd521770c6cd82097c92 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f3e7c042f2c9980f6274c8185ee8bb89a8b02002 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
> d2b0cf95e5ed7664071484a27a2a1f9bafff70d6 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
> 82d92a2a9cce181eb283395a32e93cbb1586703b 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 5b2d72c22fcbcc379b4901607cf3eb682de66206 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> a239b97557ad20353c67050dbc89ef16da898330 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> bccbac3357cf942446604e6cf5d16c3d594b 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 3fcf1471a035e35a2cac22442655ad65a84a9793 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> 880e216e33dd178d0baa47d3958c84cda4d9e25e 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 2f1d3e002140f34c646aab445a419c9c3d712f99 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 4db6793a21abbb7ea4de0d0fca0431237d38d013 
> 
> Diff: https://reviews.apache.org/r/43801/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 43893: Made sure that spawned processes terminated before leaving test.

[Joseph Wu]

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


Ship it!




LGTM!

- Joseph Wu


On Feb. 23, 2016, 11:50 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43893/
> ---
> 
> (Updated Feb. 23, 2016, 11:50 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4747
> https://issues.apache.org/jira/browse/MESOS-4747
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made sure that spawned processes terminated before leaving tests.
> 
> 
> Diffs
> -
> 
>   src/tests/container_logger_tests.cpp 
> 6e2b8c1b80d4b49a1042c53945c223785d0d4a3f 
> 
> Diff: https://reviews.apache.org/r/43893/diff/
> 
> 
> Testing
> ---
> 
> make check
> make check GTEST_FILTER=ContainerLoggerTest.MesosContainerizerRecover
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43893: Made sure that spawned processes terminated before leaving test.

[Benjamin Bannier]

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

(Updated Feb. 23, 2016, 8:50 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Offline comments from Joseph.


Summary (updated)
-

Made sure that spawned processes terminated before leaving test.


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


Repository: mesos


Description (updated)
---

Made sure that spawned processes terminated before leaving tests.


Diffs (updated)
-

  src/tests/container_logger_tests.cpp 6e2b8c1b80d4b49a1042c53945c223785d0d4a3f 

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


Testing
---

make check
make check GTEST_FILTER=ContainerLoggerTest.MesosContainerizerRecover


Thanks,

Benjamin Bannier

Review Request 43893: Made sure that all spawn processes have terminated before leaving test.

[Benjamin Bannier]

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

Review request for mesos.


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


Repository: mesos


Description
---

Made sure that all spawned processes terminate before leaving test.


Diffs
-

  src/tests/container_logger_tests.cpp 6e2b8c1b80d4b49a1042c53945c223785d0d4a3f 

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


Testing
---

make check
make check GTEST_FILTER=ContainerLoggerTest.MesosContainerizerRecover


Thanks,

Benjamin Bannier

Re: Review Request 43860: Used uri::Fetcher to pull docker images in docker registry puller.

[Jojy Varghese]

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




src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp (line 214)


Not yours but do we need a CHECK here or a graceful  failure.



src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp (line 256)


Does not apply for this patch, but  might have to change the port type to 
posix type `in_port_t `


- Jojy Varghese


On Feb. 23, 2016, 12:54 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43860/
> ---
> 
> (Updated Feb. 23, 2016, 12:54 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-4261 and MESOS-4499
> https://issues.apache.org/jira/browse/MESOS-4261
> https://issues.apache.org/jira/browse/MESOS-4499
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the second step of cleaning/simplying the docker registry puller code.
> 
> The new code uses the uri::Fetcher to download docker manifest and blobs 
> (instead of writting our own http logic which requires enabling of SSL for 
> Mesos).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 5b2d72c22fcbcc379b4901607cf3eb682de66206 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> a239b97557ad20353c67050dbc89ef16da898330 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> bccbac3357cf942446604e6cf5d16c3d594b 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 3fcf1471a035e35a2cac22442655ad65a84a9793 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 2f1d3e002140f34c646aab445a419c9c3d712f99 
>   src/slave/flags.hpp 54c1a69d8777f417cdd8f73ce638447d9951ab61 
>   src/slave/flags.cpp 855812e9f7cb4b96d4297f4bd5ac5de7f1d3c39a 
>   src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 4db6793a21abbb7ea4de0d0fca0431237d38d013 
> 
> Diff: https://reviews.apache.org/r/43860/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 43798: Added overview section to upgrades.md.

[Joerg Schad]

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

(Updated Feb. 23, 2016, 7:31 p.m.)


Review request for mesos, Michael Park and Neil Conway.


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


Repository: mesos


Description
---

Added overview section to upgrades.md.


Diffs (updated)
-

  docs/upgrades.md 4f30d725c6ed28c09a1c5528fd4193c3f06b2d93 

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


Testing
---

Viewed via gist (https://gist.github.com/joerg84/eddbc0302a5a4b291e81) and 
docker website container.


Thanks,

Joerg Schad

Re: Review Request 43615: Update test suite to use the reworked MesosTest helpers.

[Joseph Wu]

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

(Updated Feb. 23, 2016, 11:29 a.m.)


Review request for mesos, Bernd Mathiske and Artem Harutyunyan.


Changes
---

Rebase and address new/modified reservation tests.


Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634


Repository: mesos


Description
---

Includes the following changes:

* Added the `` header where appropriate.
* Added the namespace `using process::Owned;` where appropriate.
* Generally replaced `Try` with `Owned`.  And 
`Try` with `Owned`.
* Added the (now required) `MasterDetector` argument to all slaves.  Before, 
this was fetched from the first master in `Cluster`.
* Removed `Shutdown();` from all tests.
* Replaced `Stop(...)` with the appropriate master/slave destruction calls.
* Wrap various slave objects in `Owned` (i.e. containerizers, isolators, 
launchers, etc).
* Replace `CHECK` in tests with `ASSERT`.


Diffs (updated)
-

  src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
  src/tests/command_executor_tests.cpp 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 
  src/tests/container_logger_tests.cpp 6e2b8c1b80d4b49a1042c53945c223785d0d4a3f 
  src/tests/containerizer/docker_containerizer_tests.cpp 
a299c9e0744b5657984e5bb0afbe4874a266ddb6 
  src/tests/containerizer/external_containerizer_test.cpp 
8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
6a60962b4593b3521c182c7320331743ccffd4ba 
  src/tests/containerizer/isolator_tests.cpp 
653b037c489072f43e53dec01a811a9249dcd660 
  src/tests/containerizer/memory_pressure_tests.cpp 
4a03af2c9c0643d964b1d76e2096341b59bf5dce 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
  src/tests/containerizer/port_mapping_tests.cpp 
983a6be160aefe5a32acb6111bb3c85230ec 
  src/tests/credentials_tests.cpp 7edcc857e0f6f8e80e265deeec59d6349d392224 
  src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
  src/tests/exception_tests.cpp 6b71316d545e97f14a45daa14d0fd95204befd3b 
  src/tests/executor_http_api_tests.cpp 
36a042ed103271ca873450236f39a8152fbbf07e 
  src/tests/fault_tolerance_tests.cpp 982468f851cd9d95eb6cde7c57f2d737d46a827c 
  src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
  src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
  src/tests/hook_tests.cpp 59a00ea722a17bbc82b14c69bda826f68cbac6e9 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_authorization_tests.cpp 
29c89fb11da792c3e71eb880a19657ea225b3cc8 
  src/tests/master_contender_detector_tests.cpp 
255ab8119a04b55bb4f1b61dee19c4be64499376 
  src/tests/master_maintenance_tests.cpp 
b98eedec388813ee795dd83ccc5ff27338209475 
  src/tests/master_quota_tests.cpp 8357ec911b2a158632a708ae3adff6eabc536697 
  src/tests/master_slave_reconciliation_tests.cpp 
d41178eb41df519073fc0890c5716bbc9fed6ad2 
  src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
  src/tests/master_validation_tests.cpp 
ab2df22f73052f6bd77653e56e7b460b17e7b0be 
  src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
  src/tests/monitor_tests.cpp 869c9e032817e8859a968232d4a61556a3d53d45 
  src/tests/oversubscription_tests.cpp d4ae81972fd218c58a413d1968a4e9acbee52fd3 
  src/tests/partition_tests.cpp c5badbe90e302793bfbf3f16373efe241decb7d5 
  src/tests/persistent_volume_endpoints_tests.cpp 
6069ca1e9ed278459c5182e438417e95955b1924 
  src/tests/persistent_volume_tests.cpp 
e169e1b141a38dc389eefd42c11a078c413123d5 
  src/tests/rate_limiting_tests.cpp caced732ded05a334861a53488ef6391885b2263 
  src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
  src/tests/registrar_zookeeper_tests.cpp 
3df9779ee5d076e16f6a538326693a36f986b6d0 
  src/tests/repair_tests.cpp bb104562659e135492f9857e5b452c8a0a9e97da 
  src/tests/reservation_endpoints_tests.cpp 
32b2af4115211b58a5127a14dd19152c2eca120c 
  src/tests/reservation_tests.cpp b8878d51767ac0d95e346c44c0a4d5c060e565ef 
  src/tests/resource_offers_tests.cpp 0bad45dd1dabecc88fef1ab46e8ea26718070b33 
  src/tests/role_tests.cpp fc3a72894631279460ee7971a4627d73c3d8c351 
  src/tests/scheduler_driver_tests.cpp f6dc25d82ae5f1e77fc6ede7ff2660ed0d9ea039 
  src/tests/scheduler_event_call_tests.cpp 
bd8920fa9d5475e5f6533c8424ebff1588bfe645 
  src/tests/scheduler_http_api_tests.cpp 
9eb1de7d9541395b92b951f0fe0ddbb2f219fe30 
  src/tests/slave_recovery_tests.cpp e2a78a0f55b7657057ee351a747caff51024fd67 
  src/tests/slave_tests.cpp c7f5a701eff2c2f9aa3df5722583a131bf2c072a 
  src/tests/status_update_manager_tests.cpp 
d64d3b8c96270478f6b681c038de77c3a9eb68fe 
  src/tests/teardown_tests.cpp 

Re: Review Request 43860: Used uri::Fetcher to pull docker images in docker registry puller.

[Gilbert Song]

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



The patch looks good to me. Could you rebase? I want to apply the patch and 
test with mesos-execute.


src/tests/containerizer/provisioner_docker_tests.cpp (line 330)


Could we do `EXPECT_EQ(2u, imageInfo->layers.size())` ?


- Gilbert Song


On Feb. 22, 2016, 4:54 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43860/
> ---
> 
> (Updated Feb. 22, 2016, 4:54 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-4261 and MESOS-4499
> https://issues.apache.org/jira/browse/MESOS-4261
> https://issues.apache.org/jira/browse/MESOS-4499
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the second step of cleaning/simplying the docker registry puller code.
> 
> The new code uses the uri::Fetcher to download docker manifest and blobs 
> (instead of writting our own http logic which requires enabling of SSL for 
> Mesos).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 5b2d72c22fcbcc379b4901607cf3eb682de66206 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> a239b97557ad20353c67050dbc89ef16da898330 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> bccbac3357cf942446604e6cf5d16c3d594b 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 3fcf1471a035e35a2cac22442655ad65a84a9793 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 2f1d3e002140f34c646aab445a419c9c3d712f99 
>   src/slave/flags.hpp 54c1a69d8777f417cdd8f73ce638447d9951ab61 
>   src/slave/flags.cpp 855812e9f7cb4b96d4297f4bd5ac5de7f1d3c39a 
>   src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 4db6793a21abbb7ea4de0d0fca0431237d38d013 
> 
> Diff: https://reviews.apache.org/r/43860/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 43855: Added Appc fetcher support to store.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43855]

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 Feb. 23, 2016, 3:30 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43855/
> ---
> 
> (Updated Feb. 23, 2016, 3:30 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change allows store to fetch an image using Appc image fetcher when an
> image is not found in the cache. It also recursively fetches the dependencies
> for the image.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 4b3829175f57fb9aea2478040d96f2f127cbc551 
>   src/tests/fetcher_cache_tests.cpp e10b3f7ebc21c8c1095564fc40f123087dcf320e 
> 
> Diff: https://reviews.apache.org/r/43855/diff/
> 
> 
> Testing
> ---
> 
> make check; local image server.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 43799: WIP: Removed race condition from libevent based poll implementation.

[Alexander Rojas]

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

(Updated Feb. 23, 2016, 8:08 p.m.)


Review request for mesos, Joris Van Remoortere and Michael Park.


Changes
---

Marking as WIP, there are still issues here.


Summary (updated)
-

WIP: Removed race condition from libevent based poll implementation.


Bugs: MESOS-3271 and MESOS-4711
https://issues.apache.org/jira/browse/MESOS-3271
https://issues.apache.org/jira/browse/MESOS-4711


Repository: mesos


Description (updated)
---

WIP: Patch not ready for commit.

Under certains circumstances, the future returned by poll is discarded right
after the event is triggered, this causes the event callback to be called
before the discard callback which results in an abort signal being raised
by the libevent library.


Diffs
-

  3rdparty/libprocess/src/libevent_poll.cpp 
461624ca003e97be5ea9cf956d86fc294e6f1bc1 

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


Testing
---

```bash
# On CentOS 6.7 running in virtualbox
../configure --enable-ssl --enable-libevent
make -j4 check
sudo ./bin/mesos-tests.sh 
--gtest_filter="MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery" 
--gtest_repeat=1000
```


Thanks,

Alexander Rojas

Re: Review Request 43850: Added watis at different points in the test to ensure milestones.

[Alexander Rojas]

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

(Updated Feb. 23, 2016, 7:46 p.m.)


Review request for mesos, Bernd Mathiske and Joseph Wu.


Summary (updated)
-

Added watis at different points in the test to ensure milestones.


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


Repository: mesos


Description (updated)
---

Sometimes MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery will fail because
the tracker of the cgroups pressure counter is not finished by the time the
expectations are set.

This patch ensures that different test milestones are reached; e.g. slave
is registered, KILLED_TASK reached the scheduler, etc; before continuing
with the test.


Diffs (updated)
-

  src/tests/containerizer/memory_pressure_tests.cpp 
4a03af2c9c0643d964b1d76e2096341b59bf5dce 

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


Testing (updated)
---

On a CentOS 6.7 virtual box machine:

```bash
MESOS_VERBOSE=1 sudo .libs/mesos-tests 
--gtest_filter="MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery" 
--gtest_repeat=1000 --gtest_break_on_failure
```


Thanks,

Alexander Rojas

Re: Review Request 43819: Added Scheduler-Driver API to app-framework-development-guide.md.

[Neil Conway]

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


Fix it, then Ship it!





docs/app-framework-development-guide.md (line 17)


I'd say "framework schedulers" for simplicity.


- Neil Conway


On Feb. 22, 2016, 3 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> ---
> 
> (Updated Feb. 22, 2016, 3 a.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Scheduler-Driver API to app-framework-development-guide.md.
> 
> 
> Diffs
> -
> 
>   docs/app-framework-development-guide.md 
> e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d 
> 
> Diff: https://reviews.apache.org/r/43819/diff/
> 
> 
> Testing
> ---
> 
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and 
> via docker website container.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43384: Fixed minor bug in generate-endpoint-help.py.

[Neil Conway]

> On Feb. 23, 2016, 6:17 p.m., Ben Mahler wrote:
> > support/generate-endpoint-help.py, line 316
> > 
> >
> > How about on_exit?

Sounds good.


- Neil


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


On Feb. 19, 2016, 7:21 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43384/
> ---
> 
> (Updated Feb. 19, 2016, 7:21 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the script exited when `current_subprocess` was None, it would
> print an ugly exception stack trace.
> 
> 
> Diffs
> -
> 
>   support/generate-endpoint-help.py 28333847e5603c942f25ec9d9a0429bd676f4541 
> 
> Diff: https://reviews.apache.org/r/43384/diff/
> 
> 
> Testing
> ---
> 
> Ran the script in error and non-error cases with the changes applied.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43384: Fixed minor bug in generate-endpoint-help.py.

[Ben Mahler]

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


Ship it!





support/generate-endpoint-help.py (line 316)


How about on_exit?


- Ben Mahler


On Feb. 19, 2016, 7:21 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43384/
> ---
> 
> (Updated Feb. 19, 2016, 7:21 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the script exited when `current_subprocess` was None, it would
> print an ugly exception stack trace.
> 
> 
> Diffs
> -
> 
>   support/generate-endpoint-help.py 28333847e5603c942f25ec9d9a0429bd676f4541 
> 
> Diff: https://reviews.apache.org/r/43384/diff/
> 
> 
> Testing
> ---
> 
> Ran the script in error and non-error cases with the changes applied.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43889: CMAKE: Add leveldb library to 3rdparty external builds

[haosdent huang]

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



Hi, I think the bug field for this review should be `MESOS-4746`?

- haosdent huang


On Feb. 23, 2016, 6:13 p.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43889/
> ---
> 
> (Updated Feb. 23, 2016, 6:13 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-4647
> https://issues.apache.org/jira/browse/MESOS-4647
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMAKE: Add leveldb library to 3rdparty external builds
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
> 0c80fb8d799ea1252492cd98ac0780f1228aadcd 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 3a2e0999722007475c023ade75719093e35cfc80 
>   3rdparty/libprocess/cmake/macros/External.cmake 
> e3901b67048f1c028216ae8323ee1c318a46f3cc 
> 
> Diff: https://reviews.apache.org/r/43889/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Review Request 43889: CMAKE: Add leveldb library to 3rdparty external builds

[Diana Arroyo]

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

Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

CMAKE: Add leveldb library to 3rdparty external builds


Diffs
-

  3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
0c80fb8d799ea1252492cd98ac0780f1228aadcd 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
3a2e0999722007475c023ade75719093e35cfc80 
  3rdparty/libprocess/cmake/macros/External.cmake 
e3901b67048f1c028216ae8323ee1c318a46f3cc 

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


Testing
---

Tested on Ubuntu.


Thanks,

Diana Arroyo

Re: Review Request 43888: Updated webui to reflect the new TASK_KILLING state.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Feb. 23, 2016, 6:04 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43888/
> ---
> 
> (Updated Feb. 23, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos and Abhishek Dasgupta.
> 
> 
> Bugs: MESOS-4547
> https://issues.apache.org/jira/browse/MESOS-4547
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated webui to reflect the new TASK_KILLING state.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/home.html ad3f092a8c7ae2965f2718078c8407e19ae70f10 
>   src/webui/master/static/js/controllers.js 
> ff73a46c598ebb9b32566cef3d9151ddf81891c8 
>   src/webui/master/static/slave.html 4735d66c03418ed964a4b720632614e9f9668df7 
> 
> Diff: https://reviews.apache.org/r/43888/diff/
> 
> 
> Testing
> ---
> 
> Manually ran the webui and looked for the new fields and looked for console 
> errors.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 43864: Fix typo of roles doc.

[Neil Conway]

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



The right place to make this fix is in `src/master/http.cpp`, and then rerun 
`support/generate-endpoint-help.py`. Seems like `Master::Http::ROLES_HELP` is 
missing commas in `DESCRIPTION`.

- Neil Conway


On Feb. 23, 2016, 3:20 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43864/
> ---
> 
> (Updated Feb. 23, 2016, 3:20 a.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix typo of roles doc.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/roles.json.md 
> d67779c246cceae2209f2611f32ada4493ae6f83 
>   docs/endpoints/master/roles.md 976a9b7891a17652289126ec7e7ee73cea0c2e35 
> 
> Diff: https://reviews.apache.org/r/43864/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Review Request 43888: Updated webui to reflect the new TASK_KILLING state.

[Ben Mahler]

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

Review request for mesos and Abhishek Dasgupta.


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


Repository: mesos


Description
---

Updated webui to reflect the new TASK_KILLING state.


Diffs
-

  src/webui/master/static/home.html ad3f092a8c7ae2965f2718078c8407e19ae70f10 
  src/webui/master/static/js/controllers.js 
ff73a46c598ebb9b32566cef3d9151ddf81891c8 
  src/webui/master/static/slave.html 4735d66c03418ed964a4b720632614e9f9668df7 

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


Testing
---

Manually ran the webui and looked for the new fields and looked for console 
errors.


Thanks,

Ben Mahler

Re: Review Request 43821: Updated the HA framweork guide for TASK_KILLING.

[Ben Mahler]

> On Feb. 22, 2016, 5:39 a.m., Neil Conway wrote:
> > docs/high-availability-framework-guide.md, line 204
> > 
> >
> > "has not yet been killed" ?

done


> On Feb. 22, 2016, 5:39 a.m., Neil Conway wrote:
> > docs/high-availability-framework-guide.md, line 206
> > 
> >
> > "`FrameworkInfo.capabilities`" ?

hm.. I couldn't phrase something clean with this, did the following:

terminate gracefully. Executors must not generate this state unless the
framework has the `TASK_KILLING_STATE` framework capability.


- Ben


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


On Feb. 22, 2016, 5:19 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43821/
> ---
> 
> (Updated Feb. 22, 2016, 5:19 a.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta and Neil Conway.
> 
> 
> Bugs: MESOS-4547
> https://issues.apache.org/jira/browse/MESOS-4547
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 0d9c483985d61b512339f50f395f9360de034e2d 
> 
> Diff: https://reviews.apache.org/r/43821/diff/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 43849: Fixed typos in subprocess.cpp comments.

[Bernd Mathiske]

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


Ship it!




Ship It!

- Bernd Mathiske


On Feb. 23, 2016, 9:49 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43849/
> ---
> 
> (Updated Feb. 23, 2016, 9:49 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typos in subprocess.cpp comments.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 44ca6d0869f3dbcfda1ac01d0d6b79dc20c4267c 
> 
> Diff: https://reviews.apache.org/r/43849/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43821: Updated the HA framweork guide for TASK_KILLING.

[Ben Mahler]

> On Feb. 22, 2016, 5:44 a.m., Abhishek Dasgupta wrote:
> > docs/high-availability-framework-guide.md, line 205
> > 
> >
> > How about we put an example here? Like the following:
> > This is useful for tasks that require some time to terminate 
> > gracefully(e.g. stopping a docker container).
> > As to stop a docker container, it always takes a little bit of time, so 
> > this might make a good example.

Does docker do something that inherently requires some time to terminate 
gracefully? How is it different from just a SIGTERM -> SIGKILL escalation?


- Ben


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


On Feb. 22, 2016, 5:19 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43821/
> ---
> 
> (Updated Feb. 22, 2016, 5:19 a.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta and Neil Conway.
> 
> 
> Bugs: MESOS-4547
> https://issues.apache.org/jira/browse/MESOS-4547
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 0d9c483985d61b512339f50f395f9360de034e2d 
> 
> Diff: https://reviews.apache.org/r/43821/diff/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43032]

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 Feb. 23, 2016, 8:44 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 23, 2016, 8:44 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43827: Introducing TASK_KILLING state on WebUI.

[Ben Mahler]

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



Did you test this? ;)

We need to also adjust slave.html and the javascript code, let's discard this 
since I already have a working patch.

- Ben Mahler


On Feb. 22, 2016, 10:18 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43827/
> ---
> 
> (Updated Feb. 22, 2016, 10:18 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Neil Conway.
> 
> 
> Bugs: MESOS-4547
> https://issues.apache.org/jira/browse/MESOS-4547
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introducing TASK_KILLING state on WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/home.html ad3f092a8c7ae2965f2718078c8407e19ae70f10 
> 
> Diff: https://reviews.apache.org/r/43827/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Bernd Mathiske]

> On Feb. 23, 2016, 9:33 a.m., Ben Mahler wrote:
> > Apologies, did you check with Alex prior to committing? We talked about 
> > this change recently but we didn't publish the comments, sorry that it 
> > wasn't clear! I reverted it for now, since we didn't really like the empty 
> > map constants as class members. We also needed to look into whether using 
> > empty initializer lists inline worked instead of this change.

Alex is on vacation and I found this lingering. Since it looks a lot like an 
improvement, I went ahead after reading it all and testing it. No problem, we 
can progress further from either version.


- Bernd


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


On Jan. 28, 2016, 5:12 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41950/
> ---
> 
> (Updated Jan. 28, 2016, 5:12 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, Joerg Schad, and Joris 
> Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes made:
> - empty resource map promoted to a const class field;
> - removed variable numeric suffixes where appropriate;
> - added const where appropriate.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> f18e6eb10572b0f5b8bbff338384d9406f6ad62b 
> 
> Diff: https://reviews.apache.org/r/41950/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> 
> `make check` 
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Ben Mahler]

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



Apologies, did you check with Alex prior to committing? We talked about this 
change recently but we didn't publish the comments, sorry that it wasn't clear! 
I reverted it for now, since we didn't really like the empty map constants as 
class members. We also needed to look into whether using empty initializer 
lists inline worked instead of this change.

- Ben Mahler


On Jan. 28, 2016, 1:12 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41950/
> ---
> 
> (Updated Jan. 28, 2016, 1:12 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, Joerg Schad, and Joris 
> Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes made:
> - empty resource map promoted to a const class field;
> - removed variable numeric suffixes where appropriate;
> - added const where appropriate.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> f18e6eb10572b0f5b8bbff338384d9406f6ad62b 
> 
> Diff: https://reviews.apache.org/r/41950/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> 
> `make check` 
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 43848: Used `size_t` to track number of frameworks per role.

[Ben Mahler]

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


Ship it!




Thanks! Looks like there are a few other spots in the allocation code:

https://github.com/apache/mesos/blob/0.27.1/src/master/allocator/mesos/hierarchical.cpp#L151
 (notice the non-negative CHECK)
https://github.com/apache/mesos/blob/0.27.1/src/master/allocator/sorter/sorter.hpp#L119-L121

- Ben Mahler


On Feb. 22, 2016, 7:54 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43848/
> ---
> 
> (Updated Feb. 22, 2016, 7:54 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Per informal project style, we prefer using unsigned types to
> represent non-negative quantities.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 0d39d3f3b5f4ff7f62f9de7200d062845c71818a 
> 
> Diff: https://reviews.apache.org/r/43848/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> Compilation on Linux with GCC 5.3
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 43881: Added allocator metric for the number of allocations to a framework.

[Benjamin Bannier]

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

Review request for mesos, Alexander Rukletsov and Ben Mahler.


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


Repository: mesos


Description
---

Added allocator metric for the number of allocations to a framework.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
0d39d3f3b5f4ff7f62f9de7200d062845c71818a 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
1af5c9870430eb05ca0b19ece0ca2957a05093ff 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Review Request 43879: Added allocator metrics for number of allocations made.

[Benjamin Bannier]

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

Review request for mesos, Alexander Rukletsov and Ben Mahler.


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


Repository: mesos


Description
---

Added allocator metrics for number of allocations made.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
0d39d3f3b5f4ff7f62f9de7200d062845c71818a 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
1af5c9870430eb05ca0b19ece0ca2957a05093ff 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Review Request 43883: Added allocator metrics for number of filters per framework.

[Benjamin Bannier]

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

Review request for mesos, Alexander Rukletsov and Ben Mahler.


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


Repository: mesos


Description
---

Added allocator metrics for number of filters per framework.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
0d39d3f3b5f4ff7f62f9de7200d062845c71818a 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
1af5c9870430eb05ca0b19ece0ca2957a05093ff 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Review Request 43880: Added allocator metrics for total and allocated scalar resources.

[Benjamin Bannier]

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

Review request for mesos, Alexander Rukletsov and Ben Mahler.


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


Repository: mesos


Description
---

Added allocator metrics for total and allocated scalar resources.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
0d39d3f3b5f4ff7f62f9de7200d062845c71818a 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
1af5c9870430eb05ca0b19ece0ca2957a05093ff 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Review Request 43882: Added allocation metrics for allocation time.

[Benjamin Bannier]

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

Review request for mesos, Alexander Rukletsov and Ben Mahler.


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


Repository: mesos


Description
---

Added allocation metrics for allocation time.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
0d39d3f3b5f4ff7f62f9de7200d062845c71818a 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
1af5c9870430eb05ca0b19ece0ca2957a05093ff 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Review Request 43884: Added allocator metrics for satisfied quotas.

[Benjamin Bannier]

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

Review request for mesos, Alexander Rukletsov and Ben Mahler.


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


Repository: mesos


Description
---

Added allocator metrics for satisfied quotas.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
0d39d3f3b5f4ff7f62f9de7200d062845c71818a 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
1af5c9870430eb05ca0b19ece0ca2957a05093ff 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Re: Review Request 43848: Used `size_t` to track number of frameworks per role.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43848]

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 Feb. 22, 2016, 7:54 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43848/
> ---
> 
> (Updated Feb. 22, 2016, 7:54 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Per informal project style, we prefer using unsigned types to
> represent non-negative quantities.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 0d39d3f3b5f4ff7f62f9de7200d062845c71818a 
> 
> Diff: https://reviews.apache.org/r/43848/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> Compilation on Linux with GCC 5.3
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43855: Added Appc fetcher support to store.

[Jojy Varghese]

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

(Updated Feb. 23, 2016, 3:30 p.m.)


Review request for mesos and Jie Yu.


Changes
---

addressed review.


Repository: mesos


Description
---

This change allows store to fetch an image using Appc image fetcher when an
image is not found in the cache. It also recursively fetches the dependencies
for the image.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
4b3829175f57fb9aea2478040d96f2f127cbc551 
  src/tests/fetcher_cache_tests.cpp e10b3f7ebc21c8c1095564fc40f123087dcf320e 

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


Testing
---

make check; local image server.


Thanks,

Jojy Varghese

Re: Review Request 43859: Fixed invalid usage of ATOMIC_FLAG_INIT in libprocess.

[Till Toenshoff]

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


Ship it!




Ship It!

- Till Toenshoff


On Feb. 23, 2016, 10:58 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43859/
> ---
> 
> (Updated Feb. 23, 2016, 10:58 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-4602
> https://issues.apache.org/jira/browse/MESOS-4602
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see above.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 5a987ea04ce8f1ebab8e1cc1d796e5243532271d 
> 
> Diff: https://reviews.apache.org/r/43859/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 43859: Fixed invalid usage of ATOMIC_FLAG_INIT in libprocess.

[Benjamin Bannier]

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


Ship it!




- Benjamin Bannier


On Feb. 23, 2016, 11:58 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43859/
> ---
> 
> (Updated Feb. 23, 2016, 11:58 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-4602
> https://issues.apache.org/jira/browse/MESOS-4602
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see above.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 5a987ea04ce8f1ebab8e1cc1d796e5243532271d 
> 
> Diff: https://reviews.apache.org/r/43859/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 43860: Used uri::Fetcher to pull docker images in docker registry puller.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [43860, 43801]

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

Error:
2016-02-23 14:52:20 URL:https://reviews.apache.org/r/43860/diff/raw/ 
[52668/52668] -> "43860.patch" [1]
error: patch failed: 
src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp:38
error: src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp: 
patch does not apply

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

- Mesos ReviewBot


On Feb. 23, 2016, 12:54 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43860/
> ---
> 
> (Updated Feb. 23, 2016, 12:54 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-4261 and MESOS-4499
> https://issues.apache.org/jira/browse/MESOS-4261
> https://issues.apache.org/jira/browse/MESOS-4499
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the second step of cleaning/simplying the docker registry puller code.
> 
> The new code uses the uri::Fetcher to download docker manifest and blobs 
> (instead of writting our own http logic which requires enabling of SSL for 
> Mesos).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 5b2d72c22fcbcc379b4901607cf3eb682de66206 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> a239b97557ad20353c67050dbc89ef16da898330 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> bccbac3357cf942446604e6cf5d16c3d594b 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 3fcf1471a035e35a2cac22442655ad65a84a9793 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 2f1d3e002140f34c646aab445a419c9c3d712f99 
>   src/slave/flags.hpp 54c1a69d8777f417cdd8f73ce638447d9951ab61 
>   src/slave/flags.cpp 855812e9f7cb4b96d4297f4bd5ac5de7f1d3c39a 
>   src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 4db6793a21abbb7ea4de0d0fca0431237d38d013 
> 
> Diff: https://reviews.apache.org/r/43860/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 43868: Add a getter for master::Flags.

[Jay Guo]

> On Feb. 23, 2016, 12:45 p.m., Klaus Ma wrote:
> > src/master/master.hpp, line 545
> > 
> >
> > `getFlags() const`

OK


> On Feb. 23, 2016, 12:45 p.m., Klaus Ma wrote:
> > src/tests/master_tests.cpp, line 975
> > 
> >
> > add blank line.

Sure, didn't notice double-line style


> On Feb. 23, 2016, 12:45 p.m., Klaus Ma wrote:
> > src/tests/master_tests.cpp, line 983
> > 
> >
> > Is this necessary? If so, please clear it up.

It is necessary to properly start a master. It is cleared during master 
teardown.


On Feb. 23, 2016, 12:45 p.m., Jay Guo wrote:
> > Can you also show which part of code will use this function?

Thanks for your comment. As stated in original task description, this getter is 
intended to be used by tests. For instance, in test 
MasterMaintenanceTest.InverseOffersFilters, `StartMaster()` could be used 
instead of `StartMaster(flags)`.


- Jay


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


On Feb. 23, 2016, 5:23 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43868/
> ---
> 
> (Updated Feb. 23, 2016, 5:23 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Joris Van Remoortere, and Qian Zhang.
> 
> 
> Bugs: MESOS-3481
> https://issues.apache.org/jira/browse/MESOS-3481
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3481 Add const accessor to Master flags.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
> 
> Diff: https://reviews.apache.org/r/43868/diff/
> 
> 
> Testing
> ---
> 
> Added one new unit test MasterTest::Flags
> 
> `make check` passed
> 
> 
> Thanks,
> 
> Jay Guo
> 
>