You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2019/01/07 12:25:07 UTC
Review Request 69680: Have master acknowledge operation updates of
completed frameworks.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69680/
-----------------------------------------------------------
Review request for mesos, Gastón Kleiman and Greg Mann.
Bugs: MESOS-9434
https://issues.apache.org/jira/browse/MESOS-9434
Repository: mesos
Description
-------
After a framework was removed and has unacknowledged operations status
updates, it was impossible to remove terminal operations as nobody could
acknowledge them.
In this patch we make the master acknowledge operation status updates
for frameworks it knows are removed so that e.g., terminal operations
can be removed. Since masters do not persist completed frameworks this
is not reliable (e.g., an agent was partitioned for a long time and
still tracks a completed framework's `FrameworkInfo`, and comes back
only after the master knowing about the framework's completion has
failed over). We merely extend the existing master behavior (e.g., send
`ShutdownFrameworkMessage` to all currently registered agents) to
operations.
Diffs
-----
src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
src/tests/master_tests.cpp 51fcbe8d72fce1e56790d22ab7deced4fdb3bf0e
Diff: https://reviews.apache.org/r/69680/diff/1/
Testing
-------
* `make check`
* tested on a number of configurations in internal CI
* ran added test in repetition, both with and without additional stress
Thanks,
Benjamin Bannier
Re: Review Request 69680: Have master acknowledge operation updates of
completed frameworks.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On Jan. 28, 2019, 9:37 p.m., Greg Mann wrote:
> > src/tests/master_tests.cpp
> > Lines 9469-9491 (patched)
> > <https://reviews.apache.org/r/69680/diff/1/?file=2118289#file2118289line9469>
> >
> > Can you use the mock RP's `operationDefault` method to accomplish this?
> >
> > https://github.com/apache/mesos/blob/d838f2958e18c1a75594ca4f10df132670fcd11e/src/tests/master_tests.cpp#L9200-L9201
This mock method needed to send a update UUID which I added in https://reviews.apache.org/r/69854/.
I also tried to make this us a non-speculated operation like `CREATE_DISK` instead of `RESERVE` which caused me to run into https://issues.apache.org/jira/browse/MESOS-9542.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69680/#review212392
-----------------------------------------------------------
On Jan. 29, 2019, 2:21 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69680/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2019, 2:21 p.m.)
>
>
> Review request for mesos, Gastón Kleiman and Greg Mann.
>
>
> Bugs: MESOS-9434
> https://issues.apache.org/jira/browse/MESOS-9434
>
>
> Repository: mesos
>
>
> Description
> -------
>
> After a framework was removed and has unacknowledged operations status
> updates, it was impossible to remove terminal operations as nobody could
> acknowledge them.
>
> In this patch we make the master acknowledge operation status updates
> for frameworks it knows are removed so that e.g., terminal operations
> can be removed. Since masters do not persist completed frameworks this
> is not reliable (e.g., an agent was partitioned for a long time and
> still tracks a completed framework's `FrameworkInfo`, and comes back
> only after the master knowing about the framework's completion has
> failed over). We merely extend the existing master behavior (e.g., send
> `ShutdownFrameworkMessage` to all currently registered agents) to
> operations.
>
>
> Diffs
> -----
>
> src/master/master.cpp 2e0a0de4d745112011370741b8cedbb6db17b915
> src/tests/master_tests.cpp 1f8da63aa313c4679c0d7a3934802af1474e8f28
>
>
> Diff: https://reviews.apache.org/r/69680/diff/2/
>
>
> Testing
> -------
>
> * `make check`
> * tested on a number of configurations in internal CI
> * ran added test in repetition, both with and without additional stress
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 69680: Have master acknowledge operation updates of
completed frameworks.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69680/#review212392
-----------------------------------------------------------
Fix it, then Ship it!
src/master/master.cpp
Line 8623 (original), 8623 (patched)
<https://reviews.apache.org/r/69680/#comment298153>
For the sake of readability, I have a slight preference for:
```
bool frameworkWillAcknowledge =
operation->info().has_id() &&
!isCompletedFramework(frameworkId.get());
```
but not a big deal, feel free to drop this one if you disagree.
src/tests/master_tests.cpp
Lines 9330-9331 (patched)
<https://reviews.apache.org/r/69680/#comment298154>
I might reword this comment as follows:
"Tests that the master acknowledges reliably-sent operation status updates when the framework that requested feedback for that operation has been torn down."
src/tests/master_tests.cpp
Lines 9336 (patched)
<https://reviews.apache.org/r/69680/#comment298155>
I think this may not be necessary? `StartMaster()` should call it under the hood when no flags are passed.
src/tests/master_tests.cpp
Lines 9453-9454 (patched)
<https://reviews.apache.org/r/69680/#comment298156>
I would move this comment above the `mesos.reset();` line.
src/tests/master_tests.cpp
Lines 9469-9491 (patched)
<https://reviews.apache.org/r/69680/#comment298157>
Can you use the mock RP's `operationDefault` method to accomplish this?
https://github.com/apache/mesos/blob/d838f2958e18c1a75594ca4f10df132670fcd11e/src/tests/master_tests.cpp#L9200-L9201
- Greg Mann
On Jan. 7, 2019, 12:25 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69680/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2019, 12:25 p.m.)
>
>
> Review request for mesos, Gastón Kleiman and Greg Mann.
>
>
> Bugs: MESOS-9434
> https://issues.apache.org/jira/browse/MESOS-9434
>
>
> Repository: mesos
>
>
> Description
> -------
>
> After a framework was removed and has unacknowledged operations status
> updates, it was impossible to remove terminal operations as nobody could
> acknowledge them.
>
> In this patch we make the master acknowledge operation status updates
> for frameworks it knows are removed so that e.g., terminal operations
> can be removed. Since masters do not persist completed frameworks this
> is not reliable (e.g., an agent was partitioned for a long time and
> still tracks a completed framework's `FrameworkInfo`, and comes back
> only after the master knowing about the framework's completion has
> failed over). We merely extend the existing master behavior (e.g., send
> `ShutdownFrameworkMessage` to all currently registered agents) to
> operations.
>
>
> Diffs
> -----
>
> src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
> src/tests/master_tests.cpp 51fcbe8d72fce1e56790d22ab7deced4fdb3bf0e
>
>
> Diff: https://reviews.apache.org/r/69680/diff/1/
>
>
> Testing
> -------
>
> * `make check`
> * tested on a number of configurations in internal CI
> * ran added test in repetition, both with and without additional stress
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 69680: Have master acknowledge operation updates of
completed frameworks.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69680/#review211791
-----------------------------------------------------------
PASS: Mesos patch 69680 was successfully built and tested.
Reviews applied: `['69680']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2727/mesos-review-69680
- Mesos Reviewbot Windows
On Jan. 7, 2019, 12:25 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69680/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2019, 12:25 p.m.)
>
>
> Review request for mesos, Gastón Kleiman and Greg Mann.
>
>
> Bugs: MESOS-9434
> https://issues.apache.org/jira/browse/MESOS-9434
>
>
> Repository: mesos
>
>
> Description
> -------
>
> After a framework was removed and has unacknowledged operations status
> updates, it was impossible to remove terminal operations as nobody could
> acknowledge them.
>
> In this patch we make the master acknowledge operation status updates
> for frameworks it knows are removed so that e.g., terminal operations
> can be removed. Since masters do not persist completed frameworks this
> is not reliable (e.g., an agent was partitioned for a long time and
> still tracks a completed framework's `FrameworkInfo`, and comes back
> only after the master knowing about the framework's completion has
> failed over). We merely extend the existing master behavior (e.g., send
> `ShutdownFrameworkMessage` to all currently registered agents) to
> operations.
>
>
> Diffs
> -----
>
> src/master/master.cpp b4faf2b077a0288ba36195b7a21402932489d316
> src/tests/master_tests.cpp 51fcbe8d72fce1e56790d22ab7deced4fdb3bf0e
>
>
> Diff: https://reviews.apache.org/r/69680/diff/1/
>
>
> Testing
> -------
>
> * `make check`
> * tested on a number of configurations in internal CI
> * ran added test in repetition, both with and without additional stress
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 69680: Have master acknowledge operation updates of
completed frameworks.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69680/#review212486
-----------------------------------------------------------
Fix it, then Ship it!
src/tests/master_tests.cpp
Line 9331 (original), 9334 (patched)
<https://reviews.apache.org/r/69680/#comment298246>
s/for that//
- Greg Mann
On Jan. 29, 2019, 1:21 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69680/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2019, 1:21 p.m.)
>
>
> Review request for mesos, Gastón Kleiman and Greg Mann.
>
>
> Bugs: MESOS-9434
> https://issues.apache.org/jira/browse/MESOS-9434
>
>
> Repository: mesos
>
>
> Description
> -------
>
> After a framework was removed and has unacknowledged operations status
> updates, it was impossible to remove terminal operations as nobody could
> acknowledge them.
>
> In this patch we make the master acknowledge operation status updates
> for frameworks it knows are removed so that e.g., terminal operations
> can be removed. Since masters do not persist completed frameworks this
> is not reliable (e.g., an agent was partitioned for a long time and
> still tracks a completed framework's `FrameworkInfo`, and comes back
> only after the master knowing about the framework's completion has
> failed over). We merely extend the existing master behavior (e.g., send
> `ShutdownFrameworkMessage` to all currently registered agents) to
> operations.
>
>
> Diffs
> -----
>
> src/master/master.cpp 2e0a0de4d745112011370741b8cedbb6db17b915
> src/tests/master_tests.cpp 1f8da63aa313c4679c0d7a3934802af1474e8f28
>
>
> Diff: https://reviews.apache.org/r/69680/diff/2/
>
>
> Testing
> -------
>
> * `make check`
> * tested on a number of configurations in internal CI
> * ran added test in repetition, both with and without additional stress
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 69680: Have master acknowledge operation updates of
completed frameworks.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69680/#review212565
-----------------------------------------------------------
PASS: Mesos patch 69680 was successfully built and tested.
Reviews applied: `['69854', '69680']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2853/mesos-review-69680
- Mesos Reviewbot Windows
On Feb. 5, 2019, 5:02 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69680/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2019, 5:02 p.m.)
>
>
> Review request for mesos, Gastón Kleiman and Greg Mann.
>
>
> Bugs: MESOS-9434
> https://issues.apache.org/jira/browse/MESOS-9434
>
>
> Repository: mesos
>
>
> Description
> -------
>
> After a framework was removed and has unacknowledged operations status
> updates, it was impossible to remove terminal operations as nobody could
> acknowledge them.
>
> In this patch we make the master acknowledge operation status updates
> for frameworks it knows are removed so that e.g., terminal operations
> can be removed. Since masters do not persist completed frameworks this
> is not reliable (e.g., an agent was partitioned for a long time and
> still tracks a completed framework's `FrameworkInfo`, and comes back
> only after the master knowing about the framework's completion has
> failed over). We merely extend the existing master behavior (e.g., send
> `ShutdownFrameworkMessage` to all currently registered agents) to
> operations.
>
>
> Diffs
> -----
>
> src/master/master.cpp f74b7c280569e1c24e0940463bb28bd795d429d5
> src/tests/master_tests.cpp acc6096239e4992bdca084d88880d644ab4a2385
>
>
> Diff: https://reviews.apache.org/r/69680/diff/3/
>
>
> Testing
> -------
>
> * `make check`
> * tested on a number of configurations in internal CI
> * ran added test in repetition, both with and without additional stress
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 69680: Have master acknowledge operation updates of
completed frameworks.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69680/
-----------------------------------------------------------
(Updated Feb. 5, 2019, 6:02 p.m.)
Review request for mesos, Gastón Kleiman and Greg Mann.
Bugs: MESOS-9434
https://issues.apache.org/jira/browse/MESOS-9434
Repository: mesos
Description
-------
After a framework was removed and has unacknowledged operations status
updates, it was impossible to remove terminal operations as nobody could
acknowledge them.
In this patch we make the master acknowledge operation status updates
for frameworks it knows are removed so that e.g., terminal operations
can be removed. Since masters do not persist completed frameworks this
is not reliable (e.g., an agent was partitioned for a long time and
still tracks a completed framework's `FrameworkInfo`, and comes back
only after the master knowing about the framework's completion has
failed over). We merely extend the existing master behavior (e.g., send
`ShutdownFrameworkMessage` to all currently registered agents) to
operations.
Diffs
-----
src/master/master.cpp f74b7c280569e1c24e0940463bb28bd795d429d5
src/tests/master_tests.cpp acc6096239e4992bdca084d88880d644ab4a2385
Diff: https://reviews.apache.org/r/69680/diff/3/
Testing
-------
* `make check`
* tested on a number of configurations in internal CI
* ran added test in repetition, both with and without additional stress
Thanks,
Benjamin Bannier
Re: Review Request 69680: Have master acknowledge operation updates of
completed frameworks.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69680/
-----------------------------------------------------------
(Updated Feb. 5, 2019, 5:40 p.m.)
Review request for mesos, Gastón Kleiman and Greg Mann.
Changes
-------
Fixerize comment as suggested by Greg
Bugs: MESOS-9434
https://issues.apache.org/jira/browse/MESOS-9434
Repository: mesos
Description
-------
After a framework was removed and has unacknowledged operations status
updates, it was impossible to remove terminal operations as nobody could
acknowledge them.
In this patch we make the master acknowledge operation status updates
for frameworks it knows are removed so that e.g., terminal operations
can be removed. Since masters do not persist completed frameworks this
is not reliable (e.g., an agent was partitioned for a long time and
still tracks a completed framework's `FrameworkInfo`, and comes back
only after the master knowing about the framework's completion has
failed over). We merely extend the existing master behavior (e.g., send
`ShutdownFrameworkMessage` to all currently registered agents) to
operations.
Diffs (updated)
-----
src/master/master.cpp f74b7c280569e1c24e0940463bb28bd795d429d5
src/tests/master_tests.cpp acc6096239e4992bdca084d88880d644ab4a2385
Diff: https://reviews.apache.org/r/69680/diff/3/
Changes: https://reviews.apache.org/r/69680/diff/2-3/
Testing
-------
* `make check`
* tested on a number of configurations in internal CI
* ran added test in repetition, both with and without additional stress
Thanks,
Benjamin Bannier
Re: Review Request 69680: Have master acknowledge operation updates of
completed frameworks.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69680/#review212413
-----------------------------------------------------------
PASS: Mesos patch 69680 was successfully built and tested.
Reviews applied: `['69680']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2825/mesos-review-69680
- Mesos Reviewbot Windows
On Jan. 29, 2019, 1:21 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69680/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2019, 1:21 p.m.)
>
>
> Review request for mesos, Gastón Kleiman and Greg Mann.
>
>
> Bugs: MESOS-9434
> https://issues.apache.org/jira/browse/MESOS-9434
>
>
> Repository: mesos
>
>
> Description
> -------
>
> After a framework was removed and has unacknowledged operations status
> updates, it was impossible to remove terminal operations as nobody could
> acknowledge them.
>
> In this patch we make the master acknowledge operation status updates
> for frameworks it knows are removed so that e.g., terminal operations
> can be removed. Since masters do not persist completed frameworks this
> is not reliable (e.g., an agent was partitioned for a long time and
> still tracks a completed framework's `FrameworkInfo`, and comes back
> only after the master knowing about the framework's completion has
> failed over). We merely extend the existing master behavior (e.g., send
> `ShutdownFrameworkMessage` to all currently registered agents) to
> operations.
>
>
> Diffs
> -----
>
> src/master/master.cpp 2e0a0de4d745112011370741b8cedbb6db17b915
> src/tests/master_tests.cpp 1f8da63aa313c4679c0d7a3934802af1474e8f28
>
>
> Diff: https://reviews.apache.org/r/69680/diff/2/
>
>
> Testing
> -------
>
> * `make check`
> * tested on a number of configurations in internal CI
> * ran added test in repetition, both with and without additional stress
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 69680: Have master acknowledge operation updates of
completed frameworks.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69680/
-----------------------------------------------------------
(Updated Jan. 29, 2019, 2:21 p.m.)
Review request for mesos, Gastón Kleiman and Greg Mann.
Changes
-------
Addressed Greg's comments.
Bugs: MESOS-9434
https://issues.apache.org/jira/browse/MESOS-9434
Repository: mesos
Description
-------
After a framework was removed and has unacknowledged operations status
updates, it was impossible to remove terminal operations as nobody could
acknowledge them.
In this patch we make the master acknowledge operation status updates
for frameworks it knows are removed so that e.g., terminal operations
can be removed. Since masters do not persist completed frameworks this
is not reliable (e.g., an agent was partitioned for a long time and
still tracks a completed framework's `FrameworkInfo`, and comes back
only after the master knowing about the framework's completion has
failed over). We merely extend the existing master behavior (e.g., send
`ShutdownFrameworkMessage` to all currently registered agents) to
operations.
Diffs (updated)
-----
src/master/master.cpp 2e0a0de4d745112011370741b8cedbb6db17b915
src/tests/master_tests.cpp 1f8da63aa313c4679c0d7a3934802af1474e8f28
Diff: https://reviews.apache.org/r/69680/diff/2/
Changes: https://reviews.apache.org/r/69680/diff/1-2/
Testing
-------
* `make check`
* tested on a number of configurations in internal CI
* ran added test in repetition, both with and without additional stress
Thanks,
Benjamin Bannier