You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2017/02/02 09:58:08 UTC
Review Request 56215: Reused previous task status to generate a new
one in default executor.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56215/
-----------------------------------------------------------
Review request for mesos, Gast�n Kleiman and Vinod Kone.
Bugs: MESOS-6906
https://issues.apache.org/jira/browse/MESOS-6906
Repository: mesos
Description
-------
When a new task status update is generated in the executor, we have
to make sure specific data is duplicated from the previous update
to, e.g., avoid shadowing of those data during reconciliation. For
instance, consider a check status being sent; in this status update
we must include the latest known health information.
Diffs
-----
src/launcher/default_executor.cpp 97eee05cac8cb1f62d43e2aecc08a8e54e49eac3
Diff: https://reviews.apache.org/r/56215/diff/
Testing
-------
See https://reviews.apache.org/r/56218/
Thanks,
Alexander Rukletsov
Re: Review Request 56215: Reused previous task status to generate a
new one in default executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On March 17, 2017, 10:44 a.m., Vinod Kone wrote:
> > src/launcher/default_executor.cpp
> > Line 910 (original), 921 (patched)
> > <https://reviews.apache.org/r/56215/diff/6/?file=1666160#file1666160line923>
> >
> > is this safe because before `taskHealthUpdated` is called we would've already sent a TASK_RUNNING update?
Yes, we always send a TASK_RUNNING before even starting a healh check.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56215/#review169271
-----------------------------------------------------------
On March 16, 2017, 4:45 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56215/
> -----------------------------------------------------------
>
> (Updated March 16, 2017, 4:45 p.m.)
>
>
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
>
>
> Bugs: MESOS-7249
> https://issues.apache.org/jira/browse/MESOS-7249
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Sometimes when a new task status update is generated in the executor,
> we have to make sure specific data is duplicated from the previous
> task status to, e.g., avoid shadowing of these data during
> reconciliation. For instance, consider a check status being sent;
> in this status update we must include the latest known health
> information.
>
> This patch also refactors `update()` routine into two separate calls:
> `createTaskStatus()` which is responsible for creating a task status
> from scratch and `forward()`, which is responsible for forwarding task
> status updates to the agent.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp cbd4f7ecd042e7fa603bd69774d95472df2c896d
>
>
> Diff: https://reviews.apache.org/r/56215/diff/6/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/56218/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 56215: Reused previous task status to generate a
new one in default executor.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56215/#review169271
-----------------------------------------------------------
Ship it!
src/launcher/default_executor.cpp
Line 910 (original), 921 (patched)
<https://reviews.apache.org/r/56215/#comment241620>
is this safe because before `taskHealthUpdated` is called we would've already sent a TASK_RUNNING update?
- Vinod Kone
On March 16, 2017, 4:45 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56215/
> -----------------------------------------------------------
>
> (Updated March 16, 2017, 4:45 p.m.)
>
>
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
>
>
> Bugs: MESOS-7249
> https://issues.apache.org/jira/browse/MESOS-7249
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Sometimes when a new task status update is generated in the executor,
> we have to make sure specific data is duplicated from the previous
> task status to, e.g., avoid shadowing of these data during
> reconciliation. For instance, consider a check status being sent;
> in this status update we must include the latest known health
> information.
>
> This patch also refactors `update()` routine into two separate calls:
> `createTaskStatus()` which is responsible for creating a task status
> from scratch and `forward()`, which is responsible for forwarding task
> status updates to the agent.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp cbd4f7ecd042e7fa603bd69774d95472df2c896d
>
>
> Diff: https://reviews.apache.org/r/56215/diff/6/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/56218/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 56215: Reused previous task status to generate a
new one in default executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56215/
-----------------------------------------------------------
(Updated March 23, 2017, 12:10 p.m.)
Review request for mesos, Gast�n Kleiman and Vinod Kone.
Changes
-------
Rebased. NNTR.
Bugs: MESOS-7249
https://issues.apache.org/jira/browse/MESOS-7249
Repository: mesos
Description
-------
Sometimes when a new task status update is generated in the executor,
we have to make sure specific data is duplicated from the previous
task status to, e.g., avoid shadowing of these data during
reconciliation. For instance, consider a check status being sent;
in this status update we must include the latest known health
information.
This patch also refactors `update()` routine into two separate calls:
`createTaskStatus()` which is responsible for creating a task status
from scratch and `forward()`, which is responsible for forwarding task
status updates to the agent.
Diffs (updated)
-----
src/launcher/default_executor.cpp 6a885af50db6d489194cc9b780fcf1494e853694
Diff: https://reviews.apache.org/r/56215/diff/8/
Changes: https://reviews.apache.org/r/56215/diff/7-8/
Testing
-------
See https://reviews.apache.org/r/56218/
Thanks,
Alexander Rukletsov
Re: Review Request 56215: Reused previous task status to generate a
new one in default executor.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56215/#review169571
-----------------------------------------------------------
Ship it!
Ship It!
- Gast�n Kleiman
On March 20, 2017, 11:55 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56215/
> -----------------------------------------------------------
>
> (Updated March 20, 2017, 11:55 a.m.)
>
>
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
>
>
> Bugs: MESOS-7249
> https://issues.apache.org/jira/browse/MESOS-7249
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Sometimes when a new task status update is generated in the executor,
> we have to make sure specific data is duplicated from the previous
> task status to, e.g., avoid shadowing of these data during
> reconciliation. For instance, consider a check status being sent;
> in this status update we must include the latest known health
> information.
>
> This patch also refactors `update()` routine into two separate calls:
> `createTaskStatus()` which is responsible for creating a task status
> from scratch and `forward()`, which is responsible for forwarding task
> status updates to the agent.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp cbd4f7ecd042e7fa603bd69774d95472df2c896d
>
>
> Diff: https://reviews.apache.org/r/56215/diff/7/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/56218/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 56215: Reused previous task status to generate a
new one in default executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56215/
-----------------------------------------------------------
(Updated March 20, 2017, 11:55 a.m.)
Review request for mesos, Gast�n Kleiman and Vinod Kone.
Bugs: MESOS-7249
https://issues.apache.org/jira/browse/MESOS-7249
Repository: mesos
Description
-------
Sometimes when a new task status update is generated in the executor,
we have to make sure specific data is duplicated from the previous
task status to, e.g., avoid shadowing of these data during
reconciliation. For instance, consider a check status being sent;
in this status update we must include the latest known health
information.
This patch also refactors `update()` routine into two separate calls:
`createTaskStatus()` which is responsible for creating a task status
from scratch and `forward()`, which is responsible for forwarding task
status updates to the agent.
Diffs (updated)
-----
src/launcher/default_executor.cpp cbd4f7ecd042e7fa603bd69774d95472df2c896d
Diff: https://reviews.apache.org/r/56215/diff/7/
Changes: https://reviews.apache.org/r/56215/diff/6-7/
Testing
-------
See https://reviews.apache.org/r/56218/
Thanks,
Alexander Rukletsov
Re: Review Request 56215: Reused previous task status to generate a
new one in default executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56215/
-----------------------------------------------------------
(Updated March 16, 2017, 4:45 p.m.)
Review request for mesos, Gast�n Kleiman and Vinod Kone.
Bugs: MESOS-7249
https://issues.apache.org/jira/browse/MESOS-7249
Repository: mesos
Description (updated)
-------
Sometimes when a new task status update is generated in the executor,
we have to make sure specific data is duplicated from the previous
task status to, e.g., avoid shadowing of these data during
reconciliation. For instance, consider a check status being sent;
in this status update we must include the latest known health
information.
This patch also refactors `update()` routine into two separate calls:
`createTaskStatus()` which is responsible for creating a task status
from scratch and `forward()`, which is responsible for forwarding task
status updates to the agent.
Diffs (updated)
-----
src/launcher/default_executor.cpp cbd4f7ecd042e7fa603bd69774d95472df2c896d
Diff: https://reviews.apache.org/r/56215/diff/6/
Changes: https://reviews.apache.org/r/56215/diff/5-6/
Testing
-------
See https://reviews.apache.org/r/56218/
Thanks,
Alexander Rukletsov
Re: Review Request 56215: Reused previous task status to generate a
new one in default executor.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56215/#review165696
-----------------------------------------------------------
src/launcher/default_executor.cpp (lines 943 - 970)
<https://reviews.apache.org/r/56215/#comment237558>
I am not a big fan of adding this helper in all the executors, I'd rather put https://reviews.apache.org/r/56017/ earlier in the chain and use that.
src/launcher/default_executor.cpp (line 985)
<https://reviews.apache.org/r/56215/#comment237559>
If we keep this as-is, then I'd replace `Rewrite` with `Update` or `Overwrite`.
- Gast�n Kleiman
On Feb. 9, 2017, 12:56 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56215/
> -----------------------------------------------------------
>
> (Updated Feb. 9, 2017, 12:56 p.m.)
>
>
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
>
>
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When a new task status update is generated in the executor, we have
> to make sure specific data is duplicated from the previous update
> to, e.g., avoid shadowing of those data during reconciliation. For
> instance, consider a check status being sent; in this status update
> we must include the latest known health information.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp e63cf153831088851863d0956455a024e9bc172a
>
> Diff: https://reviews.apache.org/r/56215/diff/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/56218/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 56215: Reused previous task status to generate a
new one in default executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56215/
-----------------------------------------------------------
(Updated Feb. 9, 2017, 12:56 p.m.)
Review request for mesos, Gast�n Kleiman and Vinod Kone.
Bugs: MESOS-6906
https://issues.apache.org/jira/browse/MESOS-6906
Repository: mesos
Description
-------
When a new task status update is generated in the executor, we have
to make sure specific data is duplicated from the previous update
to, e.g., avoid shadowing of those data during reconciliation. For
instance, consider a check status being sent; in this status update
we must include the latest known health information.
Diffs (updated)
-----
src/launcher/default_executor.cpp e63cf153831088851863d0956455a024e9bc172a
Diff: https://reviews.apache.org/r/56215/diff/
Testing
-------
See https://reviews.apache.org/r/56218/
Thanks,
Alexander Rukletsov
Re: Review Request 56215: Reused previous task status to generate a
new one in default executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56215/
-----------------------------------------------------------
(Updated Feb. 8, 2017, 9:06 p.m.)
Review request for mesos, Gast�n Kleiman and Vinod Kone.
Changes
-------
Fixed bad rebase.
Bugs: MESOS-6906
https://issues.apache.org/jira/browse/MESOS-6906
Repository: mesos
Description
-------
When a new task status update is generated in the executor, we have
to make sure specific data is duplicated from the previous update
to, e.g., avoid shadowing of those data during reconciliation. For
instance, consider a check status being sent; in this status update
we must include the latest known health information.
Diffs (updated)
-----
src/launcher/default_executor.cpp e63cf153831088851863d0956455a024e9bc172a
Diff: https://reviews.apache.org/r/56215/diff/
Testing
-------
See https://reviews.apache.org/r/56218/
Thanks,
Alexander Rukletsov
Re: Review Request 56215: Reused previous task status to generate a
new one in default executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56215/
-----------------------------------------------------------
(Updated Feb. 8, 2017, 4:57 p.m.)
Review request for mesos, Gast�n Kleiman and Vinod Kone.
Bugs: MESOS-6906
https://issues.apache.org/jira/browse/MESOS-6906
Repository: mesos
Description
-------
When a new task status update is generated in the executor, we have
to make sure specific data is duplicated from the previous update
to, e.g., avoid shadowing of those data during reconciliation. For
instance, consider a check status being sent; in this status update
we must include the latest known health information.
Diffs (updated)
-----
src/launcher/default_executor.cpp e63cf153831088851863d0956455a024e9bc172a
Diff: https://reviews.apache.org/r/56215/diff/
Testing
-------
See https://reviews.apache.org/r/56218/
Thanks,
Alexander Rukletsov
Re: Review Request 56215: Reused previous task status to generate a
new one in default executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56215/
-----------------------------------------------------------
(Updated Feb. 8, 2017, 3:21 p.m.)
Review request for mesos, Gast�n Kleiman and Vinod Kone.
Bugs: MESOS-6906
https://issues.apache.org/jira/browse/MESOS-6906
Repository: mesos
Description
-------
When a new task status update is generated in the executor, we have
to make sure specific data is duplicated from the previous update
to, e.g., avoid shadowing of those data during reconciliation. For
instance, consider a check status being sent; in this status update
we must include the latest known health information.
Diffs (updated)
-----
src/launcher/default_executor.cpp e63cf153831088851863d0956455a024e9bc172a
Diff: https://reviews.apache.org/r/56215/diff/
Testing
-------
See https://reviews.apache.org/r/56218/
Thanks,
Alexander Rukletsov