Re: Review Request 66385: Link execinfo in Glog build on FreeBSD.

2018-04-02 Thread David Forsythe 

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

(Updated April 3, 2018, 4:58 a.m.)


Review request for mesos and Andrew Schwartzmeyer.


Bugs: MESOS-4176 and MESOS-6849
https://issues.apache.org/jira/browse/MESOS-4176
https://issues.apache.org/jira/browse/MESOS-6849


Repository: mesos


Description
---

Link execinfo in Glog build on FreeBSD.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
  cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 


Diff: https://reviews.apache.org/r/66385/diff/2/

Changes: https://reviews.apache.org/r/66385/diff/1-2/


Testing
---


Thanks,

David Forsythe

Re: Review Request 66385: Link execinfo in Glog build on FreeBSD.

2018-04-02 Thread David Forsythe 


> On April 2, 2018, 5:05 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/CMakeLists.txt
> > Lines 359-361 (patched)
> > 
> >
> > This looks confusing (else then if endif), but I think it's because the 
> > indentation is off. This is supposed to be under the above `else ()`, right?

Indeed, will fix.


> On April 2, 2018, 5:05 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/CMakeLists.txt
> > Lines 360 (patched)
> > 
> >
> > Is this something that should actually go upstream to Glog? If so, 
> > let's make sure it does.

I'll add it to my todo list.


> On April 2, 2018, 5:05 p.m., Andrew Schwartzmeyer wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 304-306 (patched)
> > 
> >
> > FWIW we want to remove these eventually, as they create a dependency on 
> > the top-level project (Mesos) within the "3rdparty" projects (like stout).
> > 
> > Since we already have `LINUX` to fix, it's probably fine, as we're not 
> > really breaking it any further.

I'll try to avoid using it anywhere else.


- David


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


On April 3, 2018, 4:58 a.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66385/
> ---
> 
> (Updated April 3, 2018, 4:58 a.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176 and MESOS-6849
> https://issues.apache.org/jira/browse/MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-6849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Link execinfo in Glog build on FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
>   cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
> 
> 
> Diff: https://reviews.apache.org/r/66385/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66384: Link libm in ZooKeeper build on FreeBSD.

2018-04-02 Thread David Forsythe 


> On April 2, 2018, 5:06 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/zookeeper-3.4.8.patch
> > Line 154 (original), 154 (patched)
> > 
> >
> > Also, if this change should go upstream to ZooKeeper (looks to me like 
> > it should) then let's make sure it does. Their process is [pretty 
> > similar](https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute).
> > 
> > If you don't want to, I can upstream it.

I can add it to my todo list.


- David


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


