Re: Review Request 60407: Avoided master crash on agent re-registration.

[Adam B]

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




src/master/validation.cpp
Line 370 (original), 370-372 (patched)


Even if we can't `validateDynamicReservationInfo` (or  
`validateDiskInfo`?), would it be worthwhile to `validateGpus`? Maybe 
clone/parameterize `resource::validate` to validate what we can?


- Adam B


On June 23, 2017, 6:48 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60407/
> ---
> 
> (Updated June 23, 2017, 6:48 p.m.)
> 
> 
> Review request for mesos, Adam B and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When validating the agent's ReregisterSlaveMessage, the master's
> validation code neglected to account for the fact that the task
> resources might not be in post-refinement format (e.g., if the agent
> does not support reservation refinement). This lead to a `CHECK` failure
> during validation.
> 
> Fix this by relaxing the validation of ReregisterSlaveMessage so that we
> do not depend on the task resources being in post-refinement
> format. This means validation of ReregisterSlaveMessage will be less
> effective, but since it is best-effort anyway, this seems tolerable.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 33e9ff7db9e2789cbb2d6dfd015288dfa1faa7c5 
> 
> 
> Diff: https://reviews.apache.org/r/60407/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 60406: Added sanity check to resource downgrade on agent registration.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On June 23, 2017, 6:48 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60406/
> ---
> 
> (Updated June 23, 2017, 6:48 p.m.)
> 
> 
> Review request for mesos, Adam B and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `SlaveInfo.resources` should always be representable in the
> "pre-reservation-refinement" format, so `downgradeResources` should
> always succeed.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 
> 
> 
> Diff: https://reviews.apache.org/r/60406/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 60405: Documented resource format in agent <-> master protocol.

[Adam B]

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



Just a couple of clarifying questions before we commit these comments.


src/messages/messages.proto
Line 436 (original), 439-442 (patched)


Since this is a `repeated Resource` list, is it possible that some 
resources in the list are pre-format and some are post? Or will they all be 
post if any resource has a refinement?



src/slave/slave.cpp
Lines 1405-1407 (original), 1405-1408 (patched)


If this agent has refinements, and we send post format to an old master, 
will the old master safely reject the registration, crash and burn, or 
something in between?



src/slave/slave.cpp
Line 1408 (original), 1412-1414 (patched)


We could at least log an INFO/WARN if we aren't able to downgrade, and 
still send it anyway.


- Adam B


On June 23, 2017, 6:48 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60405/
> ---
> 
> (Updated June 23, 2017, 6:48 p.m.)
> 
> 
> Review request for mesos, Adam B and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented resource format in agent <-> master protocol.
> 
> 
> Diffs
> -
> 
>   src/messages/messages.proto 2c086263fdcee4d54a76a61379c2d4dba5271d23 
>   src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 
> 
> 
> Diff: https://reviews.apache.org/r/60405/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 60404: Documented the content of the `SlaveInfo.resources` field.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On June 23, 2017, 6:49 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60404/
> ---
> 
> (Updated June 23, 2017, 6:49 p.m.)
> 
> 
> Review request for mesos, Adam B and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented the content of the `SlaveInfo.resources` field.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 38449da44a5288afc7e2ef17ef5e1380ecabd5d2 
>   include/mesos/v1/mesos.proto 81eca47ce1074f5e522a0410f01ab2b2cfc76aa2 
> 
> 
> Diff: https://reviews.apache.org/r/60404/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 60407: Avoided master crash on agent re-registration.

[Neil Conway]

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

(Updated June 24, 2017, 1:47 a.m.)


Review request for mesos and Michael Park.


Changes
---

Tweak comment.


Repository: mesos


Description
---

When validating the agent's ReregisterSlaveMessage, the master's
validation code neglected to account for the fact that the task
resources might not be in post-refinement format (e.g., if the agent
does not support reservation refinement). This lead to a `CHECK` failure
during validation.

Fix this by relaxing the validation of ReregisterSlaveMessage so that we
do not depend on the task resources being in post-refinement
format. This means validation of ReregisterSlaveMessage will be less
effective, but since it is best-effort anyway, this seems tolerable.


Diffs (updated)
-

  src/master/validation.cpp 33e9ff7db9e2789cbb2d6dfd015288dfa1faa7c5 


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

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 60105: Clean rebooted slave's state if slaveInfo mismatches.

