You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2019/02/12 21:48:03 UTC

Review Request 69957: Updated master operation handling to account for new agent capability.

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

Review request for mesos and Gastón Kleiman.


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


Repository: mesos


Description
-------

Since the new AGENT_OPERATION_FEEDBACK capability has been
made required for agent startup, we need to update the way
the master handles validation of incoming operations and
acknowledgement calls to account for cases where each of
the AGENT_OPERATION_FEEDBACK and RESOURCE_PROVIDER
capabilities are enabled/disabled.


Diffs
-----

  src/master/master.cpp cf2210ec26642028d5e4fb7fc1841eb0a1ed3396 


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


Testing
-------


Thanks,

Greg Mann


Re: Review Request 69957: Updated master operation handling to account for new agent capability.

Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69957/#review212883
-----------------------------------------------------------


Ship it!




Ship It!

- Gastón Kleiman


On Feb. 13, 2019, 8:09 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69957/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2019, 8:09 p.m.)
> 
> 
> Review request for mesos and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9535
>     https://issues.apache.org/jira/browse/MESOS-9535
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the new AGENT_OPERATION_FEEDBACK capability has been
> made required for agent startup, we need to update the way
> the master handles validation of incoming operations and
> acknowledgement calls to account for cases where each of
> the AGENT_OPERATION_FEEDBACK and RESOURCE_PROVIDER
> capabilities are enabled/disabled.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp cf2210ec26642028d5e4fb7fc1841eb0a1ed3396 
> 
> 
> Diff: https://reviews.apache.org/r/69957/diff/4/
> 
> 
> Testing
> -------
> 
> Testing details at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 69957: Updated master operation handling to account for new agent capability.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69957/
-----------------------------------------------------------

(Updated Feb. 14, 2019, 4:09 a.m.)


Review request for mesos and Gastón Kleiman.


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


Repository: mesos


Description
-------

Since the new AGENT_OPERATION_FEEDBACK capability has been
made required for agent startup, we need to update the way
the master handles validation of incoming operations and
acknowledgement calls to account for cases where each of
the AGENT_OPERATION_FEEDBACK and RESOURCE_PROVIDER
capabilities are enabled/disabled.


Diffs (updated)
-----

  src/master/master.cpp cf2210ec26642028d5e4fb7fc1841eb0a1ed3396 


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

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


Testing
-------

Testing details at the end of this chain.


Thanks,

Greg Mann


Re: Review Request 69957: Updated master operation handling to account for new agent capability.

Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69957/#review212812
-----------------------------------------------------------


Ship it!




Ship It!

- Gastón Kleiman


On Feb. 12, 2019, 6:37 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69957/
> -----------------------------------------------------------
> 
> (Updated Feb. 12, 2019, 6:37 p.m.)
> 
> 
> Review request for mesos and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9535
>     https://issues.apache.org/jira/browse/MESOS-9535
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the new AGENT_OPERATION_FEEDBACK capability has been
> made required for agent startup, we need to update the way
> the master handles validation of incoming operations and
> acknowledgement calls to account for cases where each of
> the AGENT_OPERATION_FEEDBACK and RESOURCE_PROVIDER
> capabilities are enabled/disabled.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp cf2210ec26642028d5e4fb7fc1841eb0a1ed3396 
> 
> 
> Diff: https://reviews.apache.org/r/69957/diff/3/
> 
> 
> Testing
> -------
> 
> Testing details at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 69957: Updated master operation handling to account for new agent capability.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69957/
-----------------------------------------------------------

(Updated Feb. 13, 2019, 2:37 a.m.)


Review request for mesos and Gastón Kleiman.


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


Repository: mesos


Description
-------

Since the new AGENT_OPERATION_FEEDBACK capability has been
made required for agent startup, we need to update the way
the master handles validation of incoming operations and
acknowledgement calls to account for cases where each of
the AGENT_OPERATION_FEEDBACK and RESOURCE_PROVIDER
capabilities are enabled/disabled.


Diffs (updated)
-----

  src/master/master.cpp cf2210ec26642028d5e4fb7fc1841eb0a1ed3396 


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

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


Testing
-------

Testing details at the end of this chain.


Thanks,

Greg Mann


Re: Review Request 69957: Updated master operation handling to account for new agent capability.

Posted by Greg Mann <gr...@mesosphere.io>.

