You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2018/11/29 00:00:13 UTC
Review Request 69473: Added heartbeaters for agent and HTTP executors.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69473/
-----------------------------------------------------------
Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
Bugs: MESOS-7564
https://issues.apache.org/jira/browse/MESOS-7564
Repository: mesos
Description
-------
This implements two separate heartbeaters for Executor Events (agent
to executor) and Executor Calls (executor to agent). Both are set to
non-configurable intervals of 30 minutes, which should be sufficient
to keep the connections alive while not flooding logs with warnings
if the executor/agent does not have this patch.
Diffs
-----
src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea
src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b
src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71
src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421
src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1
src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c
src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50
src/slave/slave.cpp 858b78620e1ef33f3587d0bd95a684917aaf5bbb
Diff: https://reviews.apache.org/r/69473/diff/1/
Testing
-------
make
Thanks,
Joseph Wu
Re: Review Request 69473: Added heartbeaters for agent and HTTP
executors.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69473/#review211029
-----------------------------------------------------------
src/slave/slave.cpp
Lines 4679 (patched)
<https://reviews.apache.org/r/69473/#comment295880>
Let's make the heartbeater a wrapping libprocess process class to handle the process lifecycle.
- Greg Mann
On Nov. 29, 2018, midnight, Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69473/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2018, midnight)
>
>
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This implements two separate heartbeaters for Executor Events (agent
> to executor) and Executor Calls (executor to agent). Both are set to
> non-configurable intervals of 30 minutes, which should be sufficient
> to keep the connections alive while not flooding logs with warnings
> if the executor/agent does not have this patch.
>
>
> Diffs
> -----
>
> src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea
> src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b
> src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71
> src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421
> src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1
> src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c
> src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50
> src/slave/slave.cpp 858b78620e1ef33f3587d0bd95a684917aaf5bbb
>
>
> Diff: https://reviews.apache.org/r/69473/diff/1/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 69473: Added heartbeaters for agent and HTTP
executors.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69473/#review210941
-----------------------------------------------------------
src/common/validation.cpp
Lines 605-610 (original), 605-614 (patched)
<https://reviews.apache.org/r/69473/#comment295771>
Note: Lacking this switch case will not cause crashes (`UNREACHABLE`) on older agents/executors because the new HEARTBEAT protobuf will be parsed as the `UNKNOWN` type.
- Joseph Wu
On Nov. 28, 2018, 4 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69473/
> -----------------------------------------------------------
>
> (Updated Nov. 28, 2018, 4 p.m.)
>
>
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This implements two separate heartbeaters for Executor Events (agent
> to executor) and Executor Calls (executor to agent). Both are set to
> non-configurable intervals of 30 minutes, which should be sufficient
> to keep the connections alive while not flooding logs with warnings
> if the executor/agent does not have this patch.
>
>
> Diffs
> -----
>
> src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea
> src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b
> src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71
> src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421
> src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1
> src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c
> src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50
> src/slave/slave.cpp 858b78620e1ef33f3587d0bd95a684917aaf5bbb
>
>
> Diff: https://reviews.apache.org/r/69473/diff/1/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 69473: Added heartbeaters for agent and HTTP
executors.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69473/#review211078
-----------------------------------------------------------
Ship it!
Ship It!
- Greg Mann
On Dec. 6, 2018, 1:24 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69473/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2018, 1:24 a.m.)
>
>
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This implements two separate heartbeaters for Executor Events (agent
> to executor) and Executor Calls (executor to agent). Both are set to
> non-configurable intervals of 30 minutes, which should be sufficient
> to keep the connections alive while not flooding logs with warnings
> if the executor/agent does not have this patch.
>
>
> Diffs
> -----
>
> src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea
> src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b
> src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71
> src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421
> src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1
> src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c
> src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50
> src/slave/slave.cpp 858b78620e1ef33f3587d0bd95a684917aaf5bbb
>
>
> Diff: https://reviews.apache.org/r/69473/diff/3/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 69473: Added heartbeaters for agent and HTTP
executors.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Dec. 10, 2018, 8:22 a.m., Benno Evers wrote:
> > src/executor/executor.cpp
> > Lines 798 (patched)
> > <https://reviews.apache.org/r/69473/diff/4/?file=2112645#file2112645line798>
> >
> > I think you're adding a constant in `src/slave/constants.hpp` in this review, so we could use it here. (or if you want to keep the intervals in both directions separate, let's at least define this as a constant within this file, instead of using a naked value here)
After some consideration, a good middle ground is to pull this value into a constant inside this file. We'll put off creating a new header, as there might be more things we could clean up with a header.
The test will forward-define the constant and link in the value of the constant. Luckily, since this is the executor driver (rather than the default/command executor) the code will be compiled into libmesos and won't require any special build changes.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69473/#review211142
-----------------------------------------------------------
On Dec. 7, 2018, 2:17 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69473/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2018, 2:17 p.m.)
>
>
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This implements two separate heartbeaters for Executor Events (agent
> to executor) and Executor Calls (executor to agent). Both are set to
> non-configurable intervals of 30 minutes, which should be sufficient
> to keep the connections alive while not flooding logs with warnings
> if the executor/agent does not have this patch.
>
>
> Diffs
> -----
>
> src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea
> src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b
> src/executor/v0_v1executor.cpp aebdbe705dac604da44aba484dc4d007e83b8e37
> src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71
> src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421
> src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1
> src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c
> src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50
> src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85
>
>
> Diff: https://reviews.apache.org/r/69473/diff/4/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 69473: Added heartbeaters for agent and HTTP
executors.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Dec. 10, 2018, 8:22 a.m., Benno Evers wrote:
> > src/slave/http.cpp
> > Lines 862 (patched)
> > <https://reviews.apache.org/r/69473/diff/4/?file=2112650#file2112650line862>
> >
> > I wonder: This `switch` statement doesn't have a `default` case, so if a new executor sends a heartbeat to an old agent, wouldn't it skip past the `switch` into the `UNREACHABLE` and kill the agent?
When dealing with protobuf enums in general, the `0` value is special in that any un-mapped types will be parsed as `0`.
So suppose a server knows about an enum like this:
```
enum Type {
UNKNOWN = 0;
ONE = 1;
TWO = 2;
}
```
While the client's enum has an additional row:
```
enum Type {
UNKNOWN = 0;
ONE = 1;
TWO = 2;
THREE = 3;
}
```
When passed over the wire, the client will send a `Type = 3`. The server will parse this as `UNKNOWN`, because this is the default value of an enum.
The documentation does not explicitly spell out this behavior, but it is considered best-practice for making extendable enums via protobuf: https://developers.google.com/protocol-buffers/docs/proto3#enum
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69473/#review211142
-----------------------------------------------------------
On Dec. 7, 2018, 2:17 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69473/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2018, 2:17 p.m.)
>
>
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This implements two separate heartbeaters for Executor Events (agent
> to executor) and Executor Calls (executor to agent). Both are set to
> non-configurable intervals of 30 minutes, which should be sufficient
> to keep the connections alive while not flooding logs with warnings
> if the executor/agent does not have this patch.
>
>
> Diffs
> -----
>
> src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea
> src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b
> src/executor/v0_v1executor.cpp aebdbe705dac604da44aba484dc4d007e83b8e37
> src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71
> src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421
> src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1
> src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c
> src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50
> src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85
>
>
> Diff: https://reviews.apache.org/r/69473/diff/4/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 69473: Added heartbeaters for agent and HTTP
executors.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69473/#review211142
-----------------------------------------------------------
src/executor/executor.cpp
Lines 798 (patched)
<https://reviews.apache.org/r/69473/#comment296069>
I think you're adding a constant in `src/slave/constants.hpp` in this review, so we could use it here. (or if you want to keep the intervals in both directions separate, let's at least define this as a constant within this file, instead of using a naked value here)
src/slave/http.cpp
Lines 862 (patched)
<https://reviews.apache.org/r/69473/#comment296068>
I wonder: This `switch` statement doesn't have a `default` case, so if a new executor sends a heartbeat to an old agent, wouldn't it skip past the `switch` into the `UNREACHABLE` and kill the agent?
- Benno Evers
On Dec. 7, 2018, 10:17 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69473/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2018, 10:17 p.m.)
>
>
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This implements two separate heartbeaters for Executor Events (agent
> to executor) and Executor Calls (executor to agent). Both are set to
> non-configurable intervals of 30 minutes, which should be sufficient
> to keep the connections alive while not flooding logs with warnings
> if the executor/agent does not have this patch.
>
>
> Diffs
> -----
>
> src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea
> src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b
> src/executor/v0_v1executor.cpp aebdbe705dac604da44aba484dc4d007e83b8e37
> src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71
> src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421
> src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1
> src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c
> src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50
> src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85
>
>
> Diff: https://reviews.apache.org/r/69473/diff/4/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 69473: Added heartbeaters for agent and HTTP
executors.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69473/#review211187
-----------------------------------------------------------
Ship it!
Ship It!
- Benno Evers
On Dec. 11, 2018, 1:20 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69473/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2018, 1:20 a.m.)
>
>
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This implements two separate heartbeaters for Executor Events (agent
> to executor) and Executor Calls (executor to agent). Both are set to
> non-configurable intervals of 30 minutes, which should be sufficient
> to keep the connections alive while not flooding logs with warnings
> if the executor/agent does not have this patch.
>
>
> Diffs
> -----
>
> src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea
> src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b
> src/executor/v0_v1executor.cpp aebdbe705dac604da44aba484dc4d007e83b8e37
> src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71
> src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421
> src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1
> src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c
> src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50
> src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85
>
>
> Diff: https://reviews.apache.org/r/69473/diff/5/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 69473: Added heartbeaters for agent and HTTP
executors.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69473/
-----------------------------------------------------------
(Updated Dec. 10, 2018, 5:20 p.m.)
Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
Changes
-------
Moved the 30 Minute executor heartbeat interval into a constant. This is an `extern` so that the test can define and use it too, without a header.
Bugs: MESOS-7564
https://issues.apache.org/jira/browse/MESOS-7564
Repository: mesos
Description
-------
This implements two separate heartbeaters for Executor Events (agent
to executor) and Executor Calls (executor to agent). Both are set to
non-configurable intervals of 30 minutes, which should be sufficient
to keep the connections alive while not flooding logs with warnings
if the executor/agent does not have this patch.
Diffs (updated)
-----
src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea
src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b
src/executor/v0_v1executor.cpp aebdbe705dac604da44aba484dc4d007e83b8e37
src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71
src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421
src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1
src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c
src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50
src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85
Diff: https://reviews.apache.org/r/69473/diff/5/
Changes: https://reviews.apache.org/r/69473/diff/4-5/
Testing
-------
make
Thanks,
Joseph Wu
Re: Review Request 69473: Added heartbeaters for agent and HTTP
executors.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69473/
-----------------------------------------------------------
(Updated Dec. 7, 2018, 2:17 p.m.)
Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
Changes
-------
Dealt with -Werror warning about missing cases in a switch statement.
Bugs: MESOS-7564
https://issues.apache.org/jira/browse/MESOS-7564
Repository: mesos
Description
-------
This implements two separate heartbeaters for Executor Events (agent
to executor) and Executor Calls (executor to agent). Both are set to
non-configurable intervals of 30 minutes, which should be sufficient
to keep the connections alive while not flooding logs with warnings
if the executor/agent does not have this patch.
Diffs (updated)
-----
src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea
src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b
src/executor/v0_v1executor.cpp aebdbe705dac604da44aba484dc4d007e83b8e37
src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71
src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421
src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1
src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c
src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50
src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85
Diff: https://reviews.apache.org/r/69473/diff/4/
Changes: https://reviews.apache.org/r/69473/diff/3-4/
Testing
-------
make
Thanks,
Joseph Wu
Re: Review Request 69473: Added heartbeaters for agent and HTTP
executors.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69473/
-----------------------------------------------------------
(Updated Dec. 5, 2018, 5:24 p.m.)
Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
Changes
-------
Extended a TODO and fixed an incorrect copy of the heartbeater.
Bugs: MESOS-7564
https://issues.apache.org/jira/browse/MESOS-7564
Repository: mesos
Description
-------
This implements two separate heartbeaters for Executor Events (agent
to executor) and Executor Calls (executor to agent). Both are set to
non-configurable intervals of 30 minutes, which should be sufficient
to keep the connections alive while not flooding logs with warnings
if the executor/agent does not have this patch.
Diffs (updated)
-----
src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea
src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b
src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71
src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421
src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1
src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c
src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50
src/slave/slave.cpp 858b78620e1ef33f3587d0bd95a684917aaf5bbb
Diff: https://reviews.apache.org/r/69473/diff/3/
Changes: https://reviews.apache.org/r/69473/diff/2-3/
Testing
-------
make
Thanks,
Joseph Wu
Re: Review Request 69473: Added heartbeaters for agent and HTTP
executors.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Dec. 5, 2018, 4:49 p.m., Greg Mann wrote:
> > src/executor/executor.cpp
> > Lines 797 (patched)
> > <https://reviews.apache.org/r/69473/diff/2/?file=2112292#file2112292line797>
> >
> > Can we break `Minutes(30)` out into a common header somewhere so we can use it in the test as well? If we don't do this now, let's leave a TODO for that as well.
I've opted for the TODO, since we don't have a good header for executor constants right now. There are a few other constants and flags for executors which we could potentially put into a header somewhere. But we can discuss/do that in future.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69473/#review211070
-----------------------------------------------------------
On Dec. 5, 2018, 5:24 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69473/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2018, 5:24 p.m.)
>
>
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This implements two separate heartbeaters for Executor Events (agent
> to executor) and Executor Calls (executor to agent). Both are set to
> non-configurable intervals of 30 minutes, which should be sufficient
> to keep the connections alive while not flooding logs with warnings
> if the executor/agent does not have this patch.
>
>
> Diffs
> -----
>
> src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea
> src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b
> src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71
> src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421
> src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1
> src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c
> src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50
> src/slave/slave.cpp 858b78620e1ef33f3587d0bd95a684917aaf5bbb
>
>
> Diff: https://reviews.apache.org/r/69473/diff/3/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 69473: Added heartbeaters for agent and HTTP
executors.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69473/#review211070
-----------------------------------------------------------
src/executor/executor.cpp
Lines 797 (patched)
<https://reviews.apache.org/r/69473/#comment295941>
Can we break `Minutes(30)` out into a common header somewhere so we can use it in the test as well? If we don't do this now, let's leave a TODO for that as well.
- Greg Mann
On Dec. 6, 2018, 12:24 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69473/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2018, 12:24 a.m.)
>
>
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This implements two separate heartbeaters for Executor Events (agent
> to executor) and Executor Calls (executor to agent). Both are set to
> non-configurable intervals of 30 minutes, which should be sufficient
> to keep the connections alive while not flooding logs with warnings
> if the executor/agent does not have this patch.
>
>
> Diffs
> -----
>
> src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea
> src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b
> src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71
> src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421
> src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1
> src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c
> src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50
> src/slave/slave.cpp 858b78620e1ef33f3587d0bd95a684917aaf5bbb
>
>
> Diff: https://reviews.apache.org/r/69473/diff/2/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 69473: Added heartbeaters for agent and HTTP
executors.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69473/
-----------------------------------------------------------
(Updated Dec. 5, 2018, 4:24 p.m.)
Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
Changes
-------
Updated heartbeater construction to match prior review changes.
Bugs: MESOS-7564
https://issues.apache.org/jira/browse/MESOS-7564
Repository: mesos
Description
-------
This implements two separate heartbeaters for Executor Events (agent
to executor) and Executor Calls (executor to agent). Both are set to
non-configurable intervals of 30 minutes, which should be sufficient
to keep the connections alive while not flooding logs with warnings
if the executor/agent does not have this patch.
Diffs (updated)
-----
src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea
src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b
src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71
src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421
src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1
src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c
src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50
src/slave/slave.cpp 858b78620e1ef33f3587d0bd95a684917aaf5bbb
Diff: https://reviews.apache.org/r/69473/diff/2/
Changes: https://reviews.apache.org/r/69473/diff/1-2/
Testing
-------
make
Thanks,
Joseph Wu