[Jiang Yan Xu]

> On June 23, 2017, 11:21 a.m., Jiang Yan Xu wrote:
> > src/slave/slave.cpp
> > Line 6003 (original), 6007 (patched)
> > 
> >
> > Empty line above.
> 
> Megha Sharma wrote:
> This is not possible I guess, git pre-commit hook complains: Redundant 
> blank line at the start of a code block should be deleted.

OK let's see the change. it's not the start of a code block in the verison I 
commented on. :)


- Jiang Yan


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


On June 23, 2017, 2:30 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60105/
> ---
> 
> (Updated June 23, 2017, 2:30 p.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to Mesos 1.4 we bypass the state recovery and
> start as a new agent upon reboot. Starting in Mesos 1.4
> we'll attempt to recover the slave state even after reboot
> except for when there is a SlaveInfo mismatch.
> Here, we cleanup the slave state for a rebooted agent if
> there's been a SlaveInfo mismatch during recovery to
> ensure that no other state is recovered and the
> agent enventually registers as a new agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 
> 
> 
> Diff: https://reviews.apache.org/r/60105/diff/8/
> 
> 
> Testing
> ---
> 
> make check done together with 60104 and 56895
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 60404: Documented the content of the `SlaveInfo.resources` field.

[Neil Conway]

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

(Updated June 24, 2017, 12:32 a.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


Repository: mesos


Description
---

Documented the content of the `SlaveInfo.resources` field.


Diffs (updated)
-

  include/mesos/mesos.proto 38449da44a5288afc7e2ef17ef5e1380ecabd5d2 
  include/mesos/v1/mesos.proto 81eca47ce1074f5e522a0410f01ab2b2cfc76aa2 


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

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 60405: Documented resource format in agent <-> master protocol.

[Neil Conway]

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

(Updated June 24, 2017, 12:31 a.m.)


Review request for mesos and Michael Park.


Changes
---

Tweak comment.


Repository: mesos


Description
---

Documented resource format in agent <-> master protocol.


Diffs (updated)
-

  src/messages/messages.proto 2c086263fdcee4d54a76a61379c2d4dba5271d23 
  src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 


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

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


Testing
---


Thanks,

Neil Conway

Review Request 60407: Avoided master crash on agent re-registration.

[Neil Conway]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

When validating the agent's ReregisterSlaveMessage, the master's
validation code neglected to account for the fact that the task
resources might not be in post-refinement format (e.g., if the agent
does not support reservation refinement). This lead to a `CHECK` failure
during validation.

Fix this by relaxing the validation of ReregisterSlaveMessage so that we
do not depend on the task resources being in post-refinement
format. This means validation of ReregisterSlaveMessage will be less
effective, but since it is best-effort anyway, this seems tolerable.


Diffs
-

  src/master/validation.cpp 33e9ff7db9e2789cbb2d6dfd015288dfa1faa7c5 


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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 56895: Added tests to ensure slave recovery post reboot.

[Jiang Yan Xu]

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




src/tests/slave_recovery_tests.cpp
Line 2575 (original), 2575 (patched)


Why the change? Doesn't look right.



src/tests/slave_recovery_tests.cpp
Line 2590 (original), 2590 (patched)


Why the change? Doesn't look right.

ASSERT_EQ is more appropriate since the next line would be invalid if the 
condition doesn't hold.



src/tests/slave_recovery_tests.cpp
Lines 2675 (patched)


s/postRebootContainerId/containerId/ No need to make such distinction?



src/tests/slave_recovery_tests.cpp
Lines 2699-2700 (patched)


OK I see that this is changed in the few version. I'll comment there.



src/tests/slave_recovery_tests.cpp
Lines 2699-2700 (original), 2700-2701 (patched)


s/No state recovery/No slave state recovery/

But you didn't really directly verify "No slave state recovery" right? 

I think this test just verified that the agent correctly restarted as a new 
agent? (which is sufficient IMO).



src/tests/slave_recovery_tests.cpp
Lines 2701 (patched)


Mention "reboot"?

Perhaps

s/RecoveryWithSlaveInfoMismatch/RebootWithSlaveInfoMismatch/



src/tests/slave_recovery_tests.cpp
Lines 2735 (patched)


One space before comments. 

There are some inconsistent examples in this file which I have fixed now so 
they don't get copied over.



src/tests/slave_recovery_tests.cpp
Lines 2739 (patched)


The resources being unreserved is not worth noting here?

Perhaps drop the comment altogether? It's pretty clear what the code is 
doing here.



src/tests/slave_recovery_tests.cpp
Lines 2754 (patched)


s/registerSlave/registerSlaveMessage/

Some of the examples in the file are very old and don't reflect our current 
conventions. :)



src/tests/slave_recovery_tests.cpp
Lines 2757 (patched)


s/Changing/Change/ so the tense is more consistent.



src/tests/slave_recovery_tests.cpp
Lines 2760 (patched)


Not "same flags" right? :)



