Re: Review Request 61947: Implemented handling of resource provider offer operations.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [61947, 61946, 61810, 58021, 58047, 58048, 57999, 57998, 
57997, 57911]

Failed command: python support/apply-reviews.py -n -r 58048

Error:
2017-09-08 03:05:40 URL:https://reviews.apache.org/r/58048/diff/raw/ 
[11585/11585] -> "58048.patch" [1]
error: patch failed: src/tests/mesos.hpp:875
error: src/tests/mesos.hpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/19283/console

- Mesos Reviewbot


On Sept. 7, 2017, 12:27 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61947/
> ---
> 
> (Updated Sept. 7, 2017, 12:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7594
> https://issues.apache.org/jira/browse/MESOS-7594
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the resource provider manager is responsible to apply offer
> operations by sending events to the respective resource providers,
> the master takes care of accepting these operations. Hence, for local
> resource providers the master sends an `ApplyResourceOperationMessage`
> to the agent where the resource provider is running on. The agent
> then relays the operation contained in the message to the resource
> provider manager.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 14d94cfa0e96881f77dd6e0c3449fdc0613e36ce 
>   src/messages/messages.proto dc4e19c08d671743f08223dcdf1dbc336b3940bd 
>   src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
>   src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd 
>   src/tests/resource_provider_manager_tests.cpp 
> 3bc56b51526e9dd188423f7349e74246c3295c77 
> 
> 
> Diff: https://reviews.apache.org/r/61947/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 62147: Added a comment about master sending a checkpointed resources message.

[Benjamin Mahler]

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


Ship it!





src/master/master.cpp
Lines 6617-6618 (patched)


Newline to separate the comment paragraphs?

```
  // Send checkpointed resources to the agent. This is important for
  // the cases where the master didn't fail over. In that case, the
  // master might have already applied an operation that the agent
  // didn't see (e.g., due to a breaking connection). This message
  // will sync the state between the master and the agent about
  // checkpointed resources.
  //
  // TODO(jieyu): This message is not necessary for the case where the
  // master fails over. Consider moving this to `reconcileKnownSlave`.
```


- Benjamin Mahler


On Sept. 7, 2017, 4:11 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62147/
> ---
> 
> (Updated Sept. 7, 2017, 4:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Michael Park, 
> and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a comment about master sending a checkpointed resources message.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 14d94cfa0e96881f77dd6e0c3449fdc0613e36ce 
> 
> 
> Diff: https://reviews.apache.org/r/62147/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62047: Allowed look up latest executor directory by virtual path.

[Benjamin Mahler]

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



Before I can commit this we need some tests, there is a bug below too in that 
there was no leading slash.


src/slave/paths.cpp
Lines 310-317 (patched)


This does not produce a leading slash.



src/slave/slave.cpp
Line 5568 (original), 5568-5571 (patched)


Can you file a ticket to remove the real paths post 2.0 and link to it in 
your comments?



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


virtualLatestPath here and elsewhere



src/slave/slave.cpp
Line 5577 (original), 5584-5585 (patched)


Since we don't need to chain them, can you do:

```
garbageCollect(path)
  .then(defer(self(), [=]() {
detachFile(latestPath);
detachFile(virtualLatestPath);
  });
```



src/slave/slave.cpp
Lines 7287-7290 (original), 7295-7298 (patched)


I think we could use some more spelling out of what's going on there, to 
make it easier on the reader (I myself was puzzled at what was going on with 
the original two attach calls):

```
// We expose the executor's sandbox in the /files endpoints
// via the following paths:
// 
//  (1) /agent_workdir/frameworks/FID/executors/EID/runs/CID
//  (2) /agent_workdir/frameworks/FID/executors/EID/runs/latest
//  (3) /frameworks/FID/executors/EID/runs/latest
//
// Originally we just exposed the real path (1) and later
// exposed the 'latest' symlink (2) since it's not easy for
// users to know the run's container ID. We deprecated
// (1) and (2) by exposing a virtual path (3) since we do not
// want to expose the agent's work directory and it's not
// something users care about in this context.
//
// TODO(zhitao): Remove (1) and (2) per MESOS- once we
// pass 2.0. They remain now for backwards compatibility.
```

You can put my name in the TODO if you like.



src/slave/slave.cpp
Lines 7470-7472 (original), 7491-7493 (patched)


This needs updating as well to match the other comment, re: real vs virtual 
instead of absolute vs relative.


- Benjamin Mahler


On Sept. 7, 2017, 9:18 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62047/
> ---
> 
> (Updated Sept. 7, 2017, 9:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jason Lai.
> 
> 
> Bugs: MESOS-7899
> https://issues.apache.org/jira/browse/MESOS-7899
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added support to attach a virtual path
> `/frameworks/{framework-id}/executors/{executor-id}/latest` to `Files` class
> in agent, which can be used to navigate an executor's latest sandbox without
> needing to know `work_dir` and `slave_id`.
> 
> 
> Diffs
> -
> 
>   src/slave/paths.hpp 51b481fc0870f1e95448f85ee2fd485fceea1919 
>   src/slave/paths.cpp 1a6943f03493079ab291616d3e36e0fd8809cf33 
>   src/slave/slave.cpp df920ec07cd59c7ba6baccfc1c20ed3809f187d6 
> 
> 
> Diff: https://reviews.apache.org/r/62047/diff/2/
> 
> 
> Testing
> ---
> 
> Ran a master/agent pair, launch a task with `mesos-execute` and use 
> `files/browse` endpoint to browse the virtual path.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 62040: Also log attached virtual path in agent.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/slave/slave.cpp
Lines 939-943 (original), 947-953 (patched)


Can you open and close the quotes on the same line?

```
  if (result.isReady()) {
VLOG(1) << "Successfully attached '" << path << "'"
<< " to virtual path '" << virtualPath << "'";
  } else {
LOG(ERROR) << "Failed to attach '" << path << "'
   << " to virtual path '" << virtualPath << "': "
   << (result.isFailed() ? result.failure() : "discarded");
  }
```


- Benjamin Mahler


On Sept. 7, 2017, 9:16 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62040/
> ---
> 
> (Updated Sept. 7, 2017, 9:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jason Lai.
> 
> 
> Bugs: MESOS-7899
> https://issues.apache.org/jira/browse/MESOS-7899
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The same physical directory will be mounted to multiple virtual paths,
>   so log which virtual path in agent log.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 435c37e28987119c4ef43c9dad61ac052020acd8 
>   src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
>   src/slave/slave.cpp df920ec07cd59c7ba6baccfc1c20ed3809f187d6 
> 
> 
> Diff: https://reviews.apache.org/r/62040/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 62174: Replace name with virtualPath in files API.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On Sept. 7, 2017, 9:16 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62174/
> ---
> 
> (Updated Sept. 7, 2017, 9:16 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7899
> https://issues.apache.org/jira/browse/MESOS-7899
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The terminology for `virtualPath` reflects better on the nature of the 
> `files` API.
> 
> 
> Diffs
> -
> 
>   src/files/files.hpp 1e123fac21ab98c508ca96112e080bd7d48389cb 
>   src/files/files.cpp b03279ee0b23b3a33383c6d3e0c31faa97dab8eb 
> 
> 
> Diff: https://reviews.apache.org/r/62174/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 62003: Added `network/ports` isolator nested container tests.

[Mesos Reviewbot Windows]

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



ERROR: Failed to apply patch 60491. Please check 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62003/logs/apply-reviews.log
 for any relevant errors

Reviews applied: [60491, 60493, 60494, 60764, 60901, 60836, 61538, 60902, 
60492, 60495, 61536, 60496, 60592, 60591, 60766, 60903, 60765, 60593, 62003]

Logs available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62003/logs

- Mesos Reviewbot Windows


On Sept. 7, 2017, 2:57 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62003/
> ---
> 
> (Updated Sept. 7, 2017, 2:57 p.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `network/ports` isolator nested container tests using the v1
> TaskGroups API. This tests that rogue port usage by a nested task is
> detected both with and without agent recovery, and that a well-behaved
> task is preserved across agent recovery.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/ports_isolator_tests.cpp PRE-CREATION 
>   src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554 
> 
> 
> Diff: https://reviews.apache.org/r/62003/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60496: Added socket checking to the network ports isolator.

[James Peach]

> On Sept. 5, 2017, 7:12 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/ports.cpp
> > Lines 271-284 (patched)
> > 
> >
> > Mind to explain why running the loop inside the isolator process (since 
> > you supplied `pid` as the first parameter of `process::loop()`) and making 
> > `collectContainerListeners()` a static function?
> > 
> > Can we follow the similar way of what DRF allocator process does? I 
> > mean we can run the loop outside the isolator process and make 
> > `collectContainerListeners()` a member method of 
> > `NetworkPortsIsolatorProcess`, and then invoke 
> > `collectContainerListeners()` with a `dispatch()`. In this way, we do not 
> > need to pass those parameters (like `portsProcess->infos.keys()`) to 
> > `collectContainerListeners()` since as a member method it can access them 
> > naturally.
> 
> James Peach wrote:
> > Why running the loop inside the isolator process
> 
> We need to sample the updated set of container IDs (`info.keys()`), which 
> means that at some point we need to dispatch onto the isolator process (to 
> ensure serialized access). If we ran the `loop` on a separate process, we 
> would need a helper method returning a `Future` on the 
> isolator that we could `dispatch` to. This would be equivalent to the code we 
> have here. `collectContainerListeners()` is a static function because of your 
> previous feedback that you didn't want to see a separate class to manage the 
> additional process.
> 
> > Can we follow the similar way of what DRF allocator process does?
> 
> Collecting the ports is expensive (as discussed elsewhere we need to scan 
> a large number of `/proc/fd` entries). If we `dispatch` this onto the main 
> process, then we block the isolator process for a (potentially) long time. 
> The very first version of this patch series implemented this suggestion and I 
> re-wrote it because we agreed that we didn't want to block the isolator 
> process.
> 
> Qian Zhang wrote:
> Agree. And I think `initialize()` should be a better place to start this 
> loop since it will be invoked when the process is spawned: 
> (https://github.com/apache/mesos/blob/1.3.1/3rdparty/libprocess/include/process/process.hpp#L94:L97)
> 
> So I think we should move this logic there like this:
> ```
> void initialize()
> {
>   // Start a loop to run periodically reconcile listening ports
>   // against allocated resources.
>   process::loop(
>   self(),
>   [=]() {
> return process::after(flags.container_ports_watch_interval);
>   },
>   [=](const Nothing&) {
> return process::async(
> ,
> flags.cgroups_root,
> freezerHierarchy.get(),
> agentPorts,
> infos.keys())
>   .then(defer(self(), ::check, 
> lambda::_1))
>   .then([]() -> ControlFlow { return Continue(); });
>   });
> }
> ```

Yes, this is much better.


- James


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


On Sept. 8, 2017, 12:09 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60496/
> ---
> 
> (Updated Sept. 8, 2017, 12:09 a.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented ports resource restrictions in the network ports isolator.
> Periodically, scan for listening sockets and match them up to all
> the open sockets in the containers we are tracking in the network.
> Check any sockets we find against the ports resource and trigger a
> resource limitation if the port has not been allocated.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60496/diff/18/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60496: Added socket checking to the network ports isolator.

[James Peach]

> On Aug. 17, 2017, 2:45 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/ports.cpp
> > Lines 148-150 (patched)
> > 
> >
> > It seems we only care about `port`, so it might not be needed to 
> > construct this oject. What about just using 
> > `ntohs(socketInfo.sourcePort.get())` in the code below?
> 
> James Peach wrote:
> I think we should keep the full address. There's no performance impact 
> and it is helpful for code clarity and debugging.

Dropping this; let's talk further if you disagree :)


- James


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


On Sept. 8, 2017, 12:09 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60496/
> ---
> 
> (Updated Sept. 8, 2017, 12:09 a.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented ports resource restrictions in the network ports isolator.
> Periodically, scan for listening sockets and match them up to all
> the open sockets in the containers we are tracking in the network.
> Check any sockets we find against the ports resource and trigger a
> resource limitation if the port has not been allocated.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60496/diff/18/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60496: Added socket checking to the network ports isolator.

[James Peach]

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

(Updated Sept. 8, 2017, 12:09 a.m.)


Review request for mesos, Qian Zhang and Jiang Yan Xu.


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


Repository: mesos


Description
---

Implemented ports resource restrictions in the network ports isolator.
Periodically, scan for listening sockets and match them up to all
the open sockets in the containers we are tracking in the network.
Check any sockets we find against the ports resource and trigger a
resource limitation if the port has not been allocated.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/60496/diff/18/

Changes: https://reviews.apache.org/r/60496/diff/17-18/


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 62176: Added cmake dependency check for libsasl2 on non-Windows platforms.

[Mesos Reviewbot Windows]

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



FAIL: Mesos tests failed to build. Please check 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62176/logs/mesos-tests-cmake-build.log
 for any relevant errors

Reviews applied: [62105, 62106, 62176]

Logs available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62176/logs

- Mesos Reviewbot Windows


On Sept. 7, 2017, 10:21 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62176/
> ---
> 
> (Updated Sept. 7, 2017, 10:21 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-3110
> https://issues.apache.org/jira/browse/MESOS-3110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cmake dependency check for libsasl2 on non-Windows platforms.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
> 
> 
> Diff: https://reviews.apache.org/r/62176/diff/1/
> 
> 
> Testing
> ---
> 
> I tested this on my Ubuntu 16.04 system. When the libsasl2 library doesn't 
> exist, it fails the cmake configure/build.  I did not test this on any other 
> platform, but this code is in a "if (NOT WIN32)" block and won't affect a 
> Windows build.  I'm uncertain if there is much support for other kinds of 
> builds (like Mac OS), but this should be platform independent.
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 62003: Added `network/ports` isolator nested container tests.

[Mesos Reviewbot Windows]

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



ERROR: Failed to apply patch 60491. Please check 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62003/logs/apply-reviews.log
 for any relevant errors

Reviews applied: [60491, 60493, 60494, 60764, 60901, 60836, 61538, 60902, 
60492, 60495, 61536, 60496, 60592, 60591, 60766, 60903, 60765, 60593, 62003]

Logs available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62003/logs

- Mesos Reviewbot Windows


On Sept. 7, 2017, 9:57 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62003/
> ---
> 
> (Updated Sept. 7, 2017, 9:57 p.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `network/ports` isolator nested container tests using the v1
> TaskGroups API. This tests that rogue port usage by a nested task is
> detected both with and without agent recovery, and that a well-behaved
> task is preserved across agent recovery.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/ports_isolator_tests.cpp PRE-CREATION 
>   src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554 
> 
> 
> Diff: https://reviews.apache.org/r/62003/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 62176: Added cmake dependency check for libsasl2 on non-Windows platforms.

[Andrew Schwartzmeyer]

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




src/CMakeLists.txt
Lines 615-620 (original), 615-624 (patched)


I added a note in the other review about `-DHAS_AUTHENTICATION` which we're 
not checking here. Rather than check it, I think the whole option should go 
away.



src/CMakeLists.txt
Lines 615-620 (original), 615-624 (patched)


As I mentioned in #62105, really all of this code should be contained in 
`3rdparty/CMakeLists.txt`, and the only change to this file should be the 
addition of `sasl2` to the above `target_link_libraries()` call.



src/CMakeLists.txt
Lines 616-619 (patched)


This logic I think is fine since there does not exist a `sasl2` CMake 
module (meaning we can't use `find_package()`. However, it should live in 
`3rdparty/CMakeLists.txt` next to the `sasl2` Windows setup.



src/CMakeLists.txt
Lines 617 (patched)


I know this is strange, but this can just be `if (SALS2_LIB)`, no string 
comparison needed. CMake, for better or worse, treats values like `*-NOTFOUND` 
as false.



src/CMakeLists.txt
Line 618 (original), 622 (patched)


This compilation definition should actually be set on the `cyrus_sasl` 
target as an interface compilation definition. E.g. 
`target_compile_definitions(cyrus_sasl PUBLIC LIBSASL_EXPORTS=1)`, right in 
`3rdparty/CMakeLists.txt` where the target is created. This ensures locality of 
configuration/compilation settings required for dependencies.


- Andrew Schwartzmeyer


On Sept. 7, 2017, 3:21 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62176/
> ---
> 
> (Updated Sept. 7, 2017, 3:21 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-3110
> https://issues.apache.org/jira/browse/MESOS-3110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cmake dependency check for libsasl2 on non-Windows platforms.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
> 
> 
> Diff: https://reviews.apache.org/r/62176/diff/1/
> 
> 
> Testing
> ---
> 
> I tested this on my Ubuntu 16.04 system. When the libsasl2 library doesn't 
> exist, it fails the cmake configure/build.  I did not test this on any other 
> platform, but this code is in a "if (NOT WIN32)" block and won't affect a 
> Windows build.  I'm uncertain if there is much support for other kinds of 
> builds (like Mac OS), but this should be platform independent.
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 62105: Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.

[Andrew Schwartzmeyer]

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




3rdparty/CMakeLists.txt
Lines 193-195 (patched)


We should actually call the target `sasl2` so that its target name matches 
that used on Linux.

Our goal is that this section of code contains all the necessary data to 
build and link to `sasl2`, such that any other target which needs to link to it 
can simply do `target_link_libraries(mesos PUBLIC sasl2)`. This can be acheived 
by keeping the target name identical, and setting the compilation definitions 
as public properties here.


- Andrew Schwartzmeyer


On Sept. 6, 2017, 6:11 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62105/
> ---
> 
> (Updated Sept. 6, 2017, 6:11 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-3384
> https://issues.apache.org/jira/browse/MESOS-3384
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 
>   3rdparty/cmake/Versions.cmake 0b62ae96817ade616f8dfc52cf8df995f8fd8927 
>   3rdparty/cyrus_sasl-2.1.27rc3.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62105/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 62106: Enabled CRAM MD5 Authentication on Windows and associated tests.

[Andrew Schwartzmeyer]

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




cmake/CompilationConfigure.cmake
Lines 109-112 (original), 109-112 (patched)


It occurred to me that if we're assuming authentication support is 
univesally available (now true on Windows since it's bundled, and is already 
assumed to be true elsewhere), we should delete this entire option and make 
sure it's not used anywhere.


- Andrew Schwartzmeyer


On Sept. 6, 2017, 6:11 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62106/
> ---
> 
> (Updated Sept. 6, 2017, 6:11 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-3384
> https://issues.apache.org/jira/browse/MESOS-3384
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled CRAM MD5 Authentication on Windows and associated tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 4cb8dddf93694722af2377c4a55a195260c8a0cb 
>   docs/windows.md b47ba5a0db3d41d6f4bb002283b40622a2bb3e0f 
>   src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
>   src/authentication/cram_md5/authenticatee.cpp 
> 7b3f767e29163de489018081089e67a6f22971c5 
>   src/authentication/cram_md5/authenticator.cpp 
> 2a12bb3f4aec79056ec6c82a8c896ee3af15bb4c 
>   src/authentication/cram_md5/auxprop.hpp 
> dedcbe514283dd86a924d92ccdd17173ebe878cb 
>   src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
>   src/tests/authentication_tests.cpp b04be76d23788fbde337eaf85716ec7436ade691 
>   support/windows-build.bat 100013ed68ecc2f431c20a3f081f8aa6eca961ef 
> 
> 
> Diff: https://reviews.apache.org/r/62106/diff/3/
> 
> 
> Testing
> ---
> 
> Ran the automake build and the cmake build on Linux, as well as the cmake 
> build on Windows, with no errors.  Ran tests on both platforms as well.  
> Note: The mesos-3rdparty pull request which contains the cyrus-sasl package 
> needs to be merged to master first before these patches will work. (Or, have 
> the cyrus-sasl tarball available in a local 3rdparty directory, configured 
> with mesos)
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 62047: Allowed look up latest executor directory by virtual path.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed. Please check 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62047/logs/mesos-tests-stdout.log
 and 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62047/logs/mesos-tests-stderr.log
 for any relevant errors

Reviews applied: [62040, 62174, 62047]

Logs available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62047/logs

- Mesos Reviewbot Windows


On Sept. 7, 2017, 9:18 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62047/
> ---
> 
> (Updated Sept. 7, 2017, 9:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jason Lai.
> 
> 
> Bugs: MESOS-7899
> https://issues.apache.org/jira/browse/MESOS-7899
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added support to attach a virtual path
> `/frameworks/{framework-id}/executors/{executor-id}/latest` to `Files` class
> in agent, which can be used to navigate an executor's latest sandbox without
> needing to know `work_dir` and `slave_id`.
> 
> 
> Diffs
> -
> 
>   src/slave/paths.hpp 51b481fc0870f1e95448f85ee2fd485fceea1919 
>   src/slave/paths.cpp 1a6943f03493079ab291616d3e36e0fd8809cf33 
>   src/slave/slave.cpp df920ec07cd59c7ba6baccfc1c20ed3809f187d6 
> 
> 
> Diff: https://reviews.apache.org/r/62047/diff/2/
> 
> 
> Testing
> ---
> 
> Ran a master/agent pair, launch a task with `mesos-execute` and use 
> `files/browse` endpoint to browse the virtual path.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 61183: Triggered 'UpdateSlaveMessage' when 'ResourceProviderManager' updates.

[Benjamin Bannier]

> On Sept. 7, 2017, 1:32 a.m., Jie Yu wrote:
> > src/slave/slave.hpp
> > Lines 658 (patched)
> > 
> >
> > I am not sure if keeping another field just for resoruce provider 
> > provided resources in the agent is a good idea or not.
> > 
> > I'd much prefer we keep a single `totalResources` for regular resources 
> > (both resource provider provided or not), and `oversubscribedResources` 
> > only for oversubsribed resources.
> 
> Benjamin Bannier wrote:
> The advantage of using a dedicated member is that with it it is possible 
> to detect whether any resources came from resource providers. With that we 
> can avoid sending `UpdateSlaveMessage`s e.g., on reregistration when no 
> resource providers registered (not sure how to do this without storing at 
> least some data).
> 
> I'd suggest introducing this member as it captures everything we need.
> 
> Jie Yu wrote:
> I think we should move toward a world to have just one field tracking the 
> total resources of the agent. With this patch, we'll have three: 
> `totalResources`, `oversubscribedResources` and `resourceProviderResources`. 
> It becomes very hard to read for someone who is not familiar with the code.
> 
> Regarding your comment on optimizing. I think the right way is to track 
> the last total sent to the master (i.e., `lastSyncedTotalResources`) and only 
> send `UpdateSlaveMessage` if there is a diff.
> 
> Jie Yu wrote:
> Another benefit is to be in sync with what we track in master. in master, 
> we track `slave->totalResources`. I don't think we want to keep a separate 
> `resourceProviderResources` in the master, right?

I adjusted the patch to track the last synced resources instead of explicitly 
tracking the resource provider resources.

I believe this requires updating `lastSyncedResources` in a few places where we 
currently trigger updates to the agents total.


- Benjamin


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


On Sept. 8, 2017, 12:38 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61183/
> ---
> 
> (Updated Sept. 8, 2017, 12:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The agent's resource provider manager sends a
> 'ResourceProviderMessage' when its managed resources change. This
> commit adds handling in the agent so that an 'UpdateSlaveMessage' is
> sent to the master to update the total resource available on the
> agent. We also store this total in the agent memory so that it can be
> resent on agent resubscription.
> 
> In order to provide push-like handling of the resource provider
> manager's message queue, we chain recursive calls to the handler for
> continuous processing. Initially, processing is kicked off from
> 'Slave::initialize'. In this simple implementation we e.g., provide no
> direct way to stop processing of messages, yet, but it can be achieved
> by e.g., replacing the manager with a new instance (this would also
> require updating routes).
> 
> Since the agent can only send an 'UpdateSlaveMessage' when it is
> registered with a master, a simple back-off of 5 s is implemented which
> will defer processing of a ready message should the agent not yet have
> registered.
> 
> To facilitate logging we add a stringification function for
> 'ResourceProviderMessage's.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
>   src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd 
>   src/tests/slave_tests.cpp 1bdadce4c50cbff958f2be2a4261e130b414acfd 
> 
> 
> Diff: https://reviews.apache.org/r/61183/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 61183: Triggered 'UpdateSlaveMessage' when 'ResourceProviderManager' updates.

[Benjamin Bannier]

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

(Updated Sept. 8, 2017, 12:38 a.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Avoid explicitly tracking resource provider resources as suggested by Jie.


Repository: mesos


Description
---

The agent's resource provider manager sends a
'ResourceProviderMessage' when its managed resources change. This
commit adds handling in the agent so that an 'UpdateSlaveMessage' is
sent to the master to update the total resource available on the
agent. We also store this total in the agent memory so that it can be
resent on agent resubscription.

In order to provide push-like handling of the resource provider
manager's message queue, we chain recursive calls to the handler for
continuous processing. Initially, processing is kicked off from
'Slave::initialize'. In this simple implementation we e.g., provide no
direct way to stop processing of messages, yet, but it can be achieved
by e.g., replacing the manager with a new instance (this would also
require updating routes).

Since the agent can only send an 'UpdateSlaveMessage' when it is
registered with a master, a simple back-off of 5 s is implemented which
will defer processing of a ready message should the agent not yet have
registered.

To facilitate logging we add a stringification function for
'ResourceProviderMessage's.


Diffs (updated)
-

  src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
  src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
  src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd 
  src/tests/slave_tests.cpp 1bdadce4c50cbff958f2be2a4261e130b414acfd 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

[Benjamin Bannier]

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

(Updated Sept. 8, 2017, 12:36 a.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


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

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


Testing (updated)
---

`make check`


Thanks,

Benjamin Bannier

Review Request 62176: Added cmake dependency check for libsasl2 on non-Windows platforms.

[John Kordich via Review Board]

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

Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.


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


Repository: mesos


Description
---

Added cmake dependency check for libsasl2 on non-Windows platforms.


Diffs
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 


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


Testing
---

I tested this on my Ubuntu 16.04 system. When the libsasl2 library doesn't 
exist, it fails the cmake configure/build.  I did not test this on any other 
platform, but this code is in a "if (NOT WIN32)" block and won't affect a 
Windows build.  I'm uncertain if there is much support for other kinds of 
builds (like Mac OS), but this should be platform independent.


Thanks,

John Kordich

Re: Review Request 62003: Added `network/ports` isolator nested container tests.

[James Peach]

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

(Updated Sept. 7, 2017, 9:57 p.m.)


Review request for mesos, Qian Zhang and Jiang Yan Xu.


Changes
---

Rebase and address review feedback.


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


Repository: mesos


Description
---

Added `network/ports` isolator nested container tests using the v1
TaskGroups API. This tests that rogue port usage by a nested task is
detected both with and without agent recovery, and that a well-behaved
task is preserved across agent recovery.


Diffs (updated)
-

  src/tests/containerizer/ports_isolator_tests.cpp PRE-CREATION 
  src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554 


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

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


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 60593: Added `network/ports` isolator recovery tests.

[James Peach]

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

(Updated Sept. 7, 2017, 9:57 p.m.)


Review request for mesos, Qian Zhang and Jiang Yan Xu.


Changes
---

Rebase and address review feedback.


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


Repository: mesos


Description
---

Added a test for the `network/ports` isolator recovery by starting
a task that is listening on a rogue port. We only configure the
isolator when we restart the agent to simulate the case where a
task only starts to misbehave after an agent recovery.


Diffs (updated)
-

  src/tests/containerizer/ports_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/60593/diff/15/

Changes: https://reviews.apache.org/r/60593/diff/14-15/


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 60593: Added `network/ports` isolator recovery tests.

[James Peach]

> On Aug. 23, 2017, 2:53 p.m., Qian Zhang wrote:
> > src/tests/containerizer/ports_isolator_tests.cpp
> > Lines 853 (patched)
> > 
> >
> > It seems there is no enough time left for this code because we stop the 
> > `driver` right after the check is triggered.
> 
> James Peach wrote:
> I looked at this for a while and consulted with @xujyan and we couldn't 
> come up with a way to prove that the process is still running. The `settle()` 
> gives us confidence that no limitation was raised, but I don't know how to 
> guarantee that. Can you sujjest an approach?

Fixed by verifying that we can explicitly kill the task (same fix for the 
corresponding nested container test).


- James


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


On Aug. 31, 2017, 12:08 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60593/
> ---
> 
> (Updated Aug. 31, 2017, 12:08 a.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for the `network/ports` isolator recovery by starting
> a task that is listening on a rogue port. We only configure the
> isolator when we restart the agent to simulate the case where a
> task only starts to misbehave after an agent recovery.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/ports_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60593/diff/14/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60593: Added `network/ports` isolator recovery tests.

[James Peach]

> On Sept. 6, 2017, 12:19 p.m., Qian Zhang wrote:
> > src/tests/containerizer/ports_isolator_tests.cpp
> > Lines 741 (patched)
> > 
> >
> > Why do we need to explicitly set this flag to such a large number?

I ended up removing this altogether. It is not necessary and setting it too 
high makes recovery tests flaky.


- James


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


On Aug. 31, 2017, 12:08 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60593/
> ---
> 
> (Updated Aug. 31, 2017, 12:08 a.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for the `network/ports` isolator recovery by starting
> a task that is listening on a rogue port. We only configure the
> isolator when we restart the agent to simulate the case where a
> task only starts to misbehave after an agent recovery.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/ports_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60593/diff/14/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60765: Added basic `network/ports` isolator tests.

[James Peach]

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

(Updated Sept. 7, 2017, 9:57 p.m.)


Review request for mesos, Qian Zhang and Jiang Yan Xu.


Changes
---

Rebase and address review feedback.


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


Repository: mesos


Description
---

Added tests to verify that the `network/ports` is able to correctly
terminate only tasks that use rogue TCP ports.


Diffs (updated)
-

  src/tests/containerizer/ports_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/60765/diff/14/

Changes: https://reviews.apache.org/r/60765/diff/13-14/


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 60495: Added network ports isolator listen socket utilities.

[James Peach]

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

(Updated Sept. 7, 2017, 9:50 p.m.)


Review request for mesos, Qian Zhang and Jiang Yan Xu.


Changes
---

Addressed review feedback.


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


Repository: mesos


Description
---

Implemented network ports isolator socket utilities to find the
inode numbers of all the listening sockets, and the inodes of the
open sockets for a process. Together, these utilities can tell us
which sockets a process is listening on.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/60495/diff/14/

Changes: https://reviews.apache.org/r/60495/diff/13-14/


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 60492: Added a `network/ports` isolator skeleton.

[James Peach]

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

(Updated Sept. 7, 2017, 9:50 p.m.)


Review request for mesos, Qian Zhang and Jiang Yan Xu.


Changes
---

Addressed review feedback.


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


Repository: mesos


Description
---

Added the skeleton of the `network/ports` isolator and wired it up
to the autotools build. Added the --enable-network-ports-isolator
configuration option to build the isolator and check for the libnl3
dependencies.


Diffs (updated)
-

  configure.ac 92bc1aa5f9604e3b2b678225a57622cd2eb8679a 
  src/Makefile.am a4f2ee008eba263fe52c4e58c305e3fb9ba42f6b 
  src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/60492/diff/13/

Changes: https://reviews.apache.org/r/60492/diff/12-13/


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 62047: Allowed look up latest executor directory by virtual path.

[Zhitao Li]

> On Sept. 6, 2017, 9:28 p.m., Benjamin Mahler wrote:
> > src/slave/slave.cpp
> > Lines 7466-7470 (original), 7484-7491 (patched)
> > 
> >
> > What was going on here originally with the two different calls?

rebase error


- Zhitao


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


On Sept. 1, 2017, 10:54 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62047/
> ---
> 
> (Updated Sept. 1, 2017, 10:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jason Lai.
> 
> 
> Bugs: MESOS-7899
> https://issues.apache.org/jira/browse/MESOS-7899
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added support to attach a virtual path
> `/frameworks/{framework-id}/executors/{executor-id}/latest` to `Files` class
> in agent, which can be used to navigate an executor's latest sandbox without
> needing to know `work_dir` and `slave_id`.
> 
> 
> Diffs
> -
> 
>   src/slave/paths.hpp 51b481fc0870f1e95448f85ee2fd485fceea1919 
>   src/slave/paths.cpp 1a6943f03493079ab291616d3e36e0fd8809cf33 
>   src/slave/slave.cpp df920ec07cd59c7ba6baccfc1c20ed3809f187d6 
> 
> 
> Diff: https://reviews.apache.org/r/62047/diff/1/
> 
> 
> Testing
> ---
> 
> Ran a master/agent pair, launch a task with `mesos-execute` and use 
> `files/browse` endpoint to browse the virtual path.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 62047: Allowed look up latest executor directory by virtual path.

[Zhitao Li]

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

(Updated Sept. 7, 2017, 9:18 p.m.)


Review request for mesos, Benjamin Mahler and Jason Lai.


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


Repository: mesos


Description
---

This patch added support to attach a virtual path
`/frameworks/{framework-id}/executors/{executor-id}/latest` to `Files` class
in agent, which can be used to navigate an executor's latest sandbox without
needing to know `work_dir` and `slave_id`.


Diffs (updated)
-

  src/slave/paths.hpp 51b481fc0870f1e95448f85ee2fd485fceea1919 
  src/slave/paths.cpp 1a6943f03493079ab291616d3e36e0fd8809cf33 
  src/slave/slave.cpp df920ec07cd59c7ba6baccfc1c20ed3809f187d6 


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

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


Testing
---

Ran a master/agent pair, launch a task with `mesos-execute` and use 
`files/browse` endpoint to browse the virtual path.


Thanks,

Zhitao Li

Re: Review Request 62040: Also log attached virtual path in agent.

[Zhitao Li]

> On Sept. 6, 2017, 8:57 p.m., Benjamin Mahler wrote:
> > src/slave/slave.hpp
> > Lines 377-378 (original), 377-379 (patched)
> > 
> >
> > How about path and virtualPath here and "virtual path" in the logging? 
> > We probably should have picked these names in the files API as well, feel 
> > free to make that change there if you like.

Also done in https://reviews.apache.org/r/62174


- Zhitao


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


On Sept. 7, 2017, 9:16 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62040/
> ---
> 
> (Updated Sept. 7, 2017, 9:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jason Lai.
> 
> 
> Bugs: MESOS-7899
> https://issues.apache.org/jira/browse/MESOS-7899
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The same physical directory will be mounted to multiple virtual paths,
>   so log which virtual path in agent log.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 435c37e28987119c4ef43c9dad61ac052020acd8 
>   src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
>   src/slave/slave.cpp df920ec07cd59c7ba6baccfc1c20ed3809f187d6 
> 
> 
> Diff: https://reviews.apache.org/r/62040/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Review Request 62174: Replace name with virtualPath in files API.

[Zhitao Li]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

The terminology for `virtualPath` reflects better on the nature of the `files` 
API.


Diffs
-

  src/files/files.hpp 1e123fac21ab98c508ca96112e080bd7d48389cb 
  src/files/files.cpp b03279ee0b23b3a33383c6d3e0c31faa97dab8eb 


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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 62040: Also log attached virtual path in agent.

[Zhitao Li]

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

(Updated Sept. 7, 2017, 9:16 p.m.)


Review request for mesos, Benjamin Mahler and Jason Lai.


Summary (updated)
-

Also log attached virtual path in agent.


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


Repository: mesos


Description (updated)
---

The same physical directory will be mounted to multiple virtual paths,
  so log which virtual path in agent log.


Diffs (updated)
-

  src/slave/constants.hpp 435c37e28987119c4ef43c9dad61ac052020acd8 
  src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
  src/slave/slave.cpp df920ec07cd59c7ba6baccfc1c20ed3809f187d6 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 62166: Updated RC tagging+voting mechanism.

[Vinod Kone]

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




docs/release-guide.md
Lines 162-164 (original), 162-164 (patched)


Update the comment?



docs/release-guide.md
Line 164 (original), 164 (patched)


I would just keep the old name `vote.sh`, you can think of pushing the git 
tag as just part of the vote process.



support/vote.sh
Line 147 (original), 153 (patched)


while you are here

s/up in Maven//


- Vinod Kone


On Sept. 7, 2017, 5:57 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62166/
> ---
> 
> (Updated Sept. 7, 2017, 5:57 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Till Toenshoff, Vinod Kone, and 
> Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Stop "releasing" maven artifacts for RC tags. The artificats should only be 
> placed in the staging repository for voting. Once the vote has passed, the 
> staging repository should be released. If the vote fails, the staging 
> repository should be dropped.
> * tag.sh and vote.sh scripts have been unified into rc_tag_vote.sh script.
> * Updated tagging/voting instructions in release guide.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md fa607287456dff5e078fe97690ef3728a6419e8a 
>   support/tag.sh 0ff90cb935bd719c6b4bca5c94f48df1ada5ded9 
>   support/vote.sh 98643a1ad4ced0bd18cec6bc4488add9acc229fa 
> 
> 
> Diff: https://reviews.apache.org/r/62166/diff/1/
> 
> 
> Testing
> ---
> 
> Used the updated script to tag and vote 1.4.0-rc4.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 62042: Change `EXPECT` to `ASSERT` when relying on the assertion afterwards.

[Mesos Reviewbot Windows]

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



ERROR: Failed to apply patch 61920. Please check 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62042/logs/apply-reviews.log
 for any relevant errors

Reviews applied: [61920, 61921, 61982, 62042]

Logs available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62042/logs

- Mesos Reviewbot Windows


On Sept. 7, 2017, 6:09 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62042/
> ---
> 
> (Updated Sept. 7, 2017, 6:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Joerg Schad.
> 
> 
> Bugs: MESOS-7877
> https://issues.apache.org/jira/browse/MESOS-7877
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change `EXPECT` to `ASSERT` when relying on the assertion afterwards.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 0902a66772d178505e83b81878b7878a45c7bb25 
>   src/tests/check_tests.cpp cc7be317030d147c5617e784a50e42b0365b9df6 
>   src/tests/container_logger_tests.cpp 
> 97e79792d3ea8023890ad2a705db47f2aeb419cf 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 68b8181b9c6eb9ce2e8c3c8980451364d410516b 
>   src/tests/containerizer/cpu_isolator_tests.cpp 
> f7cab58b75994dc38eede6a84556ae73d412e1a1 
>   src/tests/containerizer/environment_secret_isolator_tests.cpp 
> 38fef2d5f677f768a0533d1ac085b1197b3b764d 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> de42be9ae2019c7cdb24a0976a8565631a12fca3 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> a1ff9b5e67fcccb8aa9afd90ca704796c1b0a252 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 1ad9464e99b7dd4dfd2c9bd1a4c6c9c5cce615bd 
>   src/tests/containerizer/memory_isolator_tests.cpp 
> a7028da62b27b109cb455048acbea0976da9a140 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> eec2e05d399d097fcb997bb44e353b9c3e0563f7 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> f7a715812001f61748749c5cecbed9376fc9ef17 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 544eaef1215f309f67ae177249c209afc71c5d5e 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> 8bf6a11de85d4dacf081c009ace9e7b78e904849 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> 11dc7f596671d373cd0c9b443a1d217053285a63 
>   src/tests/default_executor_tests.cpp 
> 186b8333c02ba3b9257e19437c6d689761085362 
>   src/tests/disk_quota_tests.cpp 3bf0508238a228d86737d6cc899fa68e2046f2e2 
>   src/tests/exception_tests.cpp 0e72cdfce2f12a6961efefb23fe14eadbadce01f 
>   src/tests/fault_tolerance_tests.cpp 
> 5ac38a897fefb6f40d69ca6e27a6f23176e42d36 
>   src/tests/gc_tests.cpp da9a31c30e759e30492963c199da2b16e9f91550 
>   src/tests/health_check_tests.cpp 2c43241a5cc245a1797ea16c68c94638cec6803d 
>   src/tests/hook_tests.cpp c5acbd0fde7cb0923779a925fe2f6a8214be4163 
>   src/tests/http_fault_tolerance_tests.cpp 
> f05aee8200e05b10595fd821413f44bc36839de2 
>   src/tests/master_authorization_tests.cpp 
> a776a5811b66cb93cfc3484c183e815248dea944 
>   src/tests/master_maintenance_tests.cpp 
> 7595e8b1d8360920f61af4685e210c9a9f4569af 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 0b26953c73c3a41e7f5e4e62a41a0756fb2097e2 
>   src/tests/master_tests.cpp 59fbad468ae671a8fd39457632262015989f26c2 
>   src/tests/master_validation_tests.cpp 
> 710b25c1afdff4de7b2eb9b8a38f6856c373fb3c 
>   src/tests/oversubscription_tests.cpp 
> a4c4a6083a2cb55e2d69eba1719555a7a13ee8fb 
>   src/tests/persistent_volume_tests.cpp 
> 3e1d1fe468298186347b07b5882973c5a16231a3 
>   src/tests/reconciliation_tests.cpp 728961874239c7e5ce8accbaeb2a1c86a73a4f0f 
>   src/tests/registrar_zookeeper_tests.cpp 
> 96bae295bdbe839b979b879cc4b11fddcce6ad75 
>   src/tests/reservation_endpoints_tests.cpp 
> 3278732cb130c73be0f20c0204eddaee05123ff9 
>   src/tests/resource_offers_tests.cpp 
> dc9c230548ba49e92228c6625597b2294e65ca30 
>   src/tests/scheduler_driver_tests.cpp 
> d2aff699dc33d2a246b391d6322d271e7f0252b6 
>   src/tests/scheduler_tests.cpp 21f8825c214bb0c331f66bfb9b12a354d6673a43 
>   src/tests/slave_authorization_tests.cpp 
> 30eceae0920351cffc9c3393c6d08917c4041c1a 
>   src/tests/slave_recovery_tests.cpp 0e46748be266809c413fb10fc447516c51504fce 
>   src/tests/slave_tests.cpp 1bdadce4c50cbff958f2be2a4261e130b414acfd 
>   src/tests/status_update_manager_tests.cpp 
> 6922ee353123bc024c66a6bc4fadc96450287447 
>   src/tests/teardown_tests.cpp 8834333bbcdb6a04a95a6ed9632ad4ead0791f76 
> 
> 
> Diff: https://reviews.apache.org/r/62042/diff/2/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh` on GNU/Linux
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 62166: Updated RC tagging+voting mechanism.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed. Please check 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62166/logs/mesos-tests-stdout.log
 and 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62166/logs/mesos-tests-stderr.log
 for any relevant errors

Reviews applied: [62166]

Logs available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62166/logs

- Mesos Reviewbot Windows


On Sept. 7, 2017, 1:57 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62166/
> ---
> 
> (Updated Sept. 7, 2017, 1:57 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Till Toenshoff, Vinod Kone, and 
> Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Stop "releasing" maven artifacts for RC tags. The artificats should only be 
> placed in the staging repository for voting. Once the vote has passed, the 
> staging repository should be released. If the vote fails, the staging 
> repository should be dropped.
> * tag.sh and vote.sh scripts have been unified into rc_tag_vote.sh script.
> * Updated tagging/voting instructions in release guide.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md fa607287456dff5e078fe97690ef3728a6419e8a 
>   support/tag.sh 0ff90cb935bd719c6b4bca5c94f48df1ada5ded9 
>   support/vote.sh 98643a1ad4ced0bd18cec6bc4488add9acc229fa 
> 
> 
> Diff: https://reviews.apache.org/r/62166/diff/1/
> 
> 
> Testing
> ---
> 
> Used the updated script to tag and vote 1.4.0-rc4.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 60890: WIP: Defined API for launching standalone containers.

[Jie Yu]

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


Fix it, then Ship it!





include/mesos/agent/agent.proto
Lines 188 (patched)


Remove the `within an executor's tree of containers` because nested 
container can be nested under top-level container as well?

Also, i'd try to use the same term to refer to 'standalone container'. 
Let's avoid using the term `top level container` to refer to standalone 
container because top level container also applies to executor's container.


- Jie Yu


On Sept. 7, 2017, 12:37 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60890/
> ---
> 
> (Updated Sept. 7, 2017, 12:37 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Launching a standalone container is very similar to launching a
> nested container, except that the caller must specify some Resources.
> As such, this patch deprecates some nested container APIs
> and replaces them with more generically named APIs.
> 
> This applies to the Launch, Wait, Kill, and Remove
> (nested) container APIs.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 7c8c8a7d8298e91e4e002327b3b27d4c74b5cbae 
>   include/mesos/v1/agent/agent.proto 3e199124b23fa027232790d99370fe2f33660096 
> 
> 
> Diff: https://reviews.apache.org/r/60890/diff/3/
> 
> 
> Testing
> ---
> 
> make
> 
> See later patches in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 61183: Triggered 'UpdateSlaveMessage' when 'ResourceProviderManager' updates.

[Jie Yu]

> On Sept. 6, 2017, 11:32 p.m., Jie Yu wrote:
> > src/slave/slave.hpp
> > Lines 658 (patched)
> > 
> >
> > I am not sure if keeping another field just for resoruce provider 
> > provided resources in the agent is a good idea or not.
> > 
> > I'd much prefer we keep a single `totalResources` for regular resources 
> > (both resource provider provided or not), and `oversubscribedResources` 
> > only for oversubsribed resources.
> 
> Benjamin Bannier wrote:
> The advantage of using a dedicated member is that with it it is possible 
> to detect whether any resources came from resource providers. With that we 
> can avoid sending `UpdateSlaveMessage`s e.g., on reregistration when no 
> resource providers registered (not sure how to do this without storing at 
> least some data).
> 
> I'd suggest introducing this member as it captures everything we need.
> 
> Jie Yu wrote:
> I think we should move toward a world to have just one field tracking the 
> total resources of the agent. With this patch, we'll have three: 
> `totalResources`, `oversubscribedResources` and `resourceProviderResources`. 
> It becomes very hard to read for someone who is not familiar with the code.
> 
> Regarding your comment on optimizing. I think the right way is to track 
> the last total sent to the master (i.e., `lastSyncedTotalResources`) and only 
> send `UpdateSlaveMessage` if there is a diff.

Another benefit is to be in sync with what we track in master. in master, we 
track `slave->totalResources`. I don't think we want to keep a separate 
`resourceProviderResources` in the master, right?


- Jie


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


On Sept. 7, 2017, 3:13 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61183/
> ---
> 
> (Updated Sept. 7, 2017, 3:13 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The agent's resource provider manager sends a
> 'ResourceProviderMessage' when its managed resources change. This
> commit adds handling in the agent so that an 'UpdateSlaveMessage' is
> sent to the master to update the total resource available on the
> agent. We also store this total in the agent memory so that it can be
> resent on agent resubscription.
> 
> In order to provide push-like handling of the resource provider
> manager's message queue, we chain recursive calls to the handler for
> continuous processing. Initially, processing is kicked off from
> 'Slave::initialize'. In this simple implementation we e.g., provide no
> direct way to stop processing of messages, yet, but it can be achieved
> by e.g., replacing the manager with a new instance (this would also
> require updating routes).
> 
> Since the agent can only send an 'UpdateSlaveMessage' when it is
> registered with a master, a simple back-off of 5 s is implemented which
> will defer processing of a ready message should the agent not yet have
> registered.
> 
> To facilitate logging we add a stringification function for
> 'ResourceProviderMessage's.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
>   src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd 
>   src/tests/slave_tests.cpp 1bdadce4c50cbff958f2be2a4261e130b414acfd 
> 
> 
> Diff: https://reviews.apache.org/r/61183/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 61183: Triggered 'UpdateSlaveMessage' when 'ResourceProviderManager' updates.

[Jie Yu]

> On Sept. 6, 2017, 11:32 p.m., Jie Yu wrote:
> > src/slave/slave.hpp
> > Lines 658 (patched)
> > 
> >
> > I am not sure if keeping another field just for resoruce provider 
> > provided resources in the agent is a good idea or not.
> > 
> > I'd much prefer we keep a single `totalResources` for regular resources 
> > (both resource provider provided or not), and `oversubscribedResources` 
> > only for oversubsribed resources.
> 
> Benjamin Bannier wrote:
> The advantage of using a dedicated member is that with it it is possible 
> to detect whether any resources came from resource providers. With that we 
> can avoid sending `UpdateSlaveMessage`s e.g., on reregistration when no 
> resource providers registered (not sure how to do this without storing at 
> least some data).
> 
> I'd suggest introducing this member as it captures everything we need.

I think we should move toward a world to have just one field tracking the total 
resources of the agent. With this patch, we'll have three: `totalResources`, 
`oversubscribedResources` and `resourceProviderResources`. It becomes very hard 
to read for someone who is not familiar with the code.

Regarding your comment on optimizing. I think the right way is to track the 
last total sent to the master (i.e., `lastSyncedTotalResources`) and only send 
`UpdateSlaveMessage` if there is a diff.


- Jie


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


On Sept. 7, 2017, 3:13 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61183/
> ---
> 
> (Updated Sept. 7, 2017, 3:13 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The agent's resource provider manager sends a
> 'ResourceProviderMessage' when its managed resources change. This
> commit adds handling in the agent so that an 'UpdateSlaveMessage' is
> sent to the master to update the total resource available on the
> agent. We also store this total in the agent memory so that it can be
> resent on agent resubscription.
> 
> In order to provide push-like handling of the resource provider
> manager's message queue, we chain recursive calls to the handler for
> continuous processing. Initially, processing is kicked off from
> 'Slave::initialize'. In this simple implementation we e.g., provide no
> direct way to stop processing of messages, yet, but it can be achieved
> by e.g., replacing the manager with a new instance (this would also
> require updating routes).
> 
> Since the agent can only send an 'UpdateSlaveMessage' when it is
> registered with a master, a simple back-off of 5 s is implemented which
> will defer processing of a ready message should the agent not yet have
> registered.
> 
> To facilitate logging we add a stringification function for
> 'ResourceProviderMessage's.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
>   src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd 
>   src/tests/slave_tests.cpp 1bdadce4c50cbff958f2be2a4261e130b414acfd 
> 
> 
> Diff: https://reviews.apache.org/r/61183/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 62042: Change `EXPECT` to `ASSERT` when relying on the assertion afterwards.

[Gastón Kleiman]

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

(Updated Sept. 7, 2017, 6:09 p.m.)


Review request for mesos, Benjamin Bannier, Greg Mann, and Joerg Schad.


Changes
---

Rebased + fixed a few occurences that I had missed.


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


Repository: mesos


Description
---

Change `EXPECT` to `ASSERT` when relying on the assertion afterwards.


Diffs (updated)
-

  src/tests/api_tests.cpp 0902a66772d178505e83b81878b7878a45c7bb25 
  src/tests/check_tests.cpp cc7be317030d147c5617e784a50e42b0365b9df6 
  src/tests/container_logger_tests.cpp 97e79792d3ea8023890ad2a705db47f2aeb419cf 
  src/tests/containerizer/cgroups_isolator_tests.cpp 
68b8181b9c6eb9ce2e8c3c8980451364d410516b 
  src/tests/containerizer/cpu_isolator_tests.cpp 
f7cab58b75994dc38eede6a84556ae73d412e1a1 
  src/tests/containerizer/environment_secret_isolator_tests.cpp 
38fef2d5f677f768a0533d1ac085b1197b3b764d 
  src/tests/containerizer/io_switchboard_tests.cpp 
de42be9ae2019c7cdb24a0976a8565631a12fca3 
  src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
a1ff9b5e67fcccb8aa9afd90ca704796c1b0a252 
  src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
1ad9464e99b7dd4dfd2c9bd1a4c6c9c5cce615bd 
  src/tests/containerizer/memory_isolator_tests.cpp 
a7028da62b27b109cb455048acbea0976da9a140 
  src/tests/containerizer/memory_pressure_tests.cpp 
eec2e05d399d097fcb997bb44e353b9c3e0563f7 
  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
f7a715812001f61748749c5cecbed9376fc9ef17 
  src/tests/containerizer/port_mapping_tests.cpp 
544eaef1215f309f67ae177249c209afc71c5d5e 
  src/tests/containerizer/runtime_isolator_tests.cpp 
8bf6a11de85d4dacf081c009ace9e7b78e904849 
  src/tests/containerizer/xfs_quota_tests.cpp 
11dc7f596671d373cd0c9b443a1d217053285a63 
  src/tests/default_executor_tests.cpp 186b8333c02ba3b9257e19437c6d689761085362 
  src/tests/disk_quota_tests.cpp 3bf0508238a228d86737d6cc899fa68e2046f2e2 
  src/tests/exception_tests.cpp 0e72cdfce2f12a6961efefb23fe14eadbadce01f 
  src/tests/fault_tolerance_tests.cpp 5ac38a897fefb6f40d69ca6e27a6f23176e42d36 
  src/tests/gc_tests.cpp da9a31c30e759e30492963c199da2b16e9f91550 
  src/tests/health_check_tests.cpp 2c43241a5cc245a1797ea16c68c94638cec6803d 
  src/tests/hook_tests.cpp c5acbd0fde7cb0923779a925fe2f6a8214be4163 
  src/tests/http_fault_tolerance_tests.cpp 
f05aee8200e05b10595fd821413f44bc36839de2 
  src/tests/master_authorization_tests.cpp 
a776a5811b66cb93cfc3484c183e815248dea944 
  src/tests/master_maintenance_tests.cpp 
7595e8b1d8360920f61af4685e210c9a9f4569af 
  src/tests/master_slave_reconciliation_tests.cpp 
0b26953c73c3a41e7f5e4e62a41a0756fb2097e2 
  src/tests/master_tests.cpp 59fbad468ae671a8fd39457632262015989f26c2 
  src/tests/master_validation_tests.cpp 
710b25c1afdff4de7b2eb9b8a38f6856c373fb3c 
  src/tests/oversubscription_tests.cpp a4c4a6083a2cb55e2d69eba1719555a7a13ee8fb 
  src/tests/persistent_volume_tests.cpp 
3e1d1fe468298186347b07b5882973c5a16231a3 
  src/tests/reconciliation_tests.cpp 728961874239c7e5ce8accbaeb2a1c86a73a4f0f 
  src/tests/registrar_zookeeper_tests.cpp 
96bae295bdbe839b979b879cc4b11fddcce6ad75 
  src/tests/reservation_endpoints_tests.cpp 
3278732cb130c73be0f20c0204eddaee05123ff9 
  src/tests/resource_offers_tests.cpp dc9c230548ba49e92228c6625597b2294e65ca30 
  src/tests/scheduler_driver_tests.cpp d2aff699dc33d2a246b391d6322d271e7f0252b6 
  src/tests/scheduler_tests.cpp 21f8825c214bb0c331f66bfb9b12a354d6673a43 
  src/tests/slave_authorization_tests.cpp 
30eceae0920351cffc9c3393c6d08917c4041c1a 
  src/tests/slave_recovery_tests.cpp 0e46748be266809c413fb10fc447516c51504fce 
  src/tests/slave_tests.cpp 1bdadce4c50cbff958f2be2a4261e130b414acfd 
  src/tests/status_update_manager_tests.cpp 
6922ee353123bc024c66a6bc4fadc96450287447 
  src/tests/teardown_tests.cpp 8834333bbcdb6a04a95a6ed9632ad4ead0791f76 


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

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


Testing
---

`sudo bin/mesos-tests.sh` on GNU/Linux


Thanks,

Gastón Kleiman

Re: Review Request 62042: Change `EXPECT` to `ASSERT` when relying on the assertion afterwards.

[Mesos Reviewbot Windows]

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



ERROR: Failed to apply patch 61920. Please check 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62042/logs/apply-reviews.log
 for any relevant errors

Reviews applied: [61920, 61921, 61982, 62042]

Logs available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62042/logs

- Mesos Reviewbot Windows


On Sept. 1, 2017, 9:27 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62042/
> ---
> 
> (Updated Sept. 1, 2017, 9:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Joerg Schad.
> 
> 
> Bugs: MESOS-7877
> https://issues.apache.org/jira/browse/MESOS-7877
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change `EXPECT` to `ASSERT` when relying on the assertion afterwards.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 0902a66772d178505e83b81878b7878a45c7bb25 
>   src/tests/check_tests.cpp cc7be317030d147c5617e784a50e42b0365b9df6 
>   src/tests/container_logger_tests.cpp 
> 97e79792d3ea8023890ad2a705db47f2aeb419cf 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 68b8181b9c6eb9ce2e8c3c8980451364d410516b 
>   src/tests/containerizer/cpu_isolator_tests.cpp 
> f7cab58b75994dc38eede6a84556ae73d412e1a1 
>   src/tests/containerizer/environment_secret_isolator_tests.cpp 
> 38fef2d5f677f768a0533d1ac085b1197b3b764d 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> de42be9ae2019c7cdb24a0976a8565631a12fca3 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> a1ff9b5e67fcccb8aa9afd90ca704796c1b0a252 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 1ad9464e99b7dd4dfd2c9bd1a4c6c9c5cce615bd 
>   src/tests/containerizer/memory_isolator_tests.cpp 
> a7028da62b27b109cb455048acbea0976da9a140 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> eec2e05d399d097fcb997bb44e353b9c3e0563f7 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 544eaef1215f309f67ae177249c209afc71c5d5e 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> 8bf6a11de85d4dacf081c009ace9e7b78e904849 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> 11dc7f596671d373cd0c9b443a1d217053285a63 
>   src/tests/default_executor_tests.cpp 
> 186b8333c02ba3b9257e19437c6d689761085362 
>   src/tests/disk_quota_tests.cpp 3bf0508238a228d86737d6cc899fa68e2046f2e2 
>   src/tests/exception_tests.cpp 0e72cdfce2f12a6961efefb23fe14eadbadce01f 
>   src/tests/fault_tolerance_tests.cpp 
> 5ac38a897fefb6f40d69ca6e27a6f23176e42d36 
>   src/tests/gc_tests.cpp da9a31c30e759e30492963c199da2b16e9f91550 
>   src/tests/health_check_tests.cpp 2c43241a5cc245a1797ea16c68c94638cec6803d 
>   src/tests/hook_tests.cpp c5acbd0fde7cb0923779a925fe2f6a8214be4163 
>   src/tests/http_fault_tolerance_tests.cpp 
> f05aee8200e05b10595fd821413f44bc36839de2 
>   src/tests/master_authorization_tests.cpp 
> a776a5811b66cb93cfc3484c183e815248dea944 
>   src/tests/master_maintenance_tests.cpp 
> 7595e8b1d8360920f61af4685e210c9a9f4569af 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 0b26953c73c3a41e7f5e4e62a41a0756fb2097e2 
>   src/tests/master_tests.cpp 59fbad468ae671a8fd39457632262015989f26c2 
>   src/tests/master_validation_tests.cpp 
> 710b25c1afdff4de7b2eb9b8a38f6856c373fb3c 
>   src/tests/oversubscription_tests.cpp 
> a4c4a6083a2cb55e2d69eba1719555a7a13ee8fb 
>   src/tests/persistent_volume_tests.cpp 
> 3e1d1fe468298186347b07b5882973c5a16231a3 
>   src/tests/reconciliation_tests.cpp 728961874239c7e5ce8accbaeb2a1c86a73a4f0f 
>   src/tests/registrar_zookeeper_tests.cpp 
> 96bae295bdbe839b979b879cc4b11fddcce6ad75 
>   src/tests/reservation_endpoints_tests.cpp 
> 3278732cb130c73be0f20c0204eddaee05123ff9 
>   src/tests/resource_offers_tests.cpp 
> dc9c230548ba49e92228c6625597b2294e65ca30 
>   src/tests/scheduler_driver_tests.cpp 
> d2aff699dc33d2a246b391d6322d271e7f0252b6 
>   src/tests/scheduler_tests.cpp 21f8825c214bb0c331f66bfb9b12a354d6673a43 
>   src/tests/slave_authorization_tests.cpp 
> 536a8efda0cc1ba08285c379b01af6ec64a82117 
>   src/tests/slave_recovery_tests.cpp 0e46748be266809c413fb10fc447516c51504fce 
>   src/tests/slave_tests.cpp 1bdadce4c50cbff958f2be2a4261e130b414acfd 
>   src/tests/status_update_manager_tests.cpp 
> 6922ee353123bc024c66a6bc4fadc96450287447 
>   src/tests/teardown_tests.cpp 8834333bbcdb6a04a95a6ed9632ad4ead0791f76 
> 
> 
> Diff: https://reviews.apache.org/r/62042/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh` on GNU/Linux
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Review Request 62166: Updated RC tagging+voting mechanism.

[Kapil Arya]

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

Review request for mesos, Anand Mazumdar, Till Toenshoff, Vinod Kone, and 
Zhitao Li.


Repository: mesos


Description
---

* Stop "releasing" maven artifacts for RC tags. The artificats should only be 
placed in the staging repository for voting. Once the vote has passed, the 
staging repository should be released. If the vote fails, the staging 
repository should be dropped.
* tag.sh and vote.sh scripts have been unified into rc_tag_vote.sh script.
* Updated tagging/voting instructions in release guide.


Diffs
-

  docs/release-guide.md fa607287456dff5e078fe97690ef3728a6419e8a 
  support/tag.sh 0ff90cb935bd719c6b4bca5c94f48df1ada5ded9 
  support/vote.sh 98643a1ad4ced0bd18cec6bc4488add9acc229fa 


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


Testing
---

Used the updated script to tag and vote 1.4.0-rc4.


Thanks,

Kapil Arya

Re: Review Request 62042: Change `EXPECT` to `ASSERT` when relying on the assertion afterwards.

[Greg Mann]

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



I think this needs a rebase; I got a conflict in 
'slave_authorization_tests.cpp' when applying.

- Greg Mann


On Sept. 1, 2017, 9:27 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62042/
> ---
> 
> (Updated Sept. 1, 2017, 9:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Joerg Schad.
> 
> 
> Bugs: MESOS-7877
> https://issues.apache.org/jira/browse/MESOS-7877
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change `EXPECT` to `ASSERT` when relying on the assertion afterwards.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 0902a66772d178505e83b81878b7878a45c7bb25 
>   src/tests/check_tests.cpp cc7be317030d147c5617e784a50e42b0365b9df6 
>   src/tests/container_logger_tests.cpp 
> 97e79792d3ea8023890ad2a705db47f2aeb419cf 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 68b8181b9c6eb9ce2e8c3c8980451364d410516b 
>   src/tests/containerizer/cpu_isolator_tests.cpp 
> f7cab58b75994dc38eede6a84556ae73d412e1a1 
>   src/tests/containerizer/environment_secret_isolator_tests.cpp 
> 38fef2d5f677f768a0533d1ac085b1197b3b764d 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> de42be9ae2019c7cdb24a0976a8565631a12fca3 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> a1ff9b5e67fcccb8aa9afd90ca704796c1b0a252 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 1ad9464e99b7dd4dfd2c9bd1a4c6c9c5cce615bd 
>   src/tests/containerizer/memory_isolator_tests.cpp 
> a7028da62b27b109cb455048acbea0976da9a140 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> eec2e05d399d097fcb997bb44e353b9c3e0563f7 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 544eaef1215f309f67ae177249c209afc71c5d5e 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> 8bf6a11de85d4dacf081c009ace9e7b78e904849 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> 11dc7f596671d373cd0c9b443a1d217053285a63 
>   src/tests/default_executor_tests.cpp 
> 186b8333c02ba3b9257e19437c6d689761085362 
>   src/tests/disk_quota_tests.cpp 3bf0508238a228d86737d6cc899fa68e2046f2e2 
>   src/tests/exception_tests.cpp 0e72cdfce2f12a6961efefb23fe14eadbadce01f 
>   src/tests/fault_tolerance_tests.cpp 
> 5ac38a897fefb6f40d69ca6e27a6f23176e42d36 
>   src/tests/gc_tests.cpp da9a31c30e759e30492963c199da2b16e9f91550 
>   src/tests/health_check_tests.cpp 2c43241a5cc245a1797ea16c68c94638cec6803d 
>   src/tests/hook_tests.cpp c5acbd0fde7cb0923779a925fe2f6a8214be4163 
>   src/tests/http_fault_tolerance_tests.cpp 
> f05aee8200e05b10595fd821413f44bc36839de2 
>   src/tests/master_authorization_tests.cpp 
> a776a5811b66cb93cfc3484c183e815248dea944 
>   src/tests/master_maintenance_tests.cpp 
> 7595e8b1d8360920f61af4685e210c9a9f4569af 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 0b26953c73c3a41e7f5e4e62a41a0756fb2097e2 
>   src/tests/master_tests.cpp 59fbad468ae671a8fd39457632262015989f26c2 
>   src/tests/master_validation_tests.cpp 
> 710b25c1afdff4de7b2eb9b8a38f6856c373fb3c 
>   src/tests/oversubscription_tests.cpp 
> a4c4a6083a2cb55e2d69eba1719555a7a13ee8fb 
>   src/tests/persistent_volume_tests.cpp 
> 3e1d1fe468298186347b07b5882973c5a16231a3 
>   src/tests/reconciliation_tests.cpp 728961874239c7e5ce8accbaeb2a1c86a73a4f0f 
>   src/tests/registrar_zookeeper_tests.cpp 
> 96bae295bdbe839b979b879cc4b11fddcce6ad75 
>   src/tests/reservation_endpoints_tests.cpp 
> 3278732cb130c73be0f20c0204eddaee05123ff9 
>   src/tests/resource_offers_tests.cpp 
> dc9c230548ba49e92228c6625597b2294e65ca30 
>   src/tests/scheduler_driver_tests.cpp 
> d2aff699dc33d2a246b391d6322d271e7f0252b6 
>   src/tests/scheduler_tests.cpp 21f8825c214bb0c331f66bfb9b12a354d6673a43 
>   src/tests/slave_authorization_tests.cpp 
> 536a8efda0cc1ba08285c379b01af6ec64a82117 
>   src/tests/slave_recovery_tests.cpp 0e46748be266809c413fb10fc447516c51504fce 
>   src/tests/slave_tests.cpp 1bdadce4c50cbff958f2be2a4261e130b414acfd 
>   src/tests/status_update_manager_tests.cpp 
> 6922ee353123bc024c66a6bc4fadc96450287447 
>   src/tests/teardown_tests.cpp 8834333bbcdb6a04a95a6ed9632ad4ead0791f76 
> 
> 
> Diff: https://reviews.apache.org/r/62042/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh` on GNU/Linux
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61947: Implemented handling of resource provider offer operations.

[Mesos Reviewbot Windows]

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



ERROR: Failed to apply patch 57911. Please check 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/61947/logs/apply-reviews.log
 for any relevant errors

Reviews applied: [57911, 57997, 57998, 57999, 58048, 58047, 58021, 61810, 
61946, 61947]

Logs available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/61947/logs

- Mesos Reviewbot Windows


On Sept. 7, 2017, 12:27 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61947/
> ---
> 
> (Updated Sept. 7, 2017, 12:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7594
> https://issues.apache.org/jira/browse/MESOS-7594
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the resource provider manager is responsible to apply offer
> operations by sending events to the respective resource providers,
> the master takes care of accepting these operations. Hence, for local
> resource providers the master sends an `ApplyResourceOperationMessage`
> to the agent where the resource provider is running on. The agent
> then relays the operation contained in the message to the resource
> provider manager.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 14d94cfa0e96881f77dd6e0c3449fdc0613e36ce 
>   src/messages/messages.proto dc4e19c08d671743f08223dcdf1dbc336b3940bd 
>   src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
>   src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd 
>   src/tests/resource_provider_manager_tests.cpp 
> 3bc56b51526e9dd188423f7349e74246c3295c77 
> 
> 
> Diff: https://reviews.apache.org/r/61947/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 62162: Revert usage of `-isystem` flag.

[Mesos Reviewbot Windows]

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



Bad review!

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

- Mesos Reviewbot Windows


On Sept. 7, 2017, 8:22 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62162/
> ---
> 
> (Updated Sept. 7, 2017, 8:22 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This flag break the build of mesos against system libraries
> installed under /usr, because it generates a command line
> of `-isystem /usr/include`, which is explicitly not supported
> by gcc. See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70129
> 
> 
> Diffs
> -
> 
>   configure.ac 92bc1aa5f9604e3b2b678225a57622cd2eb8679a 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
> 
> 
> Diff: https://reviews.apache.org/r/62162/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 62140: Shorten the default interval between disk-usage GCs.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed. Please check 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62148/logs/mesos-tests-stdout.log
 and 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62148/logs/mesos-tests-stderr.log
 for any relevant errors

Reviews applied: [62140]

Logs available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62148/logs

- Mesos Reviewbot Windows


On Sept. 7, 2017, 9:31 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62140/
> ---
> 
> (Updated Sept. 7, 2017, 9:31 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Jie Yu, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7939
> https://issues.apache.org/jira/browse/MESOS-7939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Make the default value of `disk_watch_interval` to 1 second so GC can be
> kicked in early enough before the agent fails due to a "No space left on
> disk" failure. This change would also trigger the 1st GC earlier during
> agent startup, so it can recover from the above scenario.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 435c37e28987119c4ef43c9dad61ac052020acd8 
> 
> 
> Diff: https://reviews.apache.org/r/62140/diff/2/
> 
> 
> Testing
> ---
> 
> make test
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 62147: Added a comment about master sending a checkpointed resources message.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed. Please check 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62148/logs/mesos-tests-stdout.log
 and 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62148/logs/mesos-tests-stderr.log
 for any relevant errors

Reviews applied: [62147]

Logs available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62148/logs

- Mesos Reviewbot Windows


On Sept. 7, 2017, 4:11 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62147/
> ---
> 
> (Updated Sept. 7, 2017, 4:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Michael Park, 
> and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a comment about master sending a checkpointed resources message.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 14d94cfa0e96881f77dd6e0c3449fdc0613e36ce 
> 
> 
> Diff: https://reviews.apache.org/r/62147/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62148: Made the `stop()` logic in the scheduler library simpler.

[Anand Mazumdar]

> On Sept. 7, 2017, 3:33 p.m., Benjamin Hindman wrote:
> > include/mesos/v1/scheduler.hpp
> > Lines 25 (patched)
> > 
> >
> > Have we pulled in libprocess dependencies in public headers in the past?

hmm, most of our module/hooks interfaces already had libprocess dependency 
(future.hpp) in their headers. So, I assumed that it should be fine to add it. 
That being said; both the C++ driver/v1 scheduler/executor library haven't had 
a dependency in the past.


- Anand


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


On Sept. 7, 2017, 5:54 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62148/
> ---
> 
> (Updated Sept. 7, 2017, 5:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of relying on `Clock::pause()/settle()` to ensure that no
> callbacks on the scheduler were in flight, the test
> helper can wait on the future returned by `stop()` now. This is
> possible as we acquire the mutex explicitly in `stop()` to ensure
> that there can't be any queued callbacks that get executed after
> `TestMesos` is destroyed.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661 
>   src/scheduler/scheduler.cpp ce69258027ed50867569374d2d827fc3cc651744 
>   src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554 
> 
> 
> Diff: https://reviews.apache.org/r/62148/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Ensured tests still work when run in a loop)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62148: Made the `stop()` logic in the scheduler library simpler.

[Benjamin Hindman]

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




include/mesos/v1/scheduler.hpp
Lines 25 (patched)


Have we pulled in libprocess dependencies in public headers in the past?


- Benjamin Hindman


On Sept. 7, 2017, 5:54 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62148/
> ---
> 
> (Updated Sept. 7, 2017, 5:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of relying on `Clock::pause()/settle()` to ensure that no
> callbacks on the scheduler were in flight, the test
> helper can wait on the future returned by `stop()` now. This is
> possible as we acquire the mutex explicitly in `stop()` to ensure
> that there can't be any queued callbacks that get executed after
> `TestMesos` is destroyed.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661 
>   src/scheduler/scheduler.cpp ce69258027ed50867569374d2d827fc3cc651744 
>   src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554 
> 
> 
> Diff: https://reviews.apache.org/r/62148/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Ensured tests still work when run in a loop)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 62162: Revert usage of `-isystem` flag.

[Benno Evers]

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

Review request for mesos.


Repository: mesos


Description
---

This flag break the build of mesos against system libraries
installed under /usr, because it generates a command line
of `-isystem /usr/include`, which is explicitly not supported
by gcc. See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70129


Diffs
-

  configure.ac 92bc1aa5f9604e3b2b678225a57622cd2eb8679a 
  src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 


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


Testing
---


Thanks,

Benno Evers

Review Request 62161: Update boost version.

[Benno Evers]

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

Review request for mesos.


Repository: mesos


Description
---

Update boost version.


Diffs
-

  3rdparty/boost-1.53.0.tar.gz 175f373c330ff69bdb0d44fc75a29982e68554ab 
  3rdparty/boost-1.65.0.tar.gz PRE-CREATION 
  3rdparty/versions.am cbab26d3e303180c3f210f0c5f45613a5c8c96ba 


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


Testing
---


Thanks,

Benno Evers

Review Request 62160: Fix stout build with newer boost versions.

[Benno Evers]

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

Review request for mesos.


Repository: mesos


Description
---

Starting from Boost 1.62, Boost.Variant added additional
compile-time checks to its constructors to fix this
issue: https://svn.boost.org/trac10/ticket/11602

However, this breaks some places in stout which try
to access a derived class from a variant holding the
base class.


Diffs
-

  3rdparty/stout/include/stout/json.hpp 
4f9ea1b34537026af80edd97fa0b3ae1a3dfa862 
  3rdparty/stout/include/stout/protobuf.hpp 
15690b66cc4ae0c1bf2c2176d73c385ca75d3c20 
  3rdparty/stout/tests/json_tests.cpp adaa343faf3b557ad483675318b22eb90b1eeb52 


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


Testing
---


Thanks,

Benno Evers

Re: Review Request 62140: Shorten the default interval between disk-usage GCs.

[Chun-Hung Hsiao]

> On Sept. 7, 2017, 12:05 p.m., Alexander Rukletsov wrote:
> > src/slave/constants.hpp
> > Line 74 (original), 74 (patched)
> > 
> >
> > I know it is hard to justify specific values, but `1s` feels too 
> > aggressive.

>From the scenario described in MESOS-7939, this needs to be less than 10 secs 
>to prevent agent failure, or less than 5s for agent to recover from failure. 
>Also the check itself is pretty cheap, and that's why I'm using such an 
>aggresive value.


- Chun-Hung


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


On Sept. 7, 2017, 4:01 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62140/
> ---
> 
> (Updated Sept. 7, 2017, 4:01 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Jie Yu, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7939
> https://issues.apache.org/jira/browse/MESOS-7939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Make the default value of `disk_watch_interval` to 1 second so GC can be
> kicked in early enough before the agent fails due to a "No space left on
> disk" failure. This change would also trigger the 1st GC earlier during
> agent startup, so it can recover from the above scenario.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 435c37e28987119c4ef43c9dad61ac052020acd8 
> 
> 
> Diff: https://reviews.apache.org/r/62140/diff/2/
> 
> 
> Testing
> ---
> 
> make test
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61183: Triggered 'UpdateSlaveMessage' when 'ResourceProviderManager' updates.

[Benjamin Bannier]

> On Sept. 7, 2017, 1:32 a.m., Jie Yu wrote:
> > src/slave/slave.hpp
> > Lines 658 (patched)
> > 
> >
> > I am not sure if keeping another field just for resoruce provider 
> > provided resources in the agent is a good idea or not.
> > 
> > I'd much prefer we keep a single `totalResources` for regular resources 
> > (both resource provider provided or not), and `oversubscribedResources` 
> > only for oversubsribed resources.

The advantage of using a dedicated member is that with it it is possible to 
detect whether any resources came from resource providers. With that we can 
avoid sending `UpdateSlaveMessage`s e.g., on reregistration when no resource 
providers registered (not sure how to do this without storing at least some 
data).

I'd suggest introducing this member as it captures everything we need.


> On Sept. 7, 2017, 1:32 a.m., Jie Yu wrote:
> > src/slave/slave.cpp
> > Lines 1361 (patched)
> > 
> >
> > is it possible that agent is terminating?

This was a result of a wrong copy-paste.

I removed the `CHECK` here and below as we could just send the message in any 
possible state, like is already done for oversubscribed resources.


> On Sept. 7, 2017, 1:32 a.m., Jie Yu wrote:
> > src/slave/slave.cpp
> > Lines 1370 (patched)
> > 
> >
> > what about checkpointed resources? I think we need to use 
> > `totalResources` (with checkpointed reosurces applied) here.
> > 
> > Also, i checked the master handler for `UpdateSlaveMessage`. Should we 
> > rescind offer too (like the oversubscription case)? Add a TODO there?

Fixed the code to use the correct resources now.

re:master handling of `UpdateSlaveMessage` with `TOTAL`, it makes sense to 
rescind these offers as well. I pushed a minimal implementation, 
https://reviews.apache.org/r/62158/.


- Benjamin


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


On Sept. 7, 2017, 5:13 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61183/
> ---
> 
> (Updated Sept. 7, 2017, 5:13 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The agent's resource provider manager sends a
> 'ResourceProviderMessage' when its managed resources change. This
> commit adds handling in the agent so that an 'UpdateSlaveMessage' is
> sent to the master to update the total resource available on the
> agent. We also store this total in the agent memory so that it can be
> resent on agent resubscription.
> 
> In order to provide push-like handling of the resource provider
> manager's message queue, we chain recursive calls to the handler for
> continuous processing. Initially, processing is kicked off from
> 'Slave::initialize'. In this simple implementation we e.g., provide no
> direct way to stop processing of messages, yet, but it can be achieved
> by e.g., replacing the manager with a new instance (this would also
> require updating routes).
> 
> Since the agent can only send an 'UpdateSlaveMessage' when it is
> registered with a master, a simple back-off of 5 s is implemented which
> will defer processing of a ready message should the agent not yet have
> registered.
> 
> To facilitate logging we add a stringification function for
> 'ResourceProviderMessage's.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
>   src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd 
>   src/tests/slave_tests.cpp 1bdadce4c50cbff958f2be2a4261e130b414acfd 
> 
> 
> Diff: https://reviews.apache.org/r/61183/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 61183: Triggered 'UpdateSlaveMessage' when 'ResourceProviderManager' updates.

[Benjamin Bannier]

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

(Updated Sept. 7, 2017, 5:13 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Addressed Jie's comments.


Repository: mesos


Description
---

The agent's resource provider manager sends a
'ResourceProviderMessage' when its managed resources change. This
commit adds handling in the agent so that an 'UpdateSlaveMessage' is
sent to the master to update the total resource available on the
agent. We also store this total in the agent memory so that it can be
resent on agent resubscription.

In order to provide push-like handling of the resource provider
manager's message queue, we chain recursive calls to the handler for
continuous processing. Initially, processing is kicked off from
'Slave::initialize'. In this simple implementation we e.g., provide no
direct way to stop processing of messages, yet, but it can be achieved
by e.g., replacing the manager with a new instance (this would also
require updating routes).

Since the agent can only send an 'UpdateSlaveMessage' when it is
registered with a master, a simple back-off of 5 s is implemented which
will defer processing of a ready message should the agent not yet have
registered.

To facilitate logging we add a stringification function for
'ResourceProviderMessage's.


Diffs (updated)
-

  src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
  src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
  src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd 
  src/tests/slave_tests.cpp 1bdadce4c50cbff958f2be2a4261e130b414acfd 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 62158: Rescinded offers possibly affected by updates to agent total resources.

[Benjamin Bannier]

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

Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

When an agent changes its resources, the master should rescind any
offers affected by the change. We already performed the rescind for
updates to the agent's oversubscribed resources; this patch adds offer
rescinding when an update an agent's total resources is processed.

While for updates to an agent's oversubscribed resources we currently
only rescind offers containing revocable resources to e.g., reduce
offer churn, we for updates to the total we here currently rescind all
offers for resources on the agent.


Diffs
-

  src/master/master.cpp 53ee87ad8e770a91ecbd6822cd31012811848698 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 62148: Made the `stop()` logic in the scheduler library simpler.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed. Please check 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62148/logs/mesos-tests-stdout.log
 and 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62148/logs/mesos-tests-stderr.log
 for any relevant errors

Reviews applied: [62148]

Logs available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62148/logs

- Mesos Reviewbot Windows


On Sept. 7, 2017, 5:54 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62148/
> ---
> 
> (Updated Sept. 7, 2017, 5:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of relying on `Clock::pause()/settle()` to ensure that no
> callbacks on the scheduler were in flight, the test
> helper can wait on the future returned by `stop()` now. This is
> possible as we acquire the mutex explicitly in `stop()` to ensure
> that there can't be any queued callbacks that get executed after
> `TestMesos` is destroyed.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661 
>   src/scheduler/scheduler.cpp ce69258027ed50867569374d2d827fc3cc651744 
>   src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554 
> 
> 
> Diff: https://reviews.apache.org/r/62148/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Ensured tests still work when run in a loop)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61947: Implemented handling of resource provider offer operations.

[Jan Schlicht]

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

(Updated Sept. 7, 2017, 2:27 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

While the resource provider manager is responsible to apply offer
operations by sending events to the respective resource providers,
the master takes care of accepting these operations. Hence, for local
resource providers the master sends an `ApplyResourceOperationMessage`
to the agent where the resource provider is running on. The agent
then relays the operation contained in the message to the resource
provider manager.


Diffs (updated)
-

  src/master/master.cpp 14d94cfa0e96881f77dd6e0c3449fdc0613e36ce 
  src/messages/messages.proto dc4e19c08d671743f08223dcdf1dbc336b3940bd 
  src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094 
  src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 62140: Shorten the default interval between disk-usage GCs.

[Alexander Rukletsov]

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




src/slave/constants.hpp
Line 74 (original), 74 (patched)


I know it is hard to justify specific values, but `1s` feels too aggressive.


- Alexander Rukletsov


On Sept. 7, 2017, 4:01 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62140/
> ---
> 
> (Updated Sept. 7, 2017, 4:01 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Jie Yu, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7939
> https://issues.apache.org/jira/browse/MESOS-7939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Make the default value of `disk_watch_interval` to 1 second so GC can be
> kicked in early enough before the agent fails due to a "No space left on
> disk" failure. This change would also trigger the 1st GC earlier during
> agent startup, so it can recover from the above scenario.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 435c37e28987119c4ef43c9dad61ac052020acd8 
> 
> 
> Diff: https://reviews.apache.org/r/62140/diff/2/
> 
> 
> Testing
> ---
> 
> make test
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61558: Move duplicate comment closer to implementation.

[Benno Evers]

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

(Updated Sept. 7, 2017, 12:06 p.m.)


Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

Move duplicate comment closer to implementation.


Diffs (updated)
-

  include/mesos/mesos.proto 1c74cbb3e8c97145975c13e1e42cbffd1e84c827 
  include/mesos/v1/mesos.proto 9aef05f34be5d187eb4ec20133af6f1d1030ad44 
  src/master/master.cpp 14d94cfa0e96881f77dd6e0c3449fdc0613e36ce 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 62145: WIP: Implemented Standalone Container API.

[Mesos Reviewbot Windows]

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



ERROR: Failed to apply patch 61805

Reviews applied: [61805, 61806, 60888, 60889, 60890, 60891, 62142, 62143, 
62144, 62145]

Logs available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62145/logs

- Mesos Reviewbot Windows


On Sept. 7, 2017, 6:13 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62145/
> ---
> 
> (Updated Sept. 7, 2017, 6:13 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7492
> https://issues.apache.org/jira/browse/MESOS-7492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Although the Standalone and Nested Container APIs are very similar,
> there is little shared code between them due to differences in their
> inputs, AuthZ, and outputs.
> 
> 
> Diffs
> -
> 
>   src/slave/http.hpp 44a95dec4c9b8bb65d712c5538bbd7afffe2cf7b 
>   src/slave/http.cpp 3ea7829df8c1c35d2fa3a44f19a60b7e261042ce 
> 
> 
> Diff: https://reviews.apache.org/r/62145/diff/1/
> 
> 
> Testing
> ---
> 
> TODO
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 62047: Allowed look up latest executor directory by virtual path.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed. Please check 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62047/logs/mesos-tests-stdout.log
 and 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62047/logs/mesos-tests-stderr.log
 for any relevant errors

Reviews applied: [62040, 62047]

Logs available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62047/logs

- Mesos Reviewbot Windows


On Sept. 1, 2017, 10:54 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62047/
> ---
> 
> (Updated Sept. 1, 2017, 10:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jason Lai.
> 
> 
> Bugs: MESOS-7899
> https://issues.apache.org/jira/browse/MESOS-7899
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added support to attach a virtual path
> `/frameworks/{framework-id}/executors/{executor-id}/latest` to `Files` class
> in agent, which can be used to navigate an executor's latest sandbox without
> needing to know `work_dir` and `slave_id`.
> 
> 
> Diffs
> -
> 
>   src/slave/paths.hpp 51b481fc0870f1e95448f85ee2fd485fceea1919 
>   src/slave/paths.cpp 1a6943f03493079ab291616d3e36e0fd8809cf33 
>   src/slave/slave.cpp df920ec07cd59c7ba6baccfc1c20ed3809f187d6 
> 
> 
> Diff: https://reviews.apache.org/r/62047/diff/1/
> 
> 
> Testing
> ---
> 
> Ran a master/agent pair, launch a task with `mesos-execute` and use 
> `files/browse` endpoint to browse the virtual path.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 61495: Add documentation for possible task reasons.

[Alexander Rukletsov]

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


Fix it, then Ship it!




I'll fix the last issue and commit this for you.


docs/task-state-reasons.md
Lines 20 (patched)


Change "which" back to "that"


- Alexander Rukletsov


On Sept. 6, 2017, 9:29 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61495/
> ---
> 
> (Updated Sept. 6, 2017, 9:29 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, James Peach, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5078
> https://issues.apache.org/jira/browse/MESOS-5078
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for possible task reasons.
> 
> 
> Diffs
> -
> 
>   docs/home.md ad91f2fe23b39d59dc021428899069531fce9970 
>   docs/task-state-reasons.md PRE-CREATION 
>   include/mesos/mesos.proto eede0827fc0da5f2fa3cfb432fde29b95a8c644c 
>   include/mesos/v1/mesos.proto 8c6246ee824f28d93dc5b57d25939d1ba76c986b 
> 
> 
> Diff: https://reviews.apache.org/r/61495/diff/8/
> 
> 
> Testing
> ---
> 
> Built website with site/build.sh and verified it renders ok.
> 
> HTML preview: 
> http://htmlpreview.github.io/?https://github.com/lava/mesos/blob/bennoe/task-reasons/site/publish/documentation/latest/task-state-reasons/index.html
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 62106: Enabled CRAM MD5 Authentication on Windows and associated tests.

[Mesos Reviewbot Windows]

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



FAIL: Mesos tests failed to build. Please check 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62106/logs/mesos-tests-cmake-build.log
 for any relevant errors

Reviews applied: [62105, 62106]

Logs available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62106/logs

- Mesos Reviewbot Windows


On Sept. 7, 2017, 1:11 a.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62106/
> ---
> 
> (Updated Sept. 7, 2017, 1:11 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-3384
> https://issues.apache.org/jira/browse/MESOS-3384
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled CRAM MD5 Authentication on Windows and associated tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 4cb8dddf93694722af2377c4a55a195260c8a0cb 
>   docs/windows.md b47ba5a0db3d41d6f4bb002283b40622a2bb3e0f 
>   src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
>   src/authentication/cram_md5/authenticatee.cpp 
> 7b3f767e29163de489018081089e67a6f22971c5 
>   src/authentication/cram_md5/authenticator.cpp 
> 2a12bb3f4aec79056ec6c82a8c896ee3af15bb4c 
>   src/authentication/cram_md5/auxprop.hpp 
> dedcbe514283dd86a924d92ccdd17173ebe878cb 
>   src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
>   src/tests/authentication_tests.cpp b04be76d23788fbde337eaf85716ec7436ade691 
>   support/windows-build.bat 100013ed68ecc2f431c20a3f081f8aa6eca961ef 
> 
> 
> Diff: https://reviews.apache.org/r/62106/diff/3/
> 
> 
> Testing
> ---
> 
> Ran the automake build and the cmake build on Linux, as well as the cmake 
> build on Windows, with no errors.  Ran tests on both platforms as well.  
> Note: The mesos-3rdparty pull request which contains the cyrus-sasl package 
> needs to be merged to master first before these patches will work. (Or, have 
> the cyrus-sasl tarball available in a local 3rdparty directory, configured 
> with mesos)
> 
> 
> Thanks,
> 
> John Kordich
> 
>