> On Feb. 13, 2019, 12:08 a.m., Gastón Kleiman wrote:
> > src/master/master.cpp
> > Lines 4372-4375 (original), 4373-4376 (patched)
> > <https://reviews.apache.org/r/69957/diff/2/?file=2124997#file2124997line4373>
> >
> >     The semantics of these capabilities is not very intuitive; we should update our documentation with a good explanation.

Good call; I added a patch to the end of this chain: https://reviews.apache.org/r/69969/


> On Feb. 13, 2019, 12:08 a.m., Gastón Kleiman wrote:
> > src/master/master.cpp
> > Line 8725 (original)
> > <https://reviews.apache.org/r/69957/diff/2/?file=2124997#file2124997line8729>
> >
> >     Would the following assertion hold?
> >     
> >     `CHECK(slave->capabilities.resourceProvider || slave->capabilities.agentOperationFeedback)`

Yep! Good call.


- Greg


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


On Feb. 13, 2019, 2:37 a.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69957/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2019, 2:37 a.m.)
> 
> 
> Review request for mesos and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9535
>     https://issues.apache.org/jira/browse/MESOS-9535
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the new AGENT_OPERATION_FEEDBACK capability has been
> made required for agent startup, we need to update the way
> the master handles validation of incoming operations and
> acknowledgement calls to account for cases where each of
> the AGENT_OPERATION_FEEDBACK and RESOURCE_PROVIDER
> capabilities are enabled/disabled.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp cf2210ec26642028d5e4fb7fc1841eb0a1ed3396 
> 
> 
> Diff: https://reviews.apache.org/r/69957/diff/3/
> 
> 
> Testing
> -------
> 
> Testing details at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 69957: Updated master operation handling to account for new agent capability.

Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69957/#review212772
-----------------------------------------------------------




src/master/master.cpp
Lines 4372-4375 (original), 4373-4376 (patched)
<https://reviews.apache.org/r/69957/#comment298681>

    The semantics of these capabilities is not very intuitive; we should update our documentation with a good explanation.



src/master/master.cpp
Line 6419 (original), 6422-6423 (patched)
<https://reviews.apache.org/r/69957/#comment298682>

    I think the following might be a bit more useful for operators:
    
    ```
        LOG(WARNING)
          << "Cannot send operation status update acknowledgement for status "
          << statusUuid << " of operation '" << operationId << "'"
          << " of framework " << *framework << " to agent " << slaveId
          << " because the agent does not have the RESOURCE_PROVIDER"
          << " and AGENT_OPERATION_FEEDBACK capabilities";
    ```
    
    If they get that, then it might be easier for them to debug missing capabilities.



src/master/master.cpp
Line 8725 (original)
<https://reviews.apache.org/r/69957/#comment298683>

    Would the following assertion hold?
    
    `CHECK(slave->capabilities.resourceProvider || slave->capabilities.agentOperationFeedback)`


- Gastón Kleiman


On Feb. 12, 2019, 3:39 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69957/
> -----------------------------------------------------------
> 
> (Updated Feb. 12, 2019, 3:39 p.m.)
> 
> 
> Review request for mesos and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9535
>     https://issues.apache.org/jira/browse/MESOS-9535
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the new AGENT_OPERATION_FEEDBACK capability has been
> made required for agent startup, we need to update the way
> the master handles validation of incoming operations and
> acknowledgement calls to account for cases where each of
> the AGENT_OPERATION_FEEDBACK and RESOURCE_PROVIDER
> capabilities are enabled/disabled.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp cf2210ec26642028d5e4fb7fc1841eb0a1ed3396 
> 
> 
> Diff: https://reviews.apache.org/r/69957/diff/2/
> 
> 
> Testing
> -------
> 
> Testing details at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 69957: Updated master operation handling to account for new agent capability.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69957/
-----------------------------------------------------------

(Updated Feb. 12, 2019, 11:39 p.m.)


Review request for mesos and Gastón Kleiman.


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


Repository: mesos


Description
-------

Since the new AGENT_OPERATION_FEEDBACK capability has been
made required for agent startup, we need to update the way
the master handles validation of incoming operations and
acknowledgement calls to account for cases where each of
the AGENT_OPERATION_FEEDBACK and RESOURCE_PROVIDER
capabilities are enabled/disabled.


Diffs (updated)
-----

  src/master/master.cpp cf2210ec26642028d5e4fb7fc1841eb0a1ed3396 


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

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


Testing (updated)
-------

Testing details at the end of this chain.


Thanks,

Greg Mann