src/tests/slave_recovery_tests.cpp
Lines 2769 (patched)


This expectation doesn't provide us much? We know it's recovered through 
`registerSlaveMessage` and additionally we know it's register instead of 
rereigster right?



src/tests/slave_recovery_tests.cpp
Lines 2772 (patched)


Blank line and is the comment necessary?

Instead, you could comment on that you are capturing `offers2` in order to 
get the `slaveId2` (which is what you reall want to verify)?

The same applies to `offers1`.



src/tests/slave_recovery_tests.cpp
Lines 2780 (patched)


This is not used anywhere?


- Jiang Yan Xu


On June 23, 2017, 2:45 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56895/
> ---
> 
> (Updated June 23, 2017, 2:45 p.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests to verify that the state is recovered post reboot and
> agentId is retained given the recovery finishes without errors and
> if the recovery fails due to slaveInfo mismatch then agent no
> state is recoverd making the agent register as a new agent.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp 90c172b6f7ca0082c3dfa2082050d6c18e4e636c 
> 
> 
> Diff: https://reviews.apache.org/r/56895/diff/12/
> 
> 
> Testing
> ---
> 
> make check done together with 60104 and 60103
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[James Peach]

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


Ship it!




Great idea! What do you think about setting the core rlimit to 0 in the the 
`subprocess` clone lambda in `perf::execute()`?

- James Peach


On June 23, 2017, 8:16 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> ---
> 
> (Updated June 23, 2017, 8:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.
> 
> 
> Bugs: MESOS-7160
> https://issues.apache.org/jira/browse/MESOS-7160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools build, the docker-build script performs a 'distcheck'
> build. This type of build warns if any unexpected files are left in
> the build directory after an uninstall, mainly to detect broken
> uninstall Makefile rules. The return status of the build container is
> the result of the distcheck.
> This fixes an issue where in some dockerized configurations
> invocations of 'perf' segfaulted (producing a core file as a
> side-effect), where the failure case was already anticipated and
> handled by the caller.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp a7262cd97361b55ff31238341657e764df6c9ea5 
> 
> 
> Diff: https://reviews.apache.org/r/60397/diff/1/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> Needs to be confirmed by Apache CI, e.g., reviewbot.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60398: Split perf version test into 2 separate tests.

[James Peach]

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



This drops the test that verifies we can actually parse perf versions that are 
found in the wild. That test is necessary to catch when new systems play games 
with the perf version; otherwise we would just detect that the perf version 
isn't supported and it would be hard to diagnose why not.

- James Peach


On June 23, 2017, 8:16 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60398/
> ---
> 
> (Updated June 23, 2017, 8:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Split perf version test into 2 separate tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/perf_tests.cpp 
> d8aab08eb131f974821fb85662cbc6cc685d2f3e 
> 
> 
> Diff: https://reviews.apache.org/r/60398/diff/1/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 60406: Added sanity check to resource downgrade on agent registration.

[Neil Conway]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

`SlaveInfo.resources` should always be representable in the
"pre-reservation-refinement" format, so `downgradeResources` should
always succeed.


Diffs
-

  src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 


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


Testing
---


Thanks,

Neil Conway

Review Request 60405: Documented resource format in agent <-> master protocol.

[Neil Conway]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Documented resource format in agent <-> master protocol.


Diffs
-

  src/messages/messages.proto 2c086263fdcee4d54a76a61379c2d4dba5271d23 


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


Testing
---


Thanks,

Neil Conway

Review Request 60404: Documented the content of the `SlaveInfo.resources` field.

[Neil Conway]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Documented the content of the `SlaveInfo.resources` field.


Diffs
-

  include/mesos/mesos.proto 38449da44a5288afc7e2ef17ef5e1380ecabd5d2 
  include/mesos/v1/mesos.proto 81eca47ce1074f5e522a0410f01ab2b2cfc76aa2 
  src/tests/slave_recovery_tests.cpp f6eafcbe3d89c7a69c03db0fd7bc10ae73d06584 


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


Testing
---


Thanks,

Neil Conway

Re: Review Request 56895: Added tests to ensure slave recovery post reboot.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [60103, 60104, 60105, 56895]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On June 23, 2017, 9:45 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56895/
> ---
> 
> (Updated June 23, 2017, 9:45 p.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests to verify that the state is recovered post reboot and
> agentId is retained given the recovery finishes without errors and
> if the recovery fails due to slaveInfo mismatch then agent no
> state is recoverd making the agent register as a new agent.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp 90c172b6f7ca0082c3dfa2082050d6c18e4e636c 
> 
> 
> Diff: https://reviews.apache.org/r/56895/diff/12/
> 
> 
> Testing
> ---
> 
> make check done together with 60104 and 60103
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 60400: (WIP) Skipped consulting registry if the agent is in the `slaves.recovered`.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [60400]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On June 23, 2017, 9:34 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60400/
> ---
> 
> (Updated June 23, 2017, 9:34 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-7711
> https://issues.apache.org/jira/browse/MESOS-7711
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Skipped consulting registry if the agent is in the `slaves.recovered`.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 33eca0d17459781fdc2ea915e8f40c78dd306962 
> 
> 
> Diff: https://reviews.apache.org/r/60400/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 56895: Added tests to ensure slave recovery post reboot.

[Megha Sharma]

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

(Updated June 23, 2017, 9:45 p.m.)


Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added tests to verify that the state is recovered post reboot and
agentId is retained given the recovery finishes without errors and
if the recovery fails due to slaveInfo mismatch then agent no
state is recoverd making the agent register as a new agent.


Diffs (updated)
-

  src/tests/slave_recovery_tests.cpp 90c172b6f7ca0082c3dfa2082050d6c18e4e636c 


Diff: https://reviews.apache.org/r/56895/diff/12/

Changes: https://reviews.apache.org/r/56895/diff/11-12/


Testing
---

make check done together with 60104 and 60103


Thanks,

Megha Sharma

Review Request 60400: (WIP) Skipped consulting registry if the agent is in the `slaves.recovered`.

[Jiang Yan Xu]

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

Review request for mesos and Neil Conway.


Repository: mesos


Description
---

Skipped consulting registry if the agent is in the `slaves.recovered`.


Diffs
-

  src/master/master.cpp 33eca0d17459781fdc2ea915e8f40c78dd306962 


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


Testing
---


Thanks,

Jiang Yan Xu

Re: Review Request 60105: Clean rebooted slave's state if slaveInfo mismatches.

[Megha Sharma]

> On June 23, 2017, 6:21 p.m., Jiang Yan Xu wrote:
> > src/slave/slave.cpp
> > Line 6003 (original), 6007 (patched)
> > 
> >
> > Empty line above.

This is not possible I guess, git pre-commit hook complains: Redundant blank 
line at the start of a code block should be deleted.


- Megha


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


On June 23, 2017, 9:30 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60105/
> ---
> 
> (Updated June 23, 2017, 9:30 p.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to Mesos 1.4 we bypass the state recovery and
> start as a new agent upon reboot. Starting in Mesos 1.4
> we'll attempt to recover the slave state even after reboot
> except for when there is a SlaveInfo mismatch.
> Here, we cleanup the slave state for a rebooted agent if
> there's been a SlaveInfo mismatch during recovery to
> ensure that no other state is recovered and the
> agent enventually registers as a new agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 
> 
> 
> Diff: https://reviews.apache.org/r/60105/diff/8/
> 
> 
> Testing
> ---
> 
> make check done together with 60104 and 56895
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 60105: Clean rebooted slave's state if slaveInfo mismatches.

[Megha Sharma]

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

(Updated June 23, 2017, 9:30 p.m.)


Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Prior to Mesos 1.4 we bypass the state recovery and
start as a new agent upon reboot. Starting in Mesos 1.4
we'll attempt to recover the slave state even after reboot
except for when there is a SlaveInfo mismatch.
Here, we cleanup the slave state for a rebooted agent if
there's been a SlaveInfo mismatch during recovery to
ensure that no other state is recovered and the
agent enventually registers as a new agent.


Diffs (updated)
-

  src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 


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

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


Testing
---

make check done together with 60104 and 56895


Thanks,

Megha Sharma

Re: Review Request 60105: Clean rebooted slave's state if slaveInfo mismatches.

[Megha Sharma]

> On June 23, 2017, 6:21 p.m., Jiang Yan Xu wrote:
> > src/slave/slave.cpp
> > Lines 5994-5999 (patched)
> > 
> >
> > I tweaked it a little bit:
> > 
> > ```
> >   // Fail the recovery unless the agent is recovering for the first
> >   // time after host reboot.
> >   //
> >   // Prior to Mesos 1.4 we directly bypass the state recovery and
> >   // start as a new agent upon reboot (introduced in MESOS-844).
> >   // This unncessarily discards the existing agent ID (MESOS-6223).
> >   // Starting in Mesos 1.4 we'll attempt to recover the slave state
> >   // even after reboot but in case of slave info mismatch we'll fall
> >   // back to recovering as a new agent (existing behavior). This
> >   // prevents the agent from flapping if the slave info (resources,
> >   // attributes, etc.) change is due to host maintainance associated
> >   // with the reboot.
> > ```
> > 
> > What do you think? Feel free to improve on it.

+1, good and concise explanation about the changed behavior.


- Megha


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


On June 23, 2017, 5:19 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60105/
> ---
> 
> (Updated June 23, 2017, 5:19 p.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to Mesos 1.4 we bypass the state recovery and
> start as a new agent upon reboot. Starting in Mesos 1.4
> we'll attempt to recover the slave state even after reboot
> except for when there is a SlaveInfo mismatch.
> Here, we cleanup the slave state for a rebooted agent if
> there's been a SlaveInfo mismatch during recovery to
> ensure that no other state is recovered and the
> agent enventually registers as a new agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 
> 
> 
> Diff: https://reviews.apache.org/r/60105/diff/7/
> 
> 
> Testing
> ---
> 
> make check done together with 60104 and 56895
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 60398: Split perf version test into 2 separate tests.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [60397, 60398]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On June 23, 2017, 9:16 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60398/
> ---
> 
> (Updated June 23, 2017, 9:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Split perf version test into 2 separate tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/perf_tests.cpp 
> d8aab08eb131f974821fb85662cbc6cc685d2f3e 
> 
> 
> Diff: https://reviews.apache.org/r/60398/diff/1/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Andrei Budnik]

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

Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.


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


Repository: mesos


Description
---

For autotools build, the docker-build script performs a 'distcheck'
build. This type of build warns if any unexpected files are left in
the build directory after an uninstall, mainly to detect broken
uninstall Makefile rules. The return status of the build container is
the result of the distcheck.
This fixes an issue where in some dockerized configurations
invocations of 'perf' segfaulted (producing a core file as a
side-effect), where the failure case was already anticipated and
handled by the caller.


Diffs
-

  src/tests/environment.cpp a7262cd97361b55ff31238341657e764df6c9ea5 


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


Testing
---

1. make check (mac os x 10.12, fedora 25)
2. internal CI

Needs to be confirmed by Apache CI, e.g., reviewbot.


Thanks,

Andrei Budnik

Review Request 60398: Split perf version test into 2 separate tests.

[Andrei Budnik]

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

Review request for mesos, Benjamin Bannier and James Peach.


Repository: mesos


Description
---

Split perf version test into 2 separate tests.


Diffs
-

  src/tests/containerizer/perf_tests.cpp 
d8aab08eb131f974821fb85662cbc6cc685d2f3e 


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


Testing
---

1. make check (mac os x 10.12, fedora 25)
2. internal CI


Thanks,

Andrei Budnik

Re: Review Request 56895: Added tests to ensure slave recovery post reboot.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [60103, 60104, 60105, 56895]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On June 23, 2017, 6:01 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56895/
> ---
> 
> (Updated June 23, 2017, 6:01 p.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests to verify that the state is recovered post reboot and
> agentId is retained given the recovery finishes without errors and
> if the recovery fails due to slaveInfo mismatch then agent no
> state is recoverd making the agent register as a new agent.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp 90c172b6f7ca0082c3dfa2082050d6c18e4e636c 
> 
> 
> Diff: https://reviews.apache.org/r/56895/diff/11/
> 
> 
> Testing
> ---
> 
> make check done together with 60104 and 60103
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 60105: Clean rebooted slave's state if slaveInfo mismatches.

[Jiang Yan Xu]

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




src/slave/slave.cpp
Lines 5980-5981 (original), 5980-5981 (patched)


```
SlaveInfo _info(info);
```

We typically add a prefix for two variations of the same thing.



src/slave/slave.cpp
Line 5985 (original), 5985 (patched)


Nit: s/errorMessage/message/

There should be no confusion here with a shorter name.



src/slave/slave.cpp
Lines 5994-5999 (patched)


I tweaked it a little bit:

```
  // Fail the recovery unless the agent is recovering for the first
  // time after host reboot.
  //
  // Prior to Mesos 1.4 we directly bypass the state recovery and
  // start as a new agent upon reboot (introduced in MESOS-844).
  // This unncessarily discards the existing agent ID (MESOS-6223).
  // Starting in Mesos 1.4 we'll attempt to recover the slave state
  // even after reboot but in case of slave info mismatch we'll fall
  // back to recovering as a new agent (existing behavior). This
  // prevents the agent from flapping if the slave info (resources,
  // attributes, etc.) change is due to host maintainance associated
  // with the reboot.
```

What do you think? Feel free to improve on it.



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


`if (state.rebooted)` instead? (Whitespace and variable usage)

The above examples

```
  Option resourcesState = state->resources;
  Option slaveState = state->slave;
```

help reduce line length and are used many times. It's not the same 
situation with `rebooted`.



src/slave/slave.cpp
Lines 5997-6003 (original), 6000-6016 (patched)


To avoid going to deep in the branching hierarchy perhaps the following is 
better?

```
  if (!state.rebooted) {
return Failure(message);
  }

  LOG(WARNING) << "Falling back to recover as a new agent due to error: 
"
   << message;

  // Cleaning up the slave state to avoid any state recovery for the
  // old agent.
  slaveState = None();
```



src/slave/slave.cpp
Lines 5997-6002 (original), 6001-6006 (patched)


This log message and metric increment used to occur outside

```
if (flags.recover == "reconnect" &&
!(infoCopy == slaveState->info.get())) {
```

but it's now changed.

How about we put it directly under 

```
if (slaveState.isSome() && slaveState->info.isSome()) {
```

You can find similar handling above with `resourcesState`:

```
  if (resourcesState.isSome()) {
if (resourcesState->errors > 0) {
  LOG(WARNING) << "Errors encountered during resources recovery: "
   << resourcesState->errors;

  metrics.recovery_errors += resourcesState->errors;
}
```



src/slave/slave.cpp
Line 6003 (original), 6007 (patched)


Empty line above.



src/slave/slave.cpp
Line 6004 (original), 6019 (patched)


Empty line above comments.


- Jiang Yan Xu


On June 23, 2017, 10:19 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60105/
> ---
> 
> (Updated June 23, 2017, 10:19 a.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to Mesos 1.4 we bypass the state recovery and
> start as a new agent upon reboot. Starting in Mesos 1.4
> we'll attempt to recover the slave state even after reboot
> except for when there is a SlaveInfo mismatch.
> Here, we cleanup the slave state for a rebooted agent if
> there's been a SlaveInfo mismatch during recovery to
> ensure that no other state is recovered and the
> agent enventually registers as a new agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 
> 
> 
> Diff: https://reviews.apache.org/r/60105/diff/7/
> 
> 
> Testing
> ---
> 
> make check done together with 60104 and 56895
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 56895: Added tests to ensure slave recovery post reboot.

[Megha Sharma]

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

(Updated June 23, 2017, 6:01 p.m.)


Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added tests to verify that the state is recovered post reboot and
agentId is retained given the recovery finishes without errors and
if the recovery fails due to slaveInfo mismatch then agent no
state is recoverd making the agent register as a new agent.


Diffs (updated)
-

  src/tests/slave_recovery_tests.cpp 90c172b6f7ca0082c3dfa2082050d6c18e4e636c 


Diff: https://reviews.apache.org/r/56895/diff/11/

Changes: https://reviews.apache.org/r/56895/diff/10-11/


Testing
---

make check done together with 60104 and 60103


Thanks,

Megha Sharma

Re: Review Request 60104: Added rebooted flag to State.

[Megha Sharma]

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

(Updated June 23, 2017, 5:20 p.m.)


Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added rebooted flag to State to remember whether or not
the agent has rebooted.


Diffs (updated)
-

  src/slave/state.hpp 537358cb54393dd02b655050c26504808a1505ad 
  src/slave/state.cpp 5dd8b1cc29cf1809e948015e146691596a8202b8 


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

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


Testing
---

make check done together with 60105 and 56895


Thanks,

Megha Sharma

Re: Review Request 60105: Clean rebooted slave's state if slaveInfo mismatches.

[Megha Sharma]

> On June 21, 2017, 4:39 p.m., Jiang Yan Xu wrote:
> > src/slave/slave.cpp
> > Lines 5798-5867 (original), 5798-5867 (patched)
> > 
> >
> > So all of this work is only useful for recovering frameworks, it looks 
> > like we don't need to do them before we decide we actually are going to 
> > recover the frameworks (i.e., not continue as a new agent)?
> > 
> > This reasoning certainly also applies to the current HEAD but since we 
> > are donig refactor here, we should probably address this (in a separate 
> > preceding review perhaps)
> 
> Megha Sharma wrote:
> I strongly agree we should defer doing this work until we start to 
> recover the frameworks. I was thinking of doing this may be as another review 
> for slave recovery improvements after this change is in.

Agreed to do in a separate refactoring/improvements patch may be after this one 
lands.


- Megha


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


On June 23, 2017, 5:19 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60105/
> ---
> 
> (Updated June 23, 2017, 5:19 p.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to Mesos 1.4 we bypass the state recovery and
> start as a new agent upon reboot. Starting in Mesos 1.4
> we'll attempt to recover the slave state even after reboot
> except for when there is a SlaveInfo mismatch.
> Here, we cleanup the slave state for a rebooted agent if
> there's been a SlaveInfo mismatch during recovery to
> ensure that no other state is recovered and the
> agent enventually registers as a new agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 
> 
> 
> Diff: https://reviews.apache.org/r/60105/diff/7/
> 
> 
> Testing
> ---
> 
> make check done together with 60104 and 56895
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 60105: Clean rebooted slave's state if slaveInfo mismatches.

[Megha Sharma]

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

(Updated June 23, 2017, 5:19 p.m.)


Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Prior to Mesos 1.4 we bypass the state recovery and
start as a new agent upon reboot. Starting in Mesos 1.4
we'll attempt to recover the slave state even after reboot
except for when there is a SlaveInfo mismatch.
Here, we cleanup the slave state for a rebooted agent if
there's been a SlaveInfo mismatch during recovery to
ensure that no other state is recovered and the
agent enventually registers as a new agent.


Diffs (updated)
-

  src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 


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

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


Testing
---

make check done together with 60104 and 56895


Thanks,

Megha Sharma

Re: Review Request 60370: Updated agent webui page to display allocated resources per each role.

[haosdent huang]

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




src/webui/master/static/agent.html
Lines 176 (patched)


Should we use `*` here to keep consistent with other parts?



src/webui/master/static/agent.html
Lines 184 (patched)


Is `|| 0` necessary here?



src/webui/master/static/agent.html
Lines 186 (patched)


Is `|| 0` necessary here?


- haosdent huang


On June 22, 2017, 3:26 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60370/
> ---
> 
> (Updated June 22, 2017, 3:26 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-6441
> https://issues.apache.org/jira/browse/MESOS-6441
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated agent webui page to display allocated resources per each role.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/agent.html 71e5e702e5e64e6f46c84791247aa5156c046ed9 
>   src/webui/master/static/js/controllers.js 
> 67bfd030649dd21840c16188a4964f814aa232d7 
> 
> 
> Diff: https://reviews.apache.org/r/60370/diff/1/
> 
> 
> Testing
> ---
> 
> See screenshot.
> 
> 
> File Attachments
> 
> 
> agent.html
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/22/66cc61b4-04c8-451e-a434-58f556397724__Resource_Reservations.png
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60203: Introduce HTB class.

[Ilya Pronin]

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



Looks solid to me. Comments below are mostly style issues.


src/linux/routing/queueing/htb.hpp
Lines 49 (patched)


Style: for functions the opening brace should be on a separate line.



src/linux/routing/queueing/htb.hpp
Lines 58-61 (patched)


libnl uses `uint32_t` for these. If we use `uint64_t` we have to check that 
the value. Plus we'll get a warning without an explicit cast.



src/linux/routing/queueing/htb.hpp
Lines 102 (patched)


Maybe `classExists()`?



src/linux/routing/queueing/internal.hpp
Lines 256-259 (patched)


If the link doesn't exist `rtnl_link_get_ifindex()` will return 0. Is it OK 
here? Will it simply dump all existing classes? Would we want to return an 
error in such case?



src/linux/routing/queueing/internal.hpp
Lines 274-279 (patched)


Style: The code inside `if` is overindented.



src/linux/routing/queueing/internal.hpp
Lines 566 (patched)


Spelling: s/Removs/Removes/, s/clas/class/



src/linux/routing/queueing/internal.hpp
Lines 569-572 (patched)


Style: function parameters should be indented with 4 spaces.



src/linux/routing/queueing/internal.hpp
Lines 595-597 (patched)


Why don't we handle this here to be consistent with other `*Class()` 
functions?


- Ilya Pronin


On June 19, 2017, 8:36 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60203/
> ---
> 
> (Updated June 19, 2017, 8:36 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, Jie Yu, Santhosh Kumar Shanmugham, and 
> Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Based heavily on Cong Wang's original review 45605.
> 
> Introduce support for HTB class and expose configuration.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/queueing/class.hpp PRE-CREATION 
>   src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
>   src/linux/routing/queueing/htb.cpp 464120214b75e2e422c8ea6a57c5654ba77d669f 
>   src/linux/routing/queueing/internal.hpp 
> 9fe522ee017c86af8c7b2e518cd0957af08750e4 
> 
> 
> Diff: https://reviews.apache.org/r/60203/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 60203: Introduce HTB class.

[Dmitry Zhuk]

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




src/linux/routing/queueing/htb.cpp
Lines 144-145 (patched)


shouldn't ClassConfig also use uint32_t for rate, ceil and cburst?



src/linux/routing/queueing/internal.hpp
Lines 342 (patched)


There's `exists`, `create` and `remove` for for qdisc, and similar 
functions with `Class` suffix for class.
I think we should make naming consistent for them.



src/linux/routing/queueing/internal.hpp
Lines 566 (patched)


s/clas/class/


- Dmitry Zhuk


On June 19, 2017, 7:36 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60203/
> ---
> 
> (Updated June 19, 2017, 7:36 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, Jie Yu, Santhosh Kumar Shanmugham, and 
> Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Based heavily on Cong Wang's original review 45605.
> 
> Introduce support for HTB class and expose configuration.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/queueing/class.hpp PRE-CREATION 
>   src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
>   src/linux/routing/queueing/htb.cpp 464120214b75e2e422c8ea6a57c5654ba77d669f 
>   src/linux/routing/queueing/internal.hpp 
> 9fe522ee017c86af8c7b2e518cd0957af08750e4 
> 
> 
> Diff: https://reviews.apache.org/r/60203/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 60104: Added rebooted flag to State.

[Jiang Yan Xu]

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


Fix it, then Ship it!





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


No need for this?


- Jiang Yan Xu


On June 22, 2017, 2:17 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60104/
> ---
> 
> (Updated June 22, 2017, 2:17 p.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added rebooted flag to State to remember whether or not
> the agent has rebooted.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 
>   src/slave/state.hpp 537358cb54393dd02b655050c26504808a1505ad 
>   src/slave/state.cpp 5dd8b1cc29cf1809e948015e146691596a8202b8 
> 
> 
> Diff: https://reviews.apache.org/r/60104/diff/6/
> 
> 
> Testing
> ---
> 
> make check done together with 60105 and 56895
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>