Re: Review Request 69667: Sent SIGKILL to I/O switchboard server as a safeguard.

[Qian Zhang]

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

(Updated Jan. 5, 2019, 9:25 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Sent SIGKILL to I/O switchboard server as a safeguard.


Diffs (updated)
-

  src/slave/containerizer/mesos/io/switchboard.cpp 
c445a8f09d7671d5763281bac9881489b3cc9c39 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 69667: Sent SIGKILL to I/O switchboard server as a safeguard.

[Qian Zhang]

> On Jan. 5, 2019, 12:12 a.m., Vinod Kone wrote:
> > src/slave/containerizer/mesos/io/switchboard.cpp
> > Lines 825 (patched)
> > 
> >
> > Is there anway to test this?

I manually tested it by using gdb to attach the I/O switchboard server process 
and confirmed `SIGKILL` can be sent as expected.


- Qian


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


On Jan. 5, 2019, 9:25 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69667/
> ---
> 
> (Updated Jan. 5, 2019, 9:25 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7042
> https://issues.apache.org/jira/browse/MESOS-7042
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Sent SIGKILL to I/O switchboard server as a safeguard.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> c445a8f09d7671d5763281bac9881489b3cc9c39 
> 
> 
> Diff: https://reviews.apache.org/r/69667/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 69671: Always build gRPC with its embedded c-ares.

[Jie Yu]

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


Fix it, then Ship it!





3rdparty/Makefile.am
Lines 500 (patched)


Can you add some comments about this one? Or just separate this change in a 
different patch?


- Jie Yu


On Jan. 5, 2019, 1:24 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69671/
> ---
> 
> (Updated Jan. 5, 2019, 1:24 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-9505
> https://issues.apache.org/jira/browse/MESOS-9505
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We compile gRPC with its embedded c-ares library to avoid a link error
> because Mesos is not aware of the library.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am a14216cf98f6638da06aa3dfc49e6b319fea7f87 
> 
> 
> Diff: https://reviews.apache.org/r/69671/diff/1/
> 
> 
> Testing
> ---
> 
> make check with c-ares pre-installed on mac and ubuntu.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 69671: Always build gRPC with its embedded c-ares.

[Chun-Hung Hsiao]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

We compile gRPC with its embedded c-ares library to avoid a link error
because Mesos is not aware of the library.


Diffs
-

  3rdparty/Makefile.am a14216cf98f6638da06aa3dfc49e6b319fea7f87 


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


Testing
---

make check with c-ares pre-installed on mac and ubuntu.


Thanks,

Chun-Hung Hsiao

Re: Review Request 69603: Extended `min_allocatable_resources` flag to cover non-scalar resources.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/master/allocator/mesos/hierarchical.cpp
Line 2435 (original), 2435 (patched)


we should already be in mesos::internal here, did you find you needed to 
specify?



src/master/master.cpp
Line 742 (original), 734 (patched)


not yours but looks like this colon should be removed



src/master/master.cpp
Lines 749-750 (original), 741-742 (patched)


probably `resourceQuantities->begin()` / end() is more readable?



src/master/master.cpp
Line 752 (original), 744 (patched)


Maybe we can declare first class resources down here where we use it, to 
make the code more readable? we're not worried about the cost of 
re-constructing it in each loop iteration



src/master/master.cpp
Lines 750 (patched)


Probably just want to use `*resourceQuantities` since we handle the error 
case above.


- Benjamin Mahler


On Jan. 4, 2019, 8:54 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69603/
> ---
> 
> (Updated Jan. 4, 2019, 8:54 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9324
> https://issues.apache.org/jira/browse/MESOS-9324
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master flag min_allocatable_resources currently only supports
> scalar resources. Sometimes it is desirable to control the quantities
> of non-scalar resources in the offer as well, for example, to
> ensure offers always contain port resources.
> 
> This patch extended the flag to cover non-scalar resources.
> For RANGES and SET type resources, their quantities are the
> number of different instances in the range or set. For example,
> range:[1-5] has a quantity of 5 and set:{a,b} has a quantity of 2.
> 
> This patch also updated affected tests and modified the existing
> test for min_allocatable_resources to cover the updated use cases.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master.md 83b83b0b7a41f982c8680cee21f0971a0877e49e 
>   include/mesos/allocator/allocator.hpp 
> 2a6849bf698f171fc31c84165bd574b8f4f846ea 
>   src/master/allocator/mesos/hierarchical.cpp 
> cc8ab919aedb33cf424edfdb622ca13a2cc8ff0f 
>   src/master/flags.cpp f9b68bcd5668160a7e90d6569f69f2713df802fe 
>   src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316 
>   src/tests/hierarchical_allocator_benchmarks.cpp 
> 527a25d1414882012f3906370bd1a2a7b40a92f2 
>   src/tests/hierarchical_allocator_tests.cpp 
> a3a6d7bb956a6d16f01e2c312d69f704fcbe9ddd 
>   src/tests/master_allocator_tests.cpp 
> 9dbab1822ea83ac912a56eb3196060c648a2a672 
> 
> 
> Diff: https://reviews.apache.org/r/69603/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69601: Added a `Resources` method `contains(ResourceQuantities)`.

[Benjamin Mahler]

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


Fix it, then Ship it!




Much easier to read the tests now, thanks!


src/common/resources.cpp
Lines 1518 (patched)


foreach (



src/common/resources.cpp
Lines 1520-1522 (patched)


Maybe:

```
foreach (const Resource& r, get(quantity.first)) {
```



src/tests/resources_tests.cpp
Lines 2070-2074 (patched)


Why not do the same thing as set? 3,2,1 to test the boundaries?



src/tests/resources_tests.cpp
Lines 2112 (patched)


newline above this?



src/tests/resources_tests.cpp
Lines 2128 (patched)


newline above this?



src/v1/resources.cpp
Lines 1536 (patched)


foreach (



src/v1/resources.cpp
Lines 1538-1540 (patched)


Maybe:

```
foreach (const Resource& r, get(quantity.first)) {
```


- Benjamin Mahler


On Jan. 4, 2019, 8:16 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69601/
> ---
> 
> (Updated Jan. 4, 2019, 8:16 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This method checks if the quantities of this `Resources` is a
> superset of the given `ResourceQuantities`. If a `Resource` object
> is `SCALAR` type, its quantity is its scalar value. For `RANGES`
> and `SET` type, their quantities are the number of different
> instances in the range or set. For example, "range:[1-5]" has a
> quantity of 5 and "set:{a,b}" has a quantity of 2.
> 
> Also added a dedicated test.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 36ccf0e1135ce15898d31db51c80fa7b5826907b 
>   include/mesos/v1/resources.hpp 1a9ea44e78d985286e04e040879d022838ddcc3f 
>   src/common/resources.cpp 758b5a2fac101bfb0a07ba76c204b02d1554b3ac 
>   src/tests/resources_tests.cpp 5337a5c9af9431cc1c7822c78945ffc369488900 
>   src/v1/resources.cpp d717f5327a45cfe9bc075bc3acf37316223d389d 
> 
> 
> Diff: https://reviews.apache.org/r/69601/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69600: Added tests for `ResourceQuantities`.

[Benjamin Mahler]

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



Looks good! Mainly I think nan, inf, and so on will be parsed, so we'll need to 
test that and prevent it.


src/tests/resource_quantities_tests.cpp
Lines 41 (patched)


How about FromStringValid?



src/tests/resource_quantities_tests.cpp
Lines 56-61 (patched)


s/whitespaces/whitespace

This will need to be updated, probably you just want to show triming, e.g.:

```
"cpus : 1 ; mem : 1024 ; disk : 1024"
```



src/tests/resource_quantities_tests.cpp
Lines 63-72 (patched)


Seems a little odd to test that parsing 0 is retained via the two resources 
case.



src/tests/resource_quantities_tests.cpp
Lines 98 (patched)


How about FromStringInvalid?



src/tests/resource_quantities_tests.cpp
Lines 100 (patched)


More like "Invalid scalar"?



src/tests/resource_quantities_tests.cpp
Lines 101 (patched)


No newline here to be consistent with the other blocks below?



src/tests/resource_quantities_tests.cpp
Lines 113-115 (patched)


Can you also add a section to test that "nan", "-nan", "inf", "-inf", 
"infinity", "-infinity" are disallowed?

I think they will go through, so we probably have to update values::parse


- Benjamin Mahler


On Jan. 4, 2019, 8:14 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69600/
> ---
> 
> (Updated Jan. 4, 2019, 8:14 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added parsing and insertion tests for `ResourceQuantities`.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa 
>   src/tests/CMakeLists.txt c588183e9d2b1cc733fdf3df70f37d47a5fdd7c0 
>   src/tests/resource_quantities_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69600/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69599: Pulled up a new class `ResourceQuantities`.

[Benjamin Mahler]

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



Summary: "Pulled up a new class `ResourceQuantities`." -> "Pulled up the 
resource quantities class for more general use."

It's looking pretty good, just left some comments around parsing consistency, 
comments, and some minor things.


src/Makefile.am
Lines 1022-1023 (patched)


Tabs are not aligned?



src/common/resource_quantities.hpp
Lines 36-47 (patched)


Now that it's a map, I think this can be as simple as:

```
We provide map-like semantics: [] operator for inserting/retrieving values,
keys are unique, and iteration is sorted.
```

Along with the suggested paragraph below, that seems sufficient?



src/common/resource_quantities.hpp
Lines 38 (patched)


The finite >=0 part isn't true anymore right?



src/common/resource_quantities.hpp
Lines 43 (patched)


"native" isn't clear here



src/common/resource_quantities.hpp
Lines 44-46 (patched)


It looks like you can remove the note about inserting a new key if not 
present, that's consistent with map



src/common/resource_quantities.hpp
Lines 46-47 (patched)


mindful of producing negatives when mutating...



src/common/resource_quantities.hpp
Lines 49-54 (patched)


"native" isn't clear here

How about:

```
// An alternative design for this class was to provide addition and
// subtraction functions as opposed to a map interface. However, this
// leads to some un-obvious semantics around presence of zero values
// (will zero entries be automatically removed?) and handling of negatives
// (will negatives trigger a crash? will they be converted to zero? Note
// that Value::Scalar should be non-negative but there is currently no
// enforcement of this by Value::Scalar arithmetic operations; we probably
// want all Value::Scalar operations to go through the same negative
// handling). To avoid the confusion, we provided a map like interface
// which produces clear semantics: insertion works like maps, and the
// user is responsible for performing arithmetic operations on the values
// (using the already provided Value::Scalar arithmetic overloads).
```



src/common/resource_quantities.hpp
Lines 72-75 (patched)


Hm.. let's just make it consistent with Resources::fromSimpleString? I 
guess we can't just call it since we want to be more strict and validating 
post-call is messy.

But let's make the logic nearly the same and comment in the implementation 
of the function that it's what we based it off of?



src/common/resource_quantities.hpp
Lines 79-80 (patched)


otherwise an error is returned



src/common/resource_quantities.hpp
Lines 83-88 (patched)


Put this in the .cpp?



src/common/resource_quantities.hpp
Lines 96-117 (patched)


This is probably more obvious this way?

```
  typedef std::vector>::const_iterator
iterator;
  typedef std::vector>::const_iterator
const_iterator;

  // NOTE: Non-`const` `begin()` and `end()` are __intentionally__
  // defined with `const` semantics in order to prevent direct mutable 
access.
  const_iterator begin();
  const_iterator end();
```

But this comment isn't quite appropriate here, we only need the string name 
to be non-mutable now that it's a map interface. Can you update the comment 
accordingly? I'm guessing it's hard to actually provide a const string, 
non-const value iterator, so the comment is fine.



src/common/resource_quantities.hpp
Lines 103-117 (patched)


These are a bit verbose for the header, put them in the .cpp?



src/common/resource_quantities.cpp
Lines 39-69 (patched)


Let's make it more consistent with the Resources::fromSimpleString code and 
write down in a comment that it's what this is based on?



src/common/resource_quantities.cpp
Lines 51 (patched)


Let's re-use values::parse here to be consistent with how the resource 
parsing works and so that all value scalars are parsed the same way.



src/master/allocator/sorter/dr

Re: Review Request 69603: Extended `min_allocatable_resources` flag to cover non-scalar resources.

[Meng Zhu]

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

(Updated Jan. 4, 2019, 12:54 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description (updated)
---

The master flag min_allocatable_resources currently only supports
scalar resources. Sometimes it is desirable to control the quantities
of non-scalar resources in the offer as well, for example, to
ensure offers always contain port resources.

This patch extended the flag to cover non-scalar resources.
For RANGES and SET type resources, their quantities are the
number of different instances in the range or set. For example,
range:[1-5] has a quantity of 5 and set:{a,b} has a quantity of 2.

This patch also updated affected tests and modified the existing
test for min_allocatable_resources to cover the updated use cases.


Diffs (updated)
-

  docs/configuration/master.md 83b83b0b7a41f982c8680cee21f0971a0877e49e 
  include/mesos/allocator/allocator.hpp 
2a6849bf698f171fc31c84165bd574b8f4f846ea 
  src/master/allocator/mesos/hierarchical.cpp 
cc8ab919aedb33cf424edfdb622ca13a2cc8ff0f 
  src/master/flags.cpp f9b68bcd5668160a7e90d6569f69f2713df802fe 
  src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316 
  src/tests/hierarchical_allocator_benchmarks.cpp 
527a25d1414882012f3906370bd1a2a7b40a92f2 
  src/tests/hierarchical_allocator_tests.cpp 
a3a6d7bb956a6d16f01e2c312d69f704fcbe9ddd 
  src/tests/master_allocator_tests.cpp 9dbab1822ea83ac912a56eb3196060c648a2a672 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 69603: Extended `min_allocatable_resources` flag to cover non-scalar resources.

[Meng Zhu]

> On Jan. 3, 2019, 10:06 a.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2434-2440 (original), 2434-2440 (patched)
> > 
> >
> > Is there a way (e.g. benchmark) to check that we didn't make this 
> > contains slower?

Unlikely (previously, it involved copying the whole resource object for 
subtraction), we now only copy a double. But I can run the allocator 
performance benchmark later to confirm.


> On Jan. 3, 2019, 10:06 a.m., Benjamin Mahler wrote:
> > src/master/master.cpp
> > Lines 753-754 (original), 745-746 (patched)
> > 
> >
> > We call these "first class" rather than "common", but why do you want 
> > this logic?
> > 
> > Maybe you intended to warn about potentially "scarce" (i.e. 
> > "non-ubiquitous") resources being used in the flag? Like "gpus"? Or 
> > "someCustomResourceOnlyOnSomeMachines"?

I was mainly thinking of typo, imagine the flag is set to cpu:... (instead of 
"cpus"), then no offer will be made and it could take quite some time to debug.


> On Jan. 3, 2019, 10:06 a.m., Benjamin Mahler wrote:
> > src/tests/master_allocator_tests.cpp
> > Lines 1287-1290 (original), 1288-1291 (patched)
> > 
> >
> > Maybe you can line up these contains comments to make it a bit more 
> > readable?

Done.


- Meng


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


On Jan. 4, 2019, 12:54 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69603/
> ---
> 
> (Updated Jan. 4, 2019, 12:54 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9324
> https://issues.apache.org/jira/browse/MESOS-9324
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master flag min_allocatable_resources currently only supports
> scalar resources. Sometimes it is desirable to control the quantities
> of non-scalar resources in the offer as well, for example, to
> ensure offers always contain port resources.
> 
> This patch extended the flag to cover non-scalar resources.
> For RANGES and SET type resources, their quantities are the
> number of different instances in the range or set. For example,
> range:[1-5] has a quantity of 5 and set:{a,b} has a quantity of 2.
> 
> This patch also updated affected tests and modified the existing
> test for min_allocatable_resources to cover the updated use cases.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master.md 83b83b0b7a41f982c8680cee21f0971a0877e49e 
>   include/mesos/allocator/allocator.hpp 
> 2a6849bf698f171fc31c84165bd574b8f4f846ea 
>   src/master/allocator/mesos/hierarchical.cpp 
> cc8ab919aedb33cf424edfdb622ca13a2cc8ff0f 
>   src/master/flags.cpp f9b68bcd5668160a7e90d6569f69f2713df802fe 
>   src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316 
>   src/tests/hierarchical_allocator_benchmarks.cpp 
> 527a25d1414882012f3906370bd1a2a7b40a92f2 
>   src/tests/hierarchical_allocator_tests.cpp 
> a3a6d7bb956a6d16f01e2c312d69f704fcbe9ddd 
>   src/tests/master_allocator_tests.cpp 
> 9dbab1822ea83ac912a56eb3196060c648a2a672 
> 
> 
> Diff: https://reviews.apache.org/r/69603/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69601: Added a `Resources` method `contains(ResourceQuantities)`.

[Meng Zhu]

> On Jan. 3, 2019, 9:14 a.m., Benjamin Mahler wrote:
> > include/mesos/resources.hpp
> > Lines 45 (patched)
> > 
> >
> > This is a public header and I don't think the internal headers get 
> > installed so this would break any use of the public headers?
> > 
> > Can you forward declare instead?

Done.


> On Jan. 3, 2019, 9:14 a.m., Benjamin Mahler wrote:
> > src/common/resources.cpp
> > Lines 1517 (patched)
> > 
> >
> > Do we need the iterator? Can we use a foreach auto loop here?

Done.


> On Jan. 3, 2019, 9:14 a.m., Benjamin Mahler wrote:
> > src/common/resources.cpp
> > Lines 1523-1525 (patched)
> > 
> >
> > Maybe we can make this a bit more tidy?
> > 
> > ```
> > case Value::SCALAR: remaining -= r.scalar().value();  break;
> > case Value::SET:remaining -= r.set().item_size(); break;
> > case ...
> > ```

Done.


> On Jan. 3, 2019, 9:14 a.m., Benjamin Mahler wrote:
> > src/common/resources.cpp
> > Lines 1529 (patched)
> > 
> >
> > Why the comment here but not below on the same optimization?
> > 
> > It seems fine without the comment in both places to me

Done.


> On Jan. 3, 2019, 9:14 a.m., Benjamin Mahler wrote:
> > src/tests/resources_tests.cpp
> > Lines 2054 (patched)
> > 
> >
> > Maybe we can make the tests more readable with some better variable 
> > names or lambdas that allow us to see the entire comparision on one line?
> > 
> > E.g.
> > 
> > ```
> > EXPECT_TRUE(Resources().contains(ResourceQuantities());
> > 
> > EXPECT_FALSE(nonEmptyResources.contains(emptyQuantities);
> > 
> > auto resources = [](const string& s) { ... };
> > auto quantities = [](const string& s) { ... };
> > 
> > // Can easily read what's being checked:
> > EXPECT_TRUE(resources("cpus:1;mem:2")
> >   .contains(quantities("cpus:0.1;mem:1"));
> > ```

Done, much cleaner, thanks!


- Meng


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


On Jan. 4, 2019, 12:16 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69601/
> ---
> 
> (Updated Jan. 4, 2019, 12:16 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This method checks if the quantities of this `Resources` is a
> superset of the given `ResourceQuantities`. If a `Resource` object
> is `SCALAR` type, its quantity is its scalar value. For `RANGES`
> and `SET` type, their quantities are the number of different
> instances in the range or set. For example, "range:[1-5]" has a
> quantity of 5 and "set:{a,b}" has a quantity of 2.
> 
> Also added a dedicated test.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 36ccf0e1135ce15898d31db51c80fa7b5826907b 
>   include/mesos/v1/resources.hpp 1a9ea44e78d985286e04e040879d022838ddcc3f 
>   src/common/resources.cpp 758b5a2fac101bfb0a07ba76c204b02d1554b3ac 
>   src/tests/resources_tests.cpp 5337a5c9af9431cc1c7822c78945ffc369488900 
>   src/v1/resources.cpp d717f5327a45cfe9bc075bc3acf37316223d389d 
> 
> 
> Diff: https://reviews.apache.org/r/69601/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69601: Added a `Resources` method `contains(ResourceQuantities)`.

[Meng Zhu]

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

(Updated Jan. 4, 2019, 12:16 p.m.)


Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

This method checks if the quantities of this `Resources` is a
superset of the given `ResourceQuantities`. If a `Resource` object
is `SCALAR` type, its quantity is its scalar value. For `RANGES`
and `SET` type, their quantities are the number of different
instances in the range or set. For example, "range:[1-5]" has a
quantity of 5 and "set:{a,b}" has a quantity of 2.

Also added a dedicated test.


Diffs (updated)
-

  include/mesos/resources.hpp 36ccf0e1135ce15898d31db51c80fa7b5826907b 
  include/mesos/v1/resources.hpp 1a9ea44e78d985286e04e040879d022838ddcc3f 
  src/common/resources.cpp 758b5a2fac101bfb0a07ba76c204b02d1554b3ac 
  src/tests/resources_tests.cpp 5337a5c9af9431cc1c7822c78945ffc369488900 
  src/v1/resources.cpp d717f5327a45cfe9bc075bc3acf37316223d389d 


Diff: https://reviews.apache.org/r/69601/diff/3/

Changes: https://reviews.apache.org/r/69601/diff/2-3/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 69600: Added tests for `ResourceQuantities`.

[Meng Zhu]

> On Jan. 3, 2019, 9:48 a.m., Benjamin Mahler wrote:
> > Stale summary?
> > 
> > Partial review since it looks like some of what's being tested will change 
> > based on updates to the first patch.

Ah, sorry, summary updated.


> On Jan. 3, 2019, 9:48 a.m., Benjamin Mahler wrote:
> > src/Makefile.am
> > Lines 2541 (patched)
> > 
> >
> > Seems we should name this file and the header `resource_quantities.hpp` 
> > and `resource_quantities_tests.cpp`?

Done.


> On Jan. 3, 2019, 9:48 a.m., Benjamin Mahler wrote:
> > src/tests/quantities_tests.cpp
> > Lines 27 (patched)
> > 
> >
> > Seems odd to have a namespace for this, just having 
> > mesos::internal::ResourceQuantities live one level beneath mesos::Resources 
> > seems more appropriate?
> > 
> > Also, I think we try to avoid `using` entire namespaces going forward.

Done.


> On Jan. 3, 2019, 9:48 a.m., Benjamin Mahler wrote:
> > src/tests/quantities_tests.cpp
> > Lines 158 (patched)
> > 
> >
> > Hm.. holding a vector iterator while insertions might happen seems 
> > error prone and we'd like to avoid tests crashing when they fail.
> > 
> > Is there a safer way to check what you want to check? E.g. just call 
> > `begin()` or `get("cpus")` each time.

Done.


> On Jan. 3, 2019, 9:48 a.m., Benjamin Mahler wrote:
> > src/tests/quantities_tests.cpp
> > Lines 162-163 (patched)
> > 
> >
> > Maybe this can be more readable with a scalar producing lambda?
> > 
> > ```
> > auto scalar = [](double d) { ... };
> > 
> > quantities.subtract("cpus", scalar(4));
> > ```
> > 
> > Also, is it possible to check the storage more succinctly and readably?
> > 
> > ```
> >   EXPECT_EQ({{"cpus", 6.0}}, toVector(quantities));
> > ```
> > 
> > toVector would produce a `vector>` via the 
> > iterators.

Yeah, that would be much readable. But we do not have an implicit constructor 
for value::scalar from numbers.


- Meng


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


On Jan. 4, 2019, 12:14 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69600/
> ---
> 
> (Updated Jan. 4, 2019, 12:14 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added parsing and insertion tests for `ResourceQuantities`.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa 
>   src/tests/CMakeLists.txt c588183e9d2b1cc733fdf3df70f37d47a5fdd7c0 
>   src/tests/resource_quantities_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69600/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69600: Added tests for `ResourceQuantities`.

[Meng Zhu]

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

(Updated Jan. 4, 2019, 12:14 p.m.)


Review request for mesos and Benjamin Mahler.


Summary (updated)
-

Added tests for `ResourceQuantities`.


Repository: mesos


Description (updated)
---

Added parsing and insertion tests for `ResourceQuantities`.


Diffs (updated)
-

  src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa 
  src/tests/CMakeLists.txt c588183e9d2b1cc733fdf3df70f37d47a5fdd7c0 
  src/tests/resource_quantities_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/69600/diff/3/

Changes: https://reviews.apache.org/r/69600/diff/2-3/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 68018: Added `SeccompFilter` class.

[Andrei Budnik]

> On Jan. 3, 2019, 1:58 a.m., Gilbert Song wrote:
> > src/linux/seccomp/seccomp.cpp
> > Lines 138 (patched)
> > 
> >
> > Do I understand correctly that this would not impact on the 
> > setuid/setgid after pivot_root in mesos/launch.cpp?
> > 
> > The side effect is on the task: the task cannot setuid/setgid and 
> > cannot change capabilities?

> Do I understand correctly that this would not impact on the setuid/setgid 
> after pivot_root in mesos/launch.cpp?

Yes, correct.

>From https://www.kernel.org/doc/Documentation/prctl/no_new_privs.txt:
```
Note that no_new_privs does not prevent privilege changes that do not
involve execve.  An appropriately privileged task can still call
setuid(2) and receive SCM_RIGHTS datagrams.
```

AFAIU, `NO_NEW_PRIVS` prevents changing current setuid/setgid/capabilities when 
calling `execve` with these bits set on executable.


> On Jan. 3, 2019, 1:58 a.m., Gilbert Song wrote:
> > src/linux/seccomp/seccomp.cpp
> > Lines 141-144 (patched)
> > 
> >
> > Instead of always set `SCMP_FLTATR_CTL_NNP`. Should we consider to 
> > check root privileges first (e.g., `geteuid() != 0`)?

By default, libseccomp sets `true` to the `SCMP_FLTATR_CTL_NNP` flag
https://github.com/seccomp/libseccomp/blob/1e64feb5f1a9ea02687228e3073e8b784a04ce46/src/db.c#L960

Hence, all Seccomp test pass even after removing `seccomp_attr_set(ctx, 
SCMP_FLTATR_CTL_NNP, 1)`. Also, this means that Docker daemon launches its 
containers with this flag set by default (as they also use libseccomp).

Disabling `SCMP_FLTATR_CTL_NNP` flag for a `root` means that Seccomp filter can 
be reverted anytime. So, disabling this flag is meaningless.


- Andrei


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


On Nov. 8, 2018, 3:24 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68018/
> ---
> 
> (Updated Nov. 8, 2018, 3:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9034
> https://issues.apache.org/jira/browse/MESOS-9034
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `SeccompFilter` class is a wrapper for `libseccomp` API. Its main
> purpose is to provide a translation of the `ContainerSeccompProfile`
> message into calls of `libseccomp` API.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8 
>   src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa 
>   src/linux/seccomp/seccomp.hpp PRE-CREATION 
>   src/linux/seccomp/seccomp.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68018/diff/14/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 69599: Pulled up a new class `ResourceQuantities`.

[Meng Zhu]

> On Jan. 2, 2019, 4:03 p.m., Benjamin Mahler wrote:
> > In the commit description can you add context about the other class we 
> > already introduced, along with why we are adding this sister class with a 
> > non-map like interface? (Looks like it's because we want the extra 
> > Value::Scalar validation that isn't built in to Value::Scalar arithmetic 
> > operators, so we have to take Value::Scalar in the interface rather than 
> > exposing them for the caller to manipulate?)

As discussed offline, let's preserve the map interface and thus it is very 
similar to `ScalarResourceQuantities`, so I will just "pull it up".


> On Jan. 2, 2019, 4:03 p.m., Benjamin Mahler wrote:
> > src/common/quantities.hpp
> > Lines 33-38 (patched)
> > 
> >
> > Can you also add some context about the class we already added here and 
> > why we duplicated it? Along with the plan to remove it in favor of this 
> > interface? This can all be in a TODO.

As mentioned above, this patch will just pull that class up.


> On Jan. 2, 2019, 4:03 p.m., Benjamin Mahler wrote:
> > src/common/quantities.hpp
> > Lines 47 (patched)
> > 
> >
> > Should this be a TODO to consider supporting it? Seems like it might be 
> > useful to support later?

Done.


> On Jan. 2, 2019, 4:03 p.m., Benjamin Mahler wrote:
> > src/common/quantities.hpp
> > Lines 53 (patched)
> > 
> >
> > Let's add the optimization:
> > 
> > ```
> > // Pre-reserve space for first-class scalars.
> > quantities.reserve(5u);  // [cpus, disk, gpus, mem, ports]
> > ```

Done.


> On Jan. 2, 2019, 4:03 p.m., Benjamin Mahler wrote:
> > src/common/quantities.hpp
> > Lines 89-91 (patched)
> > 
> >
> > This seems more consistent?
> > 
> > ```
> > Option get(const std::string& name) const;
> > ```
> > 
> > We're "getting" the entry for the name?

Done.


> On Jan. 2, 2019, 4:03 p.m., Benjamin Mahler wrote:
> > src/common/quantities.hpp
> > Lines 93-96 (patched)
> > 
> >
> > Shouldn't we CHECK that the incoming scalars here are non-negative and 
> > `isfinite()` rather than silently ignoring those cases?
> > 
> > Negative scalars as the input here are not ignored by the 
> > implementation?

As discussed offline, we will provide map interface and allow direct mutation 
in favor of providing arithmetic methods.


> On Jan. 2, 2019, 4:03 p.m., Benjamin Mahler wrote:
> > src/common/quantities.hpp
> > Lines 95 (patched)
> > 
> >
> > This is a bit confusing, it seems like the reader should be reading / 
> > understanding the model from the top of the class rather than here. (FWIW, 
> > add also has the same implications on the model, if there's no entry, and 
> > you `add()` a zero, will there be a zero stored? what if you `add()` a 
> > negative?

See above.


> On Jan. 2, 2019, 4:03 p.m., Benjamin Mahler wrote:
> > src/common/quantities.hpp
> > Lines 101 (patched)
> > 
> >
> > just call it `quantities` to make the code cleaner?

Done.


> On Jan. 2, 2019, 4:03 p.m., Benjamin Mahler wrote:
> > src/common/quantities.cpp
> > Lines 46 (patched)
> > 
> >
> > let's quote the token
> > 
> > also, use colon for reason:
> > 
> > ```
> > ... quantities: missing ...
> > ```

Done.


> On Jan. 2, 2019, 4:03 p.m., Benjamin Mahler wrote:
> > src/common/quantities.cpp
> > Lines 55 (patched)
> > 
> >
> > Might be a bit cleaner:
> > ```
> > if (*num < 0) {
> > ```
> > Ditto below

We do not have `*` operator for `Try`?


> On Jan. 2, 2019, 4:03 p.m., Benjamin Mahler wrote:
> > src/common/quantities.cpp
> > Lines 75-83 (patched)
> > 
> >
> > Why does this need iterators? Can't this just be a foreach with a break?
> > 
> > ```
> > foreach (auto& quantity, quantities) {
> >   if (quantity.first == name) {
> > return quantity.second;
> >   } else if (it->first > name) {
> > break;
> >   }
> > }
> > 
> > return None();
> > ```

Done.


- Meng


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

Re: Review Request 69599: Pulled up a new class `ResourceQuantities`.

[Meng Zhu]

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

(Updated Jan. 4, 2019, 12:08 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Addressed Ben's comment.


Summary (updated)
-

Pulled up a new class `ResourceQuantities`.


Repository: mesos


Description (updated)
---

There are many places that we need to express the concept of
resource quantities such as enforcing allocation quantities
in the allocator and set guaranteed resource quantities with quota.
However, the current code usually uses the complex Resources
class which is unnecessary and inefficient.

This patch pulls class ScalarResourceQuantities in sorter.hpp
up, aiming to use it for all resource quantity related usages.
We mostly preserve the map interface and added other utilities such
as parsing.


Diffs (updated)
-

  src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8 
  src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa 
  src/common/resource_quantities.hpp PRE-CREATION 
  src/common/resource_quantities.cpp PRE-CREATION 
  src/master/allocator/sorter/drf/sorter.hpp 
084df82baef91eca5775b0bd17d943f3fb8df70b 
  src/master/allocator/sorter/drf/sorter.cpp 
a648fac7e922ab0aefdf9363624d7242f1fc6588 
  src/master/allocator/sorter/random/sorter.hpp 
800b22c67126c2b14c5259d9d122d2e196cc80d8 
  src/master/allocator/sorter/sorter.hpp 
68cf6988ef1a156cf16951e3164261234e9abeeb 


Diff: https://reviews.apache.org/r/69599/diff/3/

Changes: https://reviews.apache.org/r/69599/diff/2-3/


Testing
---

make check
Dedicated test in r/69600


Thanks,

Meng Zhu

Review Request 69669: Notified frameworks when operations are marked as unreachable.

[Benno Evers]

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

Review request for mesos, Benjamin Bannier, Gastón Kleiman, Greg Mann, and 
Joseph Wu.


Repository: mesos


Description
---

When an agent is being marked as unreachable due to missing
the reregistration timeout, all operations on that agent
are implicilty transitioned to status `OPERATION_UNREACHABLE`.

This commit adds an explicit notification for this transition
to frameworks which opted-in to operation feedback.


Diffs
-

  src/master/master.hpp 99549ab857b16d722f0dd991f98dbe54e9ed19a1 
  src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316 
  src/tests/api_tests.cpp b6064cd749e42e45c2b471c71e9769a41b59f726 


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


Testing
---


Thanks,

Benno Evers

Re: Review Request 69667: Sent SIGKILL to I/O switchboard server as a safeguard.

[Vinod Kone]

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




src/slave/containerizer/mesos/io/switchboard.cpp
Lines 817 (patched)


Add a comment for posterity.

// If we are here, something really bad must have happened for I/O 
switchboard server
// to not exit after SIGTERM has been sent. We have seen this happen due to 
FD leak (See: MESOS-9502).
// We do a SIGKILL here as a safeguard, so that switchboard server 
forcefully exits and causes this
// cleanup feature to be completed, thus unblocking the container's cleanup.


- Vinod Kone


On Jan. 4, 2019, 8:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69667/
> ---
> 
> (Updated Jan. 4, 2019, 8:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7042
> https://issues.apache.org/jira/browse/MESOS-7042
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Sent SIGKILL to I/O switchboard server as a safeguard.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> c445a8f09d7671d5763281bac9881489b3cc9c39 
> 
> 
> Diff: https://reviews.apache.org/r/69667/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 69667: Sent SIGKILL to I/O switchboard server as a safeguard.

[Vinod Kone]

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




src/slave/containerizer/mesos/io/switchboard.cpp
Lines 818 (patched)


I would put a much higher timeout, say 60s, to give enough time for sigterm 
handler (draining stdout/stderr) and also to make it more visible in the logs.

also, log this at "ERROR" level since this is not expected to happen unless 
there is a bug?



src/slave/containerizer/mesos/io/switchboard.cpp
Lines 825 (patched)


Is there anway to test this?


- Vinod Kone


On Jan. 4, 2019, 8:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69667/
> ---
> 
> (Updated Jan. 4, 2019, 8:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7042
> https://issues.apache.org/jira/browse/MESOS-7042
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Sent SIGKILL to I/O switchboard server as a safeguard.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> c445a8f09d7671d5763281bac9881489b3cc9c39 
> 
> 
> Diff: https://reviews.apache.org/r/69667/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68022: Enabled Seccomp filter in the containerizer launcher.

[Andrei Budnik]

> On Dec. 27, 2018, 9:16 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 1196-1197 (patched)
> > 
> >
> > Hmm, this seems unfortunate, will it cause container cannot be launched?

Since containerizer launcher is not multithreaded, there is no chance that the 
malloc's global mutex is acquired by another thread at the moment when the main 
thread calls `fork()`. So, it's safe to call `malloc` after forking a child 
process.

Currently, the only way to load a Seccomp filter via `libseccomp` is to call 
`seccomp_load`. When libseccomp developers add a new API call, we should use 
the new one instead of `seccomp_load`.


- Andrei


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


On Aug. 6, 2018, 1:39 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68022/
> ---
> 
> (Updated Aug. 6, 2018, 1:39 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9106
> https://issues.apache.org/jira/browse/MESOS-9106
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Containerizer launcher creates an instance of `SeccompFilter`, which is
> used to setup Seccomp profile using `ContainerSeccompProfile` message
> prepared by the `linux/seccomp` isolator. The Seccomp filter is loaded
> right before calling `execve()`, so that a container will be running
> with a syscall filtering enabled.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 2f1c9e7a8748c9d7eab25bc8567ca68308e680f9 
> 
> 
> Diff: https://reviews.apache.org/r/68022/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 69667: Sent SIGKILL to I/O switchboard server as a safeguard.

[Andrei Budnik]

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



I would recommend to fix the real issue instead. See MESOS-6632.

- Andrei Budnik


On Jan. 4, 2019, 8:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69667/
> ---
> 
> (Updated Jan. 4, 2019, 8:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7042
> https://issues.apache.org/jira/browse/MESOS-7042
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Sent SIGKILL to I/O switchboard server as a safeguard.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> c445a8f09d7671d5763281bac9881489b3cc9c39 
> 
> 
> Diff: https://reviews.apache.org/r/69667/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 69662: Displayed resource provider information in the Mesos webui.

[Benjamin Bannier]

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

(Updated Jan. 4, 2019, 11:14 a.m.)


Review request for mesos, Armand Grillet, Benjamin Mahler, and Chun-Hung Hsiao.


Changes
---

Zero-initialized all common summarized resource kinds.


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


Repository: mesos


Description
---

Displayed resource provider information in the Mesos webui.


Diffs (updated)
-

  src/webui/app/agents/agent.html a101a93dcdb95f257fe0ee967c92d2cdc1c84f84 
  src/webui/app/controllers.js 8049cf611895edea7c54b3c58d71e00d823a1fd3 


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

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


Testing
---

`make check`

Ran a local test with a `./src/test-csi-plugin`.


File Attachments (updated)


Screenshot Agent screen
  
https://reviews.apache.org/media/uploaded/files/2019/01/04/ed920e7b-4072-49be-8801-3b875d529fad__Screen_Shot_2019-01-04_at_11.11.51_AM.png


Thanks,

Benjamin Bannier

Re: Review Request 69667: Sent SIGKILL to I/O switchboard server as a safeguard.

[Qian Zhang]

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

(Updated Jan. 4, 2019, 4:03 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

Sent SIGKILL to I/O switchboard server as a safeguard.


Diffs
-

  src/slave/containerizer/mesos/io/switchboard.cpp 
c445a8f09d7671d5763281bac9881489b3cc9c39 


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


Testing
---


Thanks,

Qian Zhang

Review Request 69667: Sent SIGKILL to I/O switchboard server as a safeguard.

[Qian Zhang]

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

Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Repository: mesos


Description
---

Sent SIGKILL to I/O switchboard server as a safeguard.


Diffs
-

  src/slave/containerizer/mesos/io/switchboard.cpp 
c445a8f09d7671d5763281bac9881489b3cc9c39 


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


Testing
---


Thanks,

Qian Zhang