On April 1, 2018, 2:32 a.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66384/
> ---
> 
> (Updated April 1, 2018, 2:32 a.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Link libm in ZooKeeper build on FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/zookeeper-3.4.8.patch 2eaa056dd5668d5842b5b59a42f83ae307d4aef0 
> 
> 
> Diff: https://reviews.apache.org/r/66384/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66384: Link libm in ZooKeeper build on FreeBSD.

2018-04-02 Thread David Forsythe 


> On April 2, 2018, 5:02 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/zookeeper-3.4.8.patch
> > Lines 870-873 (original), 870-872 (patched)
> > 
> >
> > This is funny, what did this get diffed against? I might have screwed 
> > up in my original diff...

I used the archive in the repo. I don't know what the workflow for changes like 
this is supposed to look like, but this is as close as I could get.


- David


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


On April 1, 2018, 2:32 a.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66384/
> ---
> 
> (Updated April 1, 2018, 2:32 a.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Link libm in ZooKeeper build on FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/zookeeper-3.4.8.patch 2eaa056dd5668d5842b5b59a42f83ae307d4aef0 
> 
> 
> Diff: https://reviews.apache.org/r/66384/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66144: Enforced task launch order on the agent.

2018-04-02 Thread Meng Zhu 

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

(Updated April 2, 2018, 5:36 p.m.)


Review request for mesos, Chun-Hung Hsiao and Greg Mann.


Bugs: MESOS-8617 and MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8617
https://issues.apache.org/jira/browse/MESOS-8624


Repository: mesos


Description
---

Up until now, Mesos does not guarantee in-order
task launch on the agent. There are two asynchronous
steps (unschedule GC and task authorization) in the
agent task launch path. Depending on the CPU scheduling
order, a later task launch may finish these two steps earlier
than its predecessors and get to the launch executor stage
earlier, resulting in out-of-order task delivery.

This patch enforces the task delivery order by sequencing
task launch after the two asynchronous steps, specifically
right before entering `__run()`.


Diffs (updated)
-

  src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
  src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a 


Diff: https://reviews.apache.org/r/66144/diff/8/

Changes: https://reviews.apache.org/r/66144/diff/7-8/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66322: Fixed a potential race in `Sequence`.

2018-04-02 Thread Meng Zhu 

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

(Updated April 2, 2018, 5:35 p.m.)


Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.


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


Repository: mesos


Description
---

Adding item to sequence is realized by dispatching
`add()` to the sequence actor. However, this could
race with the sequence actor termination.

This patch fixes this by enqueueing the terminate
message at the end of the message queue.

Also removed the clock settle in the test `DiscardAll`.
As the processing of the messages are now guaranteed
to happen before the actor termination.

Also added comments to clarify the onDiscard propagation.


Diffs (updated)
-

  3rdparty/libprocess/include/process/sequence.hpp 
b4d7593221bf225a9e53e3b7f94e45624400078a 
  3rdparty/libprocess/src/tests/sequence_tests.cpp 
43911b6c7a4220a4b8ea1baca191035355817e7b 


Diff: https://reviews.apache.org/r/66322/diff/2/

Changes: https://reviews.apache.org/r/66322/diff/1-2/


Testing
---

make check

After modifying test `DiscardAll`, it fails with the old implementation. After 
modifying the `inject` flag, it passes.


Thanks,

Meng Zhu

Re: Review Request 66143: Refactored agent task launch for better composition [2/2].

2018-04-02 Thread Meng Zhu 

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

(Updated April 2, 2018, 5:35 p.m.)


Review request for mesos, Chun-Hung Hsiao and Greg Mann.


Bugs: MESOS-8617 and MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8617
https://issues.apache.org/jira/browse/MESOS-8624


Repository: mesos


Description
---

This helps to encapsulate a task launch into a single
future which will come in handy when enforcing the task
launch order.

Affected tests are also updated.


Diffs (updated)
-

  src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
  src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a 
  src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd 
  src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63 
  src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 


Diff: https://reviews.apache.org/r/66143/diff/7/

Changes: https://reviews.apache.org/r/66143/diff/6-7/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66126: Refactored agent task launch for better composition [1/2].

2018-04-02 Thread Meng Zhu 


> On April 2, 2018, 3:38 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Line 2166 (original), 2221-2227 (patched)
> > 
> >
> > Ah whoops, we should defer this continuation to `self()`. I think it's 
> > probably safe at the moment, but best to always defer/dispatch except for 
> > trivial continuations which do not access actor state. Even if the current 
> > implementation is safe, it makes the code less fragile if we defer.

See my reply in https://reviews.apache.org/r/66144/


- Meng


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


On April 2, 2018, 5:33 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66126/
> ---
> 
> (Updated April 2, 2018, 5:33 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helps to encapsulate a task launch into a single
> future which will come in handy when enforcing the task
> launch order.
> 
> This patch also consolidated the error handling code
> in the task launch path.
> 
> Affected tests are also updated.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
>   src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a 
>   src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd 
>   src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66126/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66126: Refactored agent task launch for better composition [1/2].

2018-04-02 Thread Meng Zhu 

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

(Updated April 2, 2018, 5:33 p.m.)


Review request for mesos, Chun-Hung Hsiao and Greg Mann.


Bugs: MESOS-8617 and MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8617
https://issues.apache.org/jira/browse/MESOS-8624


Repository: mesos


Description
---

This helps to encapsulate a task launch into a single
future which will come in handy when enforcing the task
launch order.

This patch also consolidated the error handling code
in the task launch path.

Affected tests are also updated.


Diffs (updated)
-

  src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
  src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a 
  src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd 
  src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63 
  src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 


Diff: https://reviews.apache.org/r/66126/diff/5/

Changes: https://reviews.apache.org/r/66126/diff/4-5/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66144: Enforced task launch order on the agent.

2018-04-02 Thread Greg Mann 

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


Fix it, then Ship it!





src/slave/slave.cpp
Line 2233 (original), 2274 (patched)


We should defer this callback.



src/slave/slave.cpp
Lines 2275 (patched)


Ditto regarding naming of `framework_` here.


- Greg Mann


On April 2, 2018, 6:04 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66144/
> ---
> 
> (Updated April 2, 2018, 6:04 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Up until now, Mesos does not guarantee in-order
> task launch on the agent. There are two asynchronous
> steps (unschedule GC and task authorization) in the
> agent task launch path. Depending on the CPU scheduling
> order, a later task launch may finish these two steps earlier
> than its predecessors and get to the launch executor stage
> earlier, resulting in out-of-order task delivery.
> 
> This patch enforces the task delivery order by sequencing
> task launch after the two asynchronous steps, specifically
> right before entering `__run()`.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
>   src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a 
> 
> 
> Diff: https://reviews.apache.org/r/66144/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66322: Fixed a potential race in `Sequence`.

2018-04-02 Thread Greg Mann 

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


Fix it, then Ship it!





3rdparty/libprocess/include/process/sequence.hpp
Lines 117-122 (patched)


How about:

"NOTE: When we discard the notifier future, any `onDiscard()` callbacks 
registered on `promise->future` will be invoked, but `onDiscard` callbacks 
registered on the future returned by `add()` will NOT be invoked. This is 
because currently discards do not propagate through `dispatch()`. In other 
words, users should be careful when registering `onDiscard` callbacks on the 
returned future."



3rdparty/libprocess/include/process/sequence.hpp
Lines 189 (patched)


s/happened/which happened/


- Greg Mann


On March 28, 2018, 1:15 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66322/
> ---
> 
> (Updated March 28, 2018, 1:15 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8741
> https://issues.apache.org/jira/browse/MESOS-8741
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding item to sequence is realized by dispatching
> `add()` to the sequence actor. However, this could
> race with the sequence actor termination.
> 
> This patch fixes this by enqueueing the terminate
> message at the end of the message queue.
> 
> Also removed the clock settle in the test `DiscardAll`.
> As the processing of the messages are now guaranteed
> to happen before the actor termination.
> 
> Also added comments to clarify the onDiscard propagation.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/sequence.hpp 
> b4d7593221bf225a9e53e3b7f94e45624400078a 
>   3rdparty/libprocess/src/tests/sequence_tests.cpp 
> 43911b6c7a4220a4b8ea1baca191035355817e7b 
> 
> 
> Diff: https://reviews.apache.org/r/66322/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> After modifying test `DiscardAll`, it fails with the old implementation. 
> After modifying the `inject` flag, it passes.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66143: Refactored agent task launch for better composition [2/2].

2018-04-02 Thread Greg Mann 

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




src/slave/slave.cpp
Line 2332 (original), 2345 (patched)


As in the previous review, ditto here regarding the naming of `framework_`. 
Let's switch to `_framework` for local consistency.



src/slave/slave.cpp
Lines 2354-2356 (patched)


How about

"Authorization failed for  of framework : "



src/slave/slave.cpp
Lines 2358 (patched)


Ditto regarding the naming of `task_` here.



src/slave/slave.cpp
Lines 2384 (patched)


Ditto regarding naming of `framework_`, here as well as `task_` and 
`framework_` below.



src/slave/slave.cpp
Lines 2386-2389 (patched)


How about

"Cannot handle authorization failure for  because the framework 
 does not exist"



src/slave/slave.cpp
Lines 2396-2398 (patched)


Looks like we don't need this local variable?



src/slave/slave.cpp
Lines 2428-2430 (patched)


This can be a `const string`. Also, let's be consistent and use the same 
indentation here that you use for other error strings above.


- Greg Mann


On April 2, 2018, 5:59 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66143/
> ---
> 
> (Updated April 2, 2018, 5:59 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helps to encapsulate a task launch into a single
> future which will come in handy when enforcing the task
> launch order.
> 
> Affected tests are also updated.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
>   src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a 
>   src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd 
>   src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66143/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66392: Find sasl2 on non-Windows platforms before trying to link it.

2018-04-02 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [66384, 66385, 66387, 66392]

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

- Mesos Reviewbot


On April 2, 2018, 6:33 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66392/
> ---
> 
> (Updated April 2, 2018, 6:33 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Find sasl2 on non-Windows platforms before trying to link it.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
>   src/CMakeLists.txt 6fc45dba8ee70b591be03ac483655c1844a0a6b9 
> 
> 
> Diff: https://reviews.apache.org/r/66392/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66001: MESOS-6575: Add soft limit and kill to disk/xfs.

2018-04-02 Thread Ilya Pronin 


> On March 27, 2018, 9:35 a.m., James Peach wrote:
> > src/slave/containerizer/mesos/isolators/xfs/disk.cpp
> > Lines 420 (patched)
> > 
> >
> > It's unfortunately verbose, but we should do this:
> > 
> > ```
> >   CHECK_NE(
> >   static_cast(xfs::QuotaPolicy::ACCOUNTING),
> >   static_cast(quotaPolicy));
> > ```

Or we could define `operator<<(std::ostream&, xfs::QuotaPolicy)`.


- Ilya


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


On March 23, 2018, 9:13 a.m., Harold Dost wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66001/
> ---
> 
> (Updated March 23, 2018, 9:13 a.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-6575
> https://issues.apache.org/jira/browse/MESOS-6575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New Flags for disk/xfs isolator:
> - xfs_kill_containers - This will create a 10 MB buffer between the soft
>   and hard limits, and when the soft limit is exceeded it will
>   subsequently be killed.
> 
> Functionality
> - This by default disabled behavior allows for the `disk/xfs` isolator
>   to kill containers which surpass their limit.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
> 07e68a777aefba4dd35066f2eb207bba7f199d83 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
> 8d9f8f846866f9de377c59cb7fb311041283ba70 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
> e034133629a9c1cf58b776f8da2a93421332cee0 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> 2708524add1ff693b616d4fb241c4a0a3070520b 
>   src/slave/flags.hpp 949a4783caf8aac9a246a98525a5287b0f8256d8 
>   src/slave/flags.cpp dd8dfb7a8a9f7c6030939c9eea841eb47deadfc4 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> 64c3e1c3f0bc435897626cb0a13bc19c7cb1a4fe 
> 
> 
> Diff: https://reviews.apache.org/r/66001/diff/11/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Harold Dost
> 
>

Re: Review Request 66001: MESOS-6575: Add soft limit and kill to disk/xfs.

2018-04-02 Thread Ilya Pronin 

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




src/slave/containerizer/mesos/isolators/xfs/disk.cpp
Line 304 (original), 317-318 (patched)


Keep `infos.contains(containerId)` check here. Otherwise you can just 
remove this method, it will be inherited from `MesosIsolatorProcess`.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp
Line 309 (original), 328 (patched)


Style: add a space after `if` and before `{`.

Please read the [code style 
guide](http://mesos.apache.org/documentation/latest/c++-style-guide/) and look 
at the code around to see how to format it.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp
Lines 392 (patched)


Ditto re spaces around the bracketed condition.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp
Lines 393 (patched)


This should be configurable.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp
Lines 396 (patched)


Combine this with `status` declaration that is currently on line 386.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp
Lines 420 (patched)


What about `QuotaPolicy::ENFORCING_PASSIVE`? The monitoring loop should not 
be started if we don't intend to kill containers.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp
Lines 423-424 (patched)


Style: The continuation should be indented with 4 spaces.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp
Lines 426-427 (patched)


Style: The second line is overindented by 1 space.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp
Lines 441-443 (patched)


Continuation with function arguments should be indented with 4 spaces.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp
Lines 411-442 (original), 498-530 (patched)


What's the point of this drive-by change? The only change here is `info.` 
removal, right?



src/slave/containerizer/mesos/isolators/xfs/utils.hpp
Line 100 (original)


Keep this newline.



src/slave/containerizer/mesos/isolators/xfs/utils.hpp
Line 105 (original), 108-110 (patched)


Add 1 more newline between the prototypes.


- Ilya Pronin


On March 23, 2018, 9:13 a.m., Harold Dost wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66001/
> ---
> 
> (Updated March 23, 2018, 9:13 a.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-6575
> https://issues.apache.org/jira/browse/MESOS-6575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New Flags for disk/xfs isolator:
> - xfs_kill_containers - This will create a 10 MB buffer between the soft
>   and hard limits, and when the soft limit is exceeded it will
>   subsequently be killed.
> 
> Functionality
> - This by default disabled behavior allows for the `disk/xfs` isolator
>   to kill containers which surpass their limit.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
> 07e68a777aefba4dd35066f2eb207bba7f199d83 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
> 8d9f8f846866f9de377c59cb7fb311041283ba70 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
> e034133629a9c1cf58b776f8da2a93421332cee0 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> 2708524add1ff693b616d4fb241c4a0a3070520b 
>   src/slave/flags.hpp 949a4783caf8aac9a246a98525a5287b0f8256d8 
>   src/slave/flags.cpp dd8dfb7a8a9f7c6030939c9eea841eb47deadfc4 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> 64c3e1c3f0bc435897626cb0a13bc19c7cb1a4fe 
> 
> 
> Diff: https://reviews.apache.org/r/66001/diff/11/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Harold Dost
> 
>

Re: Review Request 66392: Find sasl2 on non-Windows platforms before trying to link it.

2018-04-02 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 66392 was successfully built and tested.

Reviews applied: `['66384', '66385', '66387', '66392']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66392

- Mesos Reviewbot Windows


On April 2, 2018, 11:33 a.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66392/
> ---
> 
> (Updated April 2, 2018, 11:33 a.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Find sasl2 on non-Windows platforms before trying to link it.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
>   src/CMakeLists.txt 6fc45dba8ee70b591be03ac483655c1844a0a6b9 
> 
> 
> Diff: https://reviews.apache.org/r/66392/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66314: Fix 3rdparty build commands for FreeBSD.

2018-04-02 Thread David Forsythe 


> On April 2, 2018, 6:42 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/CMakeLists.txt
> > Lines 359-360 (original), 365-366 (patched)
> > 
> >
> > If I understood the above discussion correctly, I think the following 
> > would work well:
> > 
> > Use `${CMAKE_MAKE_PROGRAM}` and `${CMAKE_MAKE_PROGRAM} install` for the 
> > BUILD/INSTALL commands, instead of adding `${GNU_MAKE}`.
> > 
> > Then in the MesosConfigure.cmake, we can have a platform check `if 
> > (FREEBSD) ... make sure CMAKE_MAKE_PROGRAM is gmake` or something to that 
> > effect.

If we do that, we would be forcing gmake for the entire build, right? It's not 
clear to me that is the best approach, since the only problem is leveldb (and 
since newer versions of leveldb seem to use cmake?). I might even be more 
incline to try patching LevelDBs Makefile rather than force that.


- David


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


On April 2, 2018, 6:36 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66314/
> ---
> 
> (Updated April 2, 2018, 6:36 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix 3rdparty build commands for FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
>   cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
> 
> 
> Diff: https://reviews.apache.org/r/66314/diff/1/
> 
> 
> Testing
> ---
> 
> cmake --build on FreeBSD
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66314: Fix 3rdparty build commands for FreeBSD.

2018-04-02 Thread David Forsythe 


> On March 28, 2018, 8:06 a.m., Benjamin Bannier wrote:
> > 3rdparty/CMakeLists.txt
> > Lines 58-63 (patched)
> > 
> >
> > Like discussed offline, I don't think there is a reason we need to bolt 
> > such logic on the used system, at least currently.
> > 
> > Let's instead use `find_program` to find a `make` and use it to set 
> > this variable.
> > 
> > Unless I miss something, we do not depend on GNU make here, so let's 
> > maybe reflect that in a more general name, e.g., just `MAKE` if it is 
> > available.
> 
> David Forsythe wrote:
> I took a look at find_program, but I don't think that will solve the 
> problem.
> 
> We actually *do* depend on gmake here (even on darwin). On FreeBSD GHU 
> make is a 3rd party application and is actually called and installed as 
> `gmake`, whereas the system make is actually Pmake (which is incompatible).
> 
> David Forsythe wrote:
> I should clarify that there is incompability in a 3rdparty Makefile, not 
> that the two makes are completely incompatible.
> 
> Particularly there are three issues - LevelDB, libev, and glog.
> 
> LevelDB is always going to fail without `gmake`, because it does not use 
> autotools. I don't know how we can get around that.
> 
> libev and glog do use autotools. They have some strange behavior when 
> building, though --
> If I use `cmake --build` they both fail on a bad target (because of an 
> incompatible Makefile).
> If I use (bsd) `make`, they build fine.
> If I use `gmake` they both fail (bad target).
> 
> For both the first and third scenario, I can run the commands that are 
> failing outside of the build and they succeed.
> 
> Accordng to the cache, cmake is using `gmake` as `CMAKE_MAKE_PROGRAM` in 
> these cases, so it's not clear to me why anything would fail when I do the 
> whole build with `gmake`. It seems like the build can go make -> gmake, but 
> not gmake -> make.
> 
> I think the baseline should be having `cmake --build` work, and the only 
> way I have gotten everything to work as expected is by forcing `gmake` 
> everywhere. I'm learning Cmake as I go so I could be missing something 
> obvious to fix this.

I've been thinking about this a bit more, and I think that the best approach 
might actually be to just leave the libev and glog calls as make and leave the 
conditional gmake call. On FreeBSD we could just tell users to use `make` 
instead of cmake build. It seems like a way less intrusive change.

Thoughts?


- David


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


On April 2, 2018, 6:36 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66314/
> ---
> 
> (Updated April 2, 2018, 6:36 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix 3rdparty build commands for FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
>   cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
> 
> 
> Diff: https://reviews.apache.org/r/66314/diff/1/
> 
> 
> Testing
> ---
> 
> cmake --build on FreeBSD
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66392: Find sasl2 on non-Windows platforms before trying to link it.

2018-04-02 Thread Andrew Schwartzmeyer 

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




3rdparty/CMakeLists.txt
Line 1051 (original), 1051 (patched)


Don't think you mean to have this here.



src/CMakeLists.txt
Lines 513-514 (original), 512-516 (patched)


Instead of this, we should have a `NOT WIN32` section in 
3rdparty/CMakeLists.txt that does `find_library(sasl2)` and then creates an 
imported `sasl2` target, that way `sasl2` will be a target on both Windows and 
non-Windows platforms, and this code can go unchanged.


- Andrew Schwartzmeyer


On April 2, 2018, 11:33 a.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66392/
> ---
> 
> (Updated April 2, 2018, 11:33 a.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Find sasl2 on non-Windows platforms before trying to link it.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
>   src/CMakeLists.txt 6fc45dba8ee70b591be03ac483655c1844a0a6b9 
> 
> 
> Diff: https://reviews.apache.org/r/66392/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66314: Fix 3rdparty build commands for FreeBSD.

2018-04-02 Thread Andrew Schwartzmeyer 

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




3rdparty/CMakeLists.txt
Lines 359-360 (original), 365-366 (patched)


If I understood the above discussion correctly, I think the following would 
work well:

Use `${CMAKE_MAKE_PROGRAM}` and `${CMAKE_MAKE_PROGRAM} install` for the 
BUILD/INSTALL commands, instead of adding `${GNU_MAKE}`.

Then in the MesosConfigure.cmake, we can have a platform check `if 
(FREEBSD) ... make sure CMAKE_MAKE_PROGRAM is gmake` or something to that 
effect.


- Andrew Schwartzmeyer


On April 2, 2018, 11:36 a.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66314/
> ---
> 
> (Updated April 2, 2018, 11:36 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix 3rdparty build commands for FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
>   cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
> 
> 
> Diff: https://reviews.apache.org/r/66314/diff/1/
> 
> 
> Testing
> ---
> 
> cmake --build on FreeBSD
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Review Request 66392: Find sasl2 on non-Windows platforms before trying to link it.

2018-04-02 Thread David Forsythe 

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

Review request for mesos.


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


Repository: mesos


Description
---

Find sasl2 on non-Windows platforms before trying to link it.


Diffs
-

  3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
  src/CMakeLists.txt 6fc45dba8ee70b591be03ac483655c1844a0a6b9 


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


Testing
---


Thanks,

David Forsythe

Re: Review Request 66367: Made the master drop `RECONCILE_OPERATIONS` calls from v0 schedulers.

2018-04-02 Thread Mesos Reviewbot Windows 

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['66362', '66363', '66364', '66365', '66366', '66367']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66367

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66367/logs/mesos-tests-stdout.log):

```
[   OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (117 ms)
[--] 9 tests from Endpoint/SlaveEndpointTest (1113 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (45 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (50 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (98 ms 
total)

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (761 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (786 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (833 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (868 ms total)

[--] Global test environment tear-down
[==] 949 tests from 94 test cases ran. (448854 ms total)
[  PASSED  ] 948 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout

 1 FAILED TEST
  YOU HAVE 214 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66367/logs/mesos-tests-stderr.log):

```
I0402 18:15:00.948766  5704 master.cpp:10449] Updating the state of task 
15e3090d-5701-425f-88ec-a189e9051f0a of framework 
3b213ea3-4a73-4657-b491-98f15c651fd1- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0402 18:15:00.948766  3684 slave.cpp:3877I0402 18:15:00.754611 10520 
exec.cpp:162] Version: 1.6.0
I0402 18:15:00.785619  5908 exec.cpp:236] Executor registered on agent 
3b213ea3-4a73-4657-b491-98f15c651fd1-S0
I0402 18:15:00.792613 11288 executor.cpp:176] Received SUBSCRIBED event
I0402 18:15:00.796603 11288 executor.cpp:180] Subscribed executor on 
winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0402 18:15:00.797605 11288 executor.cpp:176] Received LAUNCH event
I0402 18:15:00.802603 11288 executor.cpp:648] Starting task 
15e3090d-5701-425f-88ec-a189e9051f0a
I0402 18:15:00.889614 11288 executor.cpp:483] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0402 18:15:00.917609 11288 executor.cpp:661] Forked command at 7872
I0402 18:15:00.951609  8948 exec.cpp:445] Executor asked to shutdown
I0402 18:15:00.952610  8340 executor.cpp:176] Received SHUTDOWN event
I0402 18:15:00.952610  8340 executor.cpp:758] Shutting down
I0402 18:15:00.952610  8340 executor.cpp:868] Sending SIGTERM to process tree 
at pid 7] Shutting down framework 3b213ea3-4a73-4657-b491-98f15c651fd1-
I0402 18:15:00.949607  3684 slave.cpp:6574] Shutting down executor 
'15e3090d-5701-425f-88ec-a189e9051f0a' of framework 
3b213ea3-4a73-4657-b491-98f15c651fd1- at executor(1)@10.3.1.8:53036
I0402 18:15:00.950610  3684 slave.cpp:923] Agent terminating
W0402 18:15:00.950610  3684 slave.cpp:3873] Ignoring shutdown framework 
3b213ea3-4a73-4657-b491-98f15c651fd1- because it is terminating
I0402 18:15:00.950610  5704 master.cpp:10548] Removing task 
15e3090d-5701-425f-88ec-a189e9051f0a with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 3b213ea3-4a73-4657-b491-98f15c651fd1- on 
agent 3b213ea3-4a73-4657-b491-98f15c651fd1-S0 at slave(418)@10.3.1.8:53015 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0402 18:15:00.953610  5704 master.cpp:1295] Agent 
3b213ea3-4a73-4657-b491-98f15c651fd1-S0 at slave(418)@10.3.1.8:53015 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0402 18:15:00.953610  5704 master.cpp:3286] Disconnecting agent 
3b213ea3-4a73-4657-b491-98f15c651fd1-S0 at slave(418)@10.3.1.8:53015 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0402 18:15:00.954628  5704 master.cpp:3305] Deactivating agent 
3b213ea3-4a73-4657-b491-98f15c651fd1-S0 at slave(418)@10.3.1.8:53015 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0402

Re: Review Request 66144: Enforced task launch order on the agent.

2018-04-02 Thread Meng Zhu 

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

(Updated April 2, 2018, 11:04 a.m.)


Review request for mesos, Chun-Hung Hsiao and Greg Mann.


Changes
---

Thanks for the review! Patch updated.


Bugs: MESOS-8617 and MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8617
https://issues.apache.org/jira/browse/MESOS-8624


Repository: mesos


Description
---

Up until now, Mesos does not guarantee in-order
task launch on the agent. There are two asynchronous
steps (unschedule GC and task authorization) in the
agent task launch path. Depending on the CPU scheduling
order, a later task launch may finish these two steps earlier
than its predecessors and get to the launch executor stage
earlier, resulting in out-of-order task delivery.

This patch enforces the task delivery order by sequencing
task launch after the two asynchronous steps, specifically
right before entering `__run()`.


Diffs (updated)
-

  src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
  src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a 


Diff: https://reviews.apache.org/r/66144/diff/7/

Changes: https://reviews.apache.org/r/66144/diff/6-7/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66143: Refactored agent task launch for better composition [2/2].

2018-04-02 Thread Meng Zhu 

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

(Updated April 2, 2018, 10:59 a.m.)


Review request for mesos, Chun-Hung Hsiao and Greg Mann.


Bugs: MESOS-8617 and MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8617
https://issues.apache.org/jira/browse/MESOS-8624


Repository: mesos


Description
---

This helps to encapsulate a task launch into a single
future which will come in handy when enforcing the task
launch order.

Affected tests are also updated.


Diffs (updated)
-

  src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
  src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a 
  src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd 
  src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63 
  src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 


Diff: https://reviews.apache.org/r/66143/diff/6/

Changes: https://reviews.apache.org/r/66143/diff/5-6/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66126: Refactored agent task launch for better composition [1/2].

2018-04-02 Thread Meng Zhu 

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

(Updated April 2, 2018, 10:57 a.m.)


Review request for mesos, Chun-Hung Hsiao and Greg Mann.


Bugs: MESOS-8617 and MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8617
https://issues.apache.org/jira/browse/MESOS-8624


Repository: mesos


Description
---

This helps to encapsulate a task launch into a single
future which will come in handy when enforcing the task
launch order.

This patch also consolidated the error handling code
in the task launch path.

Affected tests are also updated.


Diffs (updated)
-

  src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
  src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a 
  src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd 
  src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63 
  src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 


Diff: https://reviews.apache.org/r/66126/diff/4/

Changes: https://reviews.apache.org/r/66126/diff/3-4/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66365: Fixed generation of operation statuses in `Slave::applyOperation()`.

2018-04-02 Thread Gaston Kleiman 


> On March 30, 2018, 8:38 a.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Lines 4266-4268 (patched)
> > 
> >
> > Was this formatting produced by clang-format?
> > 
> > I would probably do:
> > ```
> >   Option operationId =
> > message.operation_info().has_id()
> >   ? message.operation_info().id()
> >   : Option::none();
> > ```
> > but if clang-format generated this, then let's just keep it.

Yeah, our `clang-format` generated it... but as discussed offline, I updated 
the patch to match the style of the ternary statement right above this one.


- Gaston


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


On April 2, 2018, 10:15 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66365/
> ---
> 
> (Updated April 2, 2018, 10:15 a.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates `Slave::applyOperation()` to make it include the
> operation ID in operation statuses for operations that have an ID.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a 
> 
> 
> Diff: https://reviews.apache.org/r/66365/diff/2/
> 
> 
> Testing
> ---
> 
> Tests still pass on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66365: Fixed generation of operation statuses in `Slave::applyOperation()`.

2018-04-02 Thread Gaston Kleiman 

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

(Updated April 2, 2018, 10:15 a.m.)


Review request for mesos and Greg Mann.


Changes
---

Addressed Greg's comment.


Repository: mesos


Description
---

This patch updates `Slave::applyOperation()` to make it include the
operation ID in operation statuses for operations that have an ID.


Diffs (updated)
-

  src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a 


Diff: https://reviews.apache.org/r/66365/diff/2/

Changes: https://reviews.apache.org/r/66365/diff/1-2/


Testing
---

Tests still pass on GNU/Linux.


Thanks,

Gaston Kleiman

Re: Review Request 66387: Link subversion in stout build on FreeBSD.

2018-04-02 Thread Andrew Schwartzmeyer 

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On March 31, 2018, 8:25 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66387/
> ---
> 
> (Updated March 31, 2018, 8:25 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Link subversion in stout build on FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 5d787d87f0a4b30ba882b24d08118856eb84206b 
> 
> 
> Diff: https://reviews.apache.org/r/66387/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66384: Link libm in ZooKeeper build on FreeBSD.

2018-04-02 Thread Andrew Schwartzmeyer 

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




3rdparty/zookeeper-3.4.8.patch
Line 154 (original), 154 (patched)


Also, if this change should go upstream to ZooKeeper (looks to me like it 
should) then let's make sure it does. Their process is [pretty 
similar](https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute).

If you don't want to, I can upstream it.


- Andrew Schwartzmeyer


On March 31, 2018, 7:32 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66384/
> ---
> 
> (Updated March 31, 2018, 7:32 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Link libm in ZooKeeper build on FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/zookeeper-3.4.8.patch 2eaa056dd5668d5842b5b59a42f83ae307d4aef0 
> 
> 
> Diff: https://reviews.apache.org/r/66384/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66385: Link execinfo in Glog build on FreeBSD.

2018-04-02 Thread Andrew Schwartzmeyer 

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




3rdparty/CMakeLists.txt
Lines 359-361 (patched)


This looks confusing (else then if endif), but I think it's because the 
indentation is off. This is supposed to be under the above `else ()`, right?



3rdparty/CMakeLists.txt
Lines 360 (patched)


Is this something that should actually go upstream to Glog? If so, let's 
make sure it does.



cmake/CompilationConfigure.cmake
Lines 304-306 (patched)


FWIW we want to remove these eventually, as they create a dependency on the 
top-level project (Mesos) within the "3rdparty" projects (like stout).

Since we already have `LINUX` to fix, it's probably fine, as we're not 
really breaking it any further.


- Andrew Schwartzmeyer


On March 31, 2018, 7:33 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66385/
> ---
> 
> (Updated March 31, 2018, 7:33 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176 and MESOS-6849
> https://issues.apache.org/jira/browse/MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-6849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Link execinfo in Glog build on FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
>   cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
> 
> 
> Diff: https://reviews.apache.org/r/66385/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66384: Link libm in ZooKeeper build on FreeBSD.

2018-04-02 Thread Andrew Schwartzmeyer 

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




3rdparty/zookeeper-3.4.8.patch
Line 154 (original), 154 (patched)


LGTM



3rdparty/zookeeper-3.4.8.patch
Lines 870-873 (original), 870-872 (patched)


This is funny, what did this get diffed against? I might have screwed up in 
my original diff...


- Andrew Schwartzmeyer


On March 31, 2018, 7:32 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66384/
> ---
> 
> (Updated March 31, 2018, 7:32 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Link libm in ZooKeeper build on FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/zookeeper-3.4.8.patch 2eaa056dd5668d5842b5b59a42f83ae307d4aef0 
> 
> 
> Diff: https://reviews.apache.org/r/66384/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>