Re: Review Request 63385: Added utility functions for CSI Plugin info and volume attributes.

[Jie Yu]

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




src/csi/utils.hpp
Lines 36-43 (patched)


Can you explain to me why you need the equality check for 
GetPluginInfoResponse?



src/csi/utils.hpp
Lines 99 (patched)


We don't do typedef, especially in header.

Also, I feel this is a general protobuf util helper. We should put it under 
src/common/protobuf_utils.hpp|cpp

```
convertMapsToLabels(...)
convertLabelsToMaps(...)
```


- Jie Yu


On Nov. 22, 2017, 5:21 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63385/
> ---
> 
> (Updated Nov. 22, 2017, 5:21 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added utility functions for CSI Plugin info and volume attributes.
> 
> 
> Diffs
> -
> 
>   src/csi/utils.hpp PRE-CREATION 
>   src/csi/utils.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63385/diff/2/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 64067: Fixed a bug for starting local resource provider daemon multiple times.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Nov. 24, 2017, 6:32 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64067/
> ---
> 
> (Updated Nov. 24, 2017, 6:32 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug for starting local resource provider daemon multiple times.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/daemon.cpp 4026c14bda48e5c6c58ff60f65f859518ce6c6cd 
> 
> 
> Diff: https://reviews.apache.org/r/64067/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 64033: Terminated driver-based executors if kill arrives before launch task.

[Andrei Budnik]

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




src/exec/exec.cpp
Lines 355 (patched)


What if `killTask` received after task completion and 
[acknowledgement](https://github.com/apache/mesos/blob/708902d6f207b6f93c94b3589a31d54c812a7657/src/exec/exec.cpp#L396)?
 Maybe we should just update the log message to cover all possible states of 
the driver (including one I mentioned above).

Also, it would be helpful to have 2 different checks and 2 different log 
messages to distinguish each case.


- Andrei Budnik


On Nov. 24, 2017, 8:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64033/
> ---
> 
> (Updated Nov. 24, 2017, 8:14 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.
> 
> 
> Bugs: MESOS-8247
> https://issues.apache.org/jira/browse/MESOS-8247
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
> to a driver-based executor. Since these messages are not retried,
> without this patch an executor never starts a task and remains idle,
> ignoring kill task request. This patch ensures all driver-based
> executors eventually shut down if kill task arrives before the task
> has been started.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 3974f20052e3c12eb154a5146d19d4dc1759859f 
>   src/exec/exec.cpp cdbf149a9047ddad6beef64be1266e15e7643afc 
>   src/launcher/executor.cpp c688c04e598ac140421fd5e47359b0e48d30bcc5 
> 
> 
> Diff: https://reviews.apache.org/r/64033/diff/3/
> 
> 
> Testing
> ---
> 
> make check on MacOS 10.11.6
> 
> Manual testing using modified "exec.cpp" that drops executor registration 
> confirmation.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64033: Terminated driver-based executors if kill arrives before launch task.

[Alexander Rukletsov]

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

(Updated Nov. 24, 2017, 8:14 p.m.)


Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.


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


Repository: mesos


Description
---

`ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
to a driver-based executor. Since these messages are not retried,
without this patch an executor never starts a task and remains idle,
ignoring kill task request. This patch ensures all driver-based
executors eventually shut down if kill task arrives before the task
has been started.


Diffs (updated)
-

  src/docker/executor.cpp 3974f20052e3c12eb154a5146d19d4dc1759859f 
  src/exec/exec.cpp cdbf149a9047ddad6beef64be1266e15e7643afc 
  src/launcher/executor.cpp c688c04e598ac140421fd5e47359b0e48d30bcc5 


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

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


Testing
---

make check on MacOS 10.11.6

Manual testing using modified "exec.cpp" that drops executor registration 
confirmation.


Thanks,

Alexander Rukletsov

Review Request 64068: Increased executor log verbosity in tests.

[Alexander Rukletsov]

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

Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/mesos.hpp e5c2b697a4401f010a7d77ae9acb8a9cbd3846bc 


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


Testing
---

See https://reviews.apache.org/r/64033/


Thanks,

Alexander Rukletsov

Review Request 64070: Ensured executor adapter propagates error and shutdown messages.

[Alexander Rukletsov]

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

Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.


Repository: mesos


Description
---

Prior to this patch, if an error or shutdown occurred during
subscription / registration with the agent, it was not propagated back
to the executor if the v0_v1 executor adapter was used. This happened
because the adapter did not call the `connected` callback until after
successful registration and hence the executor did not even try to
send the `SUBSCRIBE` call, without which the adapter did not send any
events to the executor.

A fix is to call the `connected` callback if an error occurred or
shutdown even arrived before the executor had subscribed.


Diffs
-

  src/executor/v0_v1executor.cpp 61d591993e6388ba3b4d64a3bdb63c3a3513fbeb 


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


Testing
---

See https://reviews.apache.org/r/64033/


Thanks,

Alexander Rukletsov

Re: Review Request 64033: Terminated driver-based executors if kill arrives before launch task.

[Alexander Rukletsov]

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

(Updated Nov. 24, 2017, 7:13 p.m.)


Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.


Summary (updated)
-

Terminated driver-based executors if kill arrives before launch task.


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


Repository: mesos


Description (updated)
---

`ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
to a driver-based executor. Since these messages are not retried,
without this patch an executor never starts a task and remains idle,
ignoring kill task request. This patch ensures all driver-based
executors eventually shut down if kill task arrives before the task
has been started.


Diffs (updated)
-

  src/exec/exec.cpp cdbf149a9047ddad6beef64be1266e15e7643afc 


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

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


Testing
---

make check on MacOS 10.11.6

Manual testing using modified "exec.cpp" that drops executor registration 
confirmation.


Thanks,

Alexander Rukletsov

Re: Review Request 64032: Promoted log level to warning for disconnected events in exec.cpp.

[Alexander Rukletsov]

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

(Updated Nov. 24, 2017, 7:12 p.m.)


Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.


Repository: mesos


Description
---

When the executor library receives messages while being disconnected,
it might indicate an out-of-order message delivery or lost messages.
This should be logged at the warning level to simplify triaging.


Diffs (updated)
-

  src/exec/exec.cpp cdbf149a9047ddad6beef64be1266e15e7643afc 


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

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Review Request 64069: Ensured command executor always honors shutdown request.

[Alexander Rukletsov]

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

Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/launcher/executor.cpp c688c04e598ac140421fd5e47359b0e48d30bcc5 


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


Testing
---

See https://reviews.apache.org/r/64033/


Thanks,

Alexander Rukletsov

Re: Review Request 63377: Added filesystem layout for CSI plugins.

[Chun-Hung Hsiao]

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

(Updated Nov. 24, 2017, 6:48 p.m.)


Review request for mesos, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
---

Addressed jieyu's comment.


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


Repository: mesos


Description
---

A CSI plugin can now store the following persistent CSI
data in `/csi///`:

1. Mount points of CSI volumes: `volumes/`
2. States of CSI volumes: `states//state`
3. Directory to place CSI endpoints: `/tmp/mesos-csi-XX`, which
   would be symlinked by `containers//endpoint`.


Diffs (updated)
-

  src/Makefile.am 9641ad4cf2a1f7fe56a4dcccb6eeff7130872fa8 
  src/csi/paths.hpp PRE-CREATION 
  src/csi/paths.cpp PRE-CREATION 
  src/slave/paths.hpp 66dfa4544772d78ccc9229dc861da60c79913f24 
  src/slave/paths.cpp b03ffeeed83cb73228cca27769262fb08df38fb5 


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

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


Testing
---

make


Thanks,

Chun-Hung Hsiao

Review Request 64067: Fixed a bug for starting local resource provider daemon multiple times.

[Chun-Hung Hsiao]

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

Review request for mesos, Alexander Rukletsov, Jie Yu, and Jan Schlicht.


Repository: mesos


Description
---

Fixed a bug for starting local resource provider daemon multiple times.


Diffs
-

  src/resource_provider/daemon.cpp 4026c14bda48e5c6c58ff60f65f859518ce6c6cd 


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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 64033: Terminated executors if kill task received before launch task.

[Alexander Rukletsov]

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

(Updated Nov. 24, 2017, 6:21 p.m.)


Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.


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


Repository: mesos


Description
---

`ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
to a driver-based executor. Since these messages are not retried,
the executor will never start a task and will remain idle, ignoring
kill task request. This patch shutdown driver-based executors if kill
task arrives before the task has been started.


Diffs
-

  src/docker/executor.cpp 3974f20052e3c12eb154a5146d19d4dc1759859f 
  src/launcher/executor.cpp c688c04e598ac140421fd5e47359b0e48d30bcc5 


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


Testing (updated)
---

make check on MacOS 10.11.6

Manual testing using modified "exec.cpp" that drops executor registration 
confirmation.


Thanks,

Alexander Rukletsov

Re: Review Request 64033: Terminated executors if kill task received before launch task.

[Alexander Rukletsov]

> On Nov. 22, 2017, 5:44 p.m., Vinod Kone wrote:
> > can you write tests please?

Writing a test here is not trivial. We don't have infrastructure in place to 
"hook" to our executors. All built-in executors are implemented in ".cpp" 
files, which does not allow to create an instance of executor process in a test 
and, for example, drop specific calls.


- Alexander


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


On Nov. 22, 2017, 5:05 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64033/
> ---
> 
> (Updated Nov. 22, 2017, 5:05 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.
> 
> 
> Bugs: MESOS-8247
> https://issues.apache.org/jira/browse/MESOS-8247
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
> to a driver-based executor. Since these messages are not retried,
> the executor will never start a task and will remain idle, ignoring
> kill task request. This patch shutdown driver-based executors if kill
> task arrives before the task has been started.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 3974f20052e3c12eb154a5146d19d4dc1759859f 
>   src/launcher/executor.cpp c688c04e598ac140421fd5e47359b0e48d30bcc5 
> 
> 
> Diff: https://reviews.apache.org/r/64033/diff/1/
> 
> 
> Testing
> ---
> 
> make check on MacOS 10.11.6
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64033: Terminated executors if kill task received before launch task.

[Alexander Rukletsov]

> On Nov. 24, 2017, 1:07 p.m., Andrei Budnik wrote:
> > src/docker/executor.cpp
> > Lines 386 (patched)
> > 
> >
> > Consider moving this check to [the 
> > driver](https://github.com/apache/mesos/blob/708902d6f207b6f93c94b3589a31d54c812a7657/src/exec/exec.cpp#L343).

This is a very good suggestion.

First off, this way we guarantee that all driver-based executors behave 
properly in this case.

Second, behaviour of the command executor in the HTTP mode is not affected, 
which is a good thing.

And third, if the `killTask` message is not delivered to the executor (for 
example, due to V0-V1 executor adapter holding off messages until `registered` 
is received), the executor will eventually shutdown.

I will update the patch in a while.


- Alexander


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


On Nov. 22, 2017, 5:05 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64033/
> ---
> 
> (Updated Nov. 22, 2017, 5:05 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.
> 
> 
> Bugs: MESOS-8247
> https://issues.apache.org/jira/browse/MESOS-8247
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
> to a driver-based executor. Since these messages are not retried,
> the executor will never start a task and will remain idle, ignoring
> kill task request. This patch shutdown driver-based executors if kill
> task arrives before the task has been started.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 3974f20052e3c12eb154a5146d19d4dc1759859f 
>   src/launcher/executor.cpp c688c04e598ac140421fd5e47359b0e48d30bcc5 
> 
> 
> Diff: https://reviews.apache.org/r/64033/diff/1/
> 
> 
> Testing
> ---
> 
> make check on MacOS 10.11.6
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 63842: Allowed removing non-terminal offer operations.

[Benjamin Bannier]

> On Nov. 21, 2017, 7:59 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Line 10925 (original), 10922 (patched)
> > 
> >
> > Especially now that we have code in this function which switches on 
> > operation type, could we add a comment to the method's declaration briefly 
> > explaining its use? For example, it's not clear from reading the header 
> > that this method should not be called on LAUNCH or LAUNCH_GROUP operations.

Dropping this issue as this code is now much less concerned about particular 
operation types.


> On Nov. 21, 2017, 7:59 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 10936-10939 (patched)
> > 
> >
> > Nit: could we put this case at the end? I think we tend to place the 
> > UNKNOWN case last.

Dropping this issue as this code is now simplified by using a helper.


- Benjamin


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


On Nov. 24, 2017, 3:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63842/
> ---
> 
> (Updated Nov. 24, 2017, 3:52 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> During reconcilation we might be required to remove non-terminal offer
> operations from bookkeeping.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 2a2e830354db4a2191fb8321beb8174b80f7ba7d 
>   src/master/master.cpp 53263e499d88b906b6406c24c0dfb737e589e813 
>   src/slave/slave.cpp f93ff7b20815c3ccb274ce6990ee66a17b6ac51c 
> 
> 
> Diff: https://reviews.apache.org/r/63842/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`, tested as part of https://reviews.apache.org/r/63843/.
> 
> This patch requires `protobuf::isSpeculativeOperation` from 
> https://reviews.apache.org/r/63751/ which is _not_ part of this review chain 
> (to avoid multiple dependent RRs).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63842: Allowed removing non-terminal offer operations.

[Benjamin Bannier]

> On Nov. 21, 2017, 11:22 p.m., Jie Yu wrote:
> > src/master/master.hpp
> > Lines 2919 (patched)
> > 
> >
> > `removeOfferOperation` might be used for old operations too. This will 
> > throw a CHECK failure.

Dropping this as the code was removed. Now that `addOfferOperation` does not 
modify resources anymore, neither should `removeOfferOperation`.


- Benjamin


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


On Nov. 24, 2017, 3:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63842/
> ---
> 
> (Updated Nov. 24, 2017, 3:52 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> During reconcilation we might be required to remove non-terminal offer
> operations from bookkeeping.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 2a2e830354db4a2191fb8321beb8174b80f7ba7d 
>   src/master/master.cpp 53263e499d88b906b6406c24c0dfb737e589e813 
>   src/slave/slave.cpp f93ff7b20815c3ccb274ce6990ee66a17b6ac51c 
> 
> 
> Diff: https://reviews.apache.org/r/63842/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`, tested as part of https://reviews.apache.org/r/63843/.
> 
> This patch requires `protobuf::isSpeculativeOperation` from 
> https://reviews.apache.org/r/63751/ which is _not_ part of this review chain 
> (to avoid multiple dependent RRs).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63842: Allowed removing non-terminal offer operations.

[Benjamin Bannier]

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

(Updated Nov. 24, 2017, 3:52 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Rebased.


Repository: mesos


Description
---

During reconcilation we might be required to remove non-terminal offer
operations from bookkeeping.


Diffs (updated)
-

  src/master/master.hpp 2a2e830354db4a2191fb8321beb8174b80f7ba7d 
  src/master/master.cpp 53263e499d88b906b6406c24c0dfb737e589e813 
  src/slave/slave.cpp f93ff7b20815c3ccb274ce6990ee66a17b6ac51c 


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

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


Testing
---

`make check`, tested as part of https://reviews.apache.org/r/63843/.

This patch requires `protobuf::isSpeculativeOperation` from 
https://reviews.apache.org/r/63751/ which is _not_ part of this review chain 
(to avoid multiple dependent RRs).


Thanks,

Benjamin Bannier

Re: Review Request 63732: Reconciled offer operations between agent and master.

[Benjamin Bannier]

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

(Updated Nov. 24, 2017, 3:50 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.cpp 53263e499d88b906b6406c24c0dfb737e589e813 


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

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


Testing
---

`make check`, tested as part of https://reviews.apache.org/r/63843/.

This patch requires `protobuf::isSpeculativeOperation` from 
https://reviews.apache.org/r/63751/ which is _not_ part of this review chain 
(to avoid multiple dependent RRs).


Thanks,

Benjamin Bannier

Review Request 64062: Added 6 nested container tests for `docker/volume` isolator.

[Qian Zhang]

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

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


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


Repository: mesos


Description
---

Added 6 nested container tests for `docker/volume` isolator.


Diffs
-

  src/tests/containerizer/docker_volume_isolator_tests.cpp 
9123cbe50e4a3417dc4f205092a436d7230c7414 


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


Testing
---

Ran all the tests for `docker/volume` isolator repeatedly (20 times).


Thanks,

Qian Zhang

Re: Review Request 63730: Passed operations from resource provider to agent.

[Benjamin Bannier]

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

(Updated Nov. 24, 2017, 3:10 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/resource_provider/manager.cpp 988442a55edac93c562c2398673059384a1b2e22 
  src/resource_provider/message.hpp 19cf08e99b19d2d5077b3965d154cfd4554326d1 


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

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


Testing
---

`make check`, still need to implement dedicated tests.


Thanks,

Benjamin Bannier

Re: Review Request 63917: Renamed resource provider message UpdateTotalResources to UpdateState.

[Benjamin Bannier]

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

(Updated Nov. 24, 2017, 3:10 p.m.)


Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.


Changes
---

Rebased.


Repository: mesos


Description
---

This message now captures much more than just resource updates, but
instead informs users about the full resource provider state. This
change mirrors the message naming used between resource provider
manager and resource providers.


Diffs (updated)
-

  src/resource_provider/manager.cpp 988442a55edac93c562c2398673059384a1b2e22 
  src/resource_provider/message.hpp 19cf08e99b19d2d5077b3965d154cfd4554326d1 
  src/slave/slave.cpp f93ff7b20815c3ccb274ce6990ee66a17b6ac51c 
  src/tests/resource_provider_manager_tests.cpp 
27fa4403651953e1ad94bbba7ef2e46344495bfa 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63843: Implemented a test of offer operation reconcilation.

[Benjamin Bannier]

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

(Updated Nov. 24, 2017, 3:07 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Rebased.


Repository: mesos


Description
---

Whenever a speculated operation fails in a resource provider, we
expect the agent to trigger a 'UpdateSlaveMessage' to the master so it
can rollback its agent state. This patch adds such a test.


Diffs (updated)
-

  src/tests/slave_tests.cpp a2274b691cf94b003c4bc15450d176a9c73517d5 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63731: Reconciled pending resource provider operations in agent.

[Benjamin Bannier]

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

(Updated Nov. 24, 2017, 3:08 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Always send offer operations when sending an `UpdateSlaveMessage` with a 
`total`.


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


Repository: mesos


Description
---

When resource providers update their state they send a list of
pending or unacknowledged operations to the agent. This patch add
tracking for such operations to the agent. The agent can then forward
these operations to the master, e.g., for calculating the unused
resources behind an agent.

We track an operation until we either receive a updated list of
pending or unacknowledged operations from a resource provider, or
until we see an acknowledgement from a framework. This keeps the list
of operations bounded and ensures that we maintain the latest
information in the agent.


Diffs (updated)
-

  src/slave/slave.cpp f93ff7b20815c3ccb274ce6990ee66a17b6ac51c 


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

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


Testing
---

`make check`, tested as part of https://reviews.apache.org/r/63843/.


Thanks,

Benjamin Bannier

Re: Review Request 64001: Made sure all true allocator agent updates return correct status.

[Benjamin Bannier]

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

(Updated Nov. 24, 2017, 3:07 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


Repository: mesos


Description
---

The allocator method 'updateSlave' can be used to modify either an
agent's capabilities, its total resources, or both. It is expected
that this method return 'true' whenever a change occurred.

This patch fixes an issue where if the method as implemented in the
hierarchical allocator was called with new capabilities but the
current total resources, 'false' would have been returned, even though
the capabilities where updated.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
cfeeb3bfa3ad7d78ff6e22cfc4adb1f0efa05629 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63997: Added a new allocator method to add resources to agents.

[Benjamin Bannier]

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

(Updated Nov. 24, 2017, 3:06 p.m.)


Review request for mesos, Benjamin Mahler and Jie Yu.


Changes
---

Renamed added method.


Summary (updated)
-

Added a new allocator method to add resources to agents.


Repository: mesos


Description (updated)
---

The added method complements 'Allocator::addSlave'. While in
'addSlave' the total agent resources and used resources are passed,
the method 'addResourceProvider' added here allows to add additional,
potentially used resources to an existing agent.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
ae122003487ca8956573e993cd3993aa8cc286f1 
  src/master/allocator/mesos/allocator.hpp 
8fa4fdeec4ec64bcd49fc442c230d8684a11cfd9 
  src/master/allocator/mesos/hierarchical.hpp 
2c4832b29842330fa57756cd3d4202f265a820f3 
  src/master/allocator/mesos/hierarchical.cpp 
cfeeb3bfa3ad7d78ff6e22cfc4adb1f0efa05629 
  src/tests/allocator.hpp 6a84f1beb86dceb5a5e9bf4615c13a216f3d0905 
  src/tests/hierarchical_allocator_tests.cpp 
f0f95ba4f667bf8ea54e985d8cde913a4170d8ff 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63842: Allowed removing non-terminal offer operations.

[Benjamin Bannier]

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

(Updated Nov. 24, 2017, 3:08 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Rebased.


Repository: mesos


Description
---

During reconcilation we might be required to remove non-terminal offer
operations from bookkeeping.


Diffs (updated)
-

  src/master/master.hpp 2a2e830354db4a2191fb8321beb8174b80f7ba7d 
  src/master/master.cpp 53263e499d88b906b6406c24c0dfb737e589e813 
  src/slave/slave.cpp f93ff7b20815c3ccb274ce6990ee66a17b6ac51c 


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

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


Testing
---

`make check`, tested as part of https://reviews.apache.org/r/63843/.

This patch requires `protobuf::isSpeculativeOperation` from 
https://reviews.apache.org/r/63751/ which is _not_ part of this review chain 
(to avoid multiple dependent RRs).


Thanks,

Benjamin Bannier

Re: Review Request 63918: Updated ResourceProviderMessage stringification with recent changes.

[Benjamin Bannier]

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

(Updated Nov. 24, 2017, 3:07 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Rebased.


Repository: mesos


Description
---

Updated ResourceProviderMessage stringification with recent changes.


Diffs (updated)
-

  src/resource_provider/message.hpp 19cf08e99b19d2d5077b3965d154cfd4554326d1 


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

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


Testing (updated)
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63732: Reconciled offer operations between agent and master.

[Benjamin Bannier]

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

(Updated Nov. 24, 2017, 3:07 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Reworked to process resource providers one at a time.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.cpp 53263e499d88b906b6406c24c0dfb737e589e813 


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

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


Testing
---

`make check`, tested as part of https://reviews.apache.org/r/63843/.

This patch requires `protobuf::isSpeculativeOperation` from 
https://reviews.apache.org/r/63751/ which is _not_ part of this review chain 
(to avoid multiple dependent RRs).


Thanks,

Benjamin Bannier

Re: Review Request 63844: Removed acknowledged offer operation status updates.

[Benjamin Bannier]

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

(Updated Nov. 24, 2017, 3:07 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Rebased.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/slave.cpp f93ff7b20815c3ccb274ce6990ee66a17b6ac51c 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 64056: Added a helper to extract consumed resources from offer operations.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On Nov. 24, 2017, 10:31 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64056/
> ---
> 
> (Updated Nov. 24, 2017, 10:31 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch is based on the work done in
> https://reviews.apache.org/r/63104/.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp eff106f0a546f459030d5cf5b42e5eb8d4d9a34c 
>   src/common/protobuf_utils.cpp a5fc960b8387342eba4c66a711fedc92dd014e6f 
> 
> 
> Diff: https://reviews.apache.org/r/64056/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 64033: Terminated executors if kill task received before launch task.

[Andrei Budnik]

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




src/docker/executor.cpp
Lines 386 (patched)


Consider moving this check to [the 
driver](https://github.com/apache/mesos/blob/708902d6f207b6f93c94b3589a31d54c812a7657/src/exec/exec.cpp#L343).


- Andrei Budnik


On Nov. 22, 2017, 5:05 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64033/
> ---
> 
> (Updated Nov. 22, 2017, 5:05 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.
> 
> 
> Bugs: MESOS-8247
> https://issues.apache.org/jira/browse/MESOS-8247
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
> to a driver-based executor. Since these messages are not retried,
> the executor will never start a task and will remain idle, ignoring
> kill task request. This patch shutdown driver-based executors if kill
> task arrives before the task has been started.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 3974f20052e3c12eb154a5146d19d4dc1759859f 
>   src/launcher/executor.cpp c688c04e598ac140421fd5e47359b0e48d30bcc5 
> 
> 
> Diff: https://reviews.apache.org/r/64033/diff/1/
> 
> 
> Testing
> ---
> 
> make check on MacOS 10.11.6
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64054: Refactored offer operation handling for speculative operations.

[Jan Schlicht]

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

(Updated Nov. 24, 2017, 12:24 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Also refactored `addOfferOperation` in `master.cpp`.


Repository: mesos


Description
---

Applying operations has been refactored out of 'addOfferOperation' and
simplified by using 'protobuf::isSpeculativeOperation'.


Diffs (updated)
-

  src/master/master.hpp 2a2e830354db4a2191fb8321beb8174b80f7ba7d 
  src/master/master.cpp 53263e499d88b906b6406c24c0dfb737e589e813 
  src/slave/slave.cpp f93ff7b20815c3ccb274ce6990ee66a17b6ac51c 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 64056: Added a helper to extract consumed resources from offer operations.

[Benjamin Bannier]

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

Review request for mesos, Jie Yu and Jan Schlicht.


Repository: mesos


Description
---

This patch is based on the work done in
https://reviews.apache.org/r/63104/.


Diffs
-

  src/common/protobuf_utils.hpp eff106f0a546f459030d5cf5b42e5eb8d4d9a34c 
  src/common/protobuf_utils.cpp a5fc960b8387342eba4c66a711fedc92dd014e6f 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier