You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by vi...@apache.org on 2016/07/05 22:06:16 UTC

[3/4] mesos git commit: Added filtering for orphaned tasks in /state endpoint.

Added filtering for orphaned tasks in /state endpoint.

Review: https://reviews.apache.org/r/49609/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0cb4533b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0cb4533b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0cb4533b

Branch: refs/heads/master
Commit: 0cb4533b9b31e1b2a3fa52bc421757bac70d9d9c
Parents: 3c96ca4
Author: Joerg Schad <jo...@mesosphere.io>
Authored: Tue Jul 5 17:05:36 2016 -0500
Committer: Vinod Kone <vi...@gmail.com>
Committed: Tue Jul 5 17:06:05 2016 -0500

----------------------------------------------------------------------
 src/master/http.cpp | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/0cb4533b/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 6b4f85b..debedd4 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -2498,11 +2498,8 @@ Future<Response> Master::Http::state(
             });
 
         // Model all of the orphan tasks.
-        // TODO(vinod): Need to filter these tasks based on authorization! This
-        // is currently not possible because we don't have `FrameworkInfo` for
-        // these tasks. We need to either store `FrameworkInfo` for orphan
-        // tasks or persist FrameworkInfo of all frameworks in the registry.
-        writer->field("orphan_tasks", [this](JSON::ArrayWriter* writer) {
+        writer->field("orphan_tasks", [this, &tasksApprover](
+            JSON::ArrayWriter* writer) {
           // Find those orphan tasks.
           foreachvalue (const Slave* slave, master->slaves.registered) {
             typedef hashmap<TaskID, Task*> TaskMap;
@@ -2511,6 +2508,16 @@ Future<Response> Master::Http::state(
                 CHECK_NOTNULL(task);
                 if (!master->frameworks.registered.contains(
                     task->framework_id())) {
+                  CHECK(master->frameworks.recovered.contains(
+                      task->framework_id()));
+
+                  if (!approveViewTask(
+                      tasksApprover,
+                      *task,
+                      master->frameworks.recovered[task->framework_id()])) {
+                    continue;
+                  }
+
                   writer->element(*task);
                 }
               }


Re: [3/4] mesos git commit: Added filtering for orphaned tasks in /state endpoint.

Posted by haosdent <ha...@gmail.com>.
Hi, @neilc Thank you for pop up this. As we discussed in slack, @Joerg is
going to remove this check and add it back after the deprecation cycle.

On Wed, Jul 6, 2016 at 4:24 PM, Neil Conway <ne...@gmail.com> wrote:

> On Wed, Jul 6, 2016 at 12:06 AM,  <vi...@apache.org> wrote:
> > diff --git a/src/master/http.cpp b/src/master/http.cpp
> > index 6b4f85b..debedd4 100644
> > --- a/src/master/http.cpp
> > +++ b/src/master/http.cpp
> > @@ -2498,11 +2498,8 @@ Future<Response> Master::Http::state(
> >              });
> >
> >          // Model all of the orphan tasks.
> > -        // TODO(vinod): Need to filter these tasks based on
> authorization! This
> > -        // is currently not possible because we don't have
> `FrameworkInfo` for
> > -        // these tasks. We need to either store `FrameworkInfo` for
> orphan
> > -        // tasks or persist FrameworkInfo of all frameworks in the
> registry.
> > -        writer->field("orphan_tasks", [this](JSON::ArrayWriter* writer)
> {
> > +        writer->field("orphan_tasks", [this, &tasksApprover](
> > +            JSON::ArrayWriter* writer) {
> >            // Find those orphan tasks.
> >            foreachvalue (const Slave* slave, master->slaves.registered) {
> >              typedef hashmap<TaskID, Task*> TaskMap;
> > @@ -2511,6 +2508,16 @@ Future<Response> Master::Http::state(
> >                  CHECK_NOTNULL(task);
> >                  if (!master->frameworks.registered.contains(
> >                      task->framework_id())) {
> > +                  CHECK(master->frameworks.recovered.contains(
> > +                      task->framework_id()));
>
> This CHECK seems dubious: what if the orphaned task was running on an
> old version of the agent? i.e., a mixed cluster in which the master
> has been updated but the agent has not been.
>
> Neil
>



-- 
Best Regards,
Haosdent Huang

Re: [3/4] mesos git commit: Added filtering for orphaned tasks in /state endpoint.

Posted by Neil Conway <ne...@gmail.com>.
On Wed, Jul 6, 2016 at 12:06 AM,  <vi...@apache.org> wrote:
> diff --git a/src/master/http.cpp b/src/master/http.cpp
> index 6b4f85b..debedd4 100644
> --- a/src/master/http.cpp
> +++ b/src/master/http.cpp
> @@ -2498,11 +2498,8 @@ Future<Response> Master::Http::state(
>              });
>
>          // Model all of the orphan tasks.
> -        // TODO(vinod): Need to filter these tasks based on authorization! This
> -        // is currently not possible because we don't have `FrameworkInfo` for
> -        // these tasks. We need to either store `FrameworkInfo` for orphan
> -        // tasks or persist FrameworkInfo of all frameworks in the registry.
> -        writer->field("orphan_tasks", [this](JSON::ArrayWriter* writer) {
> +        writer->field("orphan_tasks", [this, &tasksApprover](
> +            JSON::ArrayWriter* writer) {
>            // Find those orphan tasks.
>            foreachvalue (const Slave* slave, master->slaves.registered) {
>              typedef hashmap<TaskID, Task*> TaskMap;
> @@ -2511,6 +2508,16 @@ Future<Response> Master::Http::state(
>                  CHECK_NOTNULL(task);
>                  if (!master->frameworks.registered.contains(
>                      task->framework_id())) {
> +                  CHECK(master->frameworks.recovered.contains(
> +                      task->framework_id()));

This CHECK seems dubious: what if the orphaned task was running on an
old version of the agent? i.e., a mixed cluster in which the master
has been updated but the agent has not been.

Neil