You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benno Evers <be...@mesosphere.com> on 2017/12/06 16:22:46 UTC

Review Request 64384: Added new 'any' setting for configuration_compatibility flag.

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

Review request for mesos.


Repository: mesos


Description
-------

Added new 'any' setting for configuration_compatibility flag.


Diffs
-----

  src/slave/compatibility.hpp 78b421a01abe5d2178c93832577577a7ba282b38 
  src/slave/compatibility.cpp 4ead4a5b655f6f3d7812aa52d656830d7cff4598 
  src/slave/flags.cpp d8764745e6aca81283d8b96388df1320c3465952 
  src/slave/slave.cpp 49270013537356c8fe9150d757b064bc3bbae3cb 


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


Testing
-------


Thanks,

Benno Evers


Re: Review Request 64384: Added new 'any' setting for reconfiguration_policy flag.

Posted by Benno Evers <be...@mesosphere.com>.

> On May 8, 2018, 11:36 p.m., Zhitao Li wrote:
> > src/slave/slave.cpp
> > Lines 6373-6375 (original), 6377-6379 (patched)
> > <https://reviews.apache.org/r/64384/diff/1/?file=1909675#file1909675line6377>
> >
> >     I noticed that we do not refresh the checkpointed information of agent, but simply proceed to run a different `AgentInfo` from what's left on the disk.
> >     
> >     Do we worry that this could cause a confusion in the future? I wonder whether we should augment the behavior for `any` to also flush out changed `AgentInfo` using `state::checkpoint()`.
> >     
> >     Thoughts?

Hi, sorry for the late reply - you're of course correct, I'm not sure how we managed to overlook this during all our reviews.

I'll try to get a patch out for this as soon as I find some time.


- Benno


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


On Dec. 6, 2017, 4:22 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64384/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2017, 4:22 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This setting allows any state change, effectively telling agents to ignore the existing state and not to run any new tasks until the existing ones fit into the new provided resources.
> 
> 
> Diffs
> -----
> 
>   src/slave/compatibility.hpp 78b421a01abe5d2178c93832577577a7ba282b38 
>   src/slave/compatibility.cpp 4ead4a5b655f6f3d7812aa52d656830d7cff4598 
>   src/slave/flags.cpp d8764745e6aca81283d8b96388df1320c3465952 
>   src/slave/slave.cpp 49270013537356c8fe9150d757b064bc3bbae3cb 
> 
> 
> Diff: https://reviews.apache.org/r/64384/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 64384: Added new 'any' setting for reconfiguration_policy flag.

Posted by Benno Evers <be...@mesosphere.com>.

> On May 8, 2018, 11:36 p.m., Zhitao Li wrote:
> > src/slave/slave.cpp
> > Lines 6373-6375 (original), 6377-6379 (patched)
> > <https://reviews.apache.org/r/64384/diff/1/?file=1909675#file1909675line6377>
> >
> >     I noticed that we do not refresh the checkpointed information of agent, but simply proceed to run a different `AgentInfo` from what's left on the disk.
> >     
> >     Do we worry that this could cause a confusion in the future? I wonder whether we should augment the behavior for `any` to also flush out changed `AgentInfo` using `state::checkpoint()`.
> >     
> >     Thoughts?
> 
> Benno Evers wrote:
>     Hi, sorry for the late reply - you're of course correct, I'm not sure how we managed to overlook this during all our reviews.
>     
>     I'll try to get a patch out for this as soon as I find some time.

Marking this issue as 'Fixed' because it's fixed in the subsequent review.


- Benno


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


On Dec. 6, 2017, 4:22 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64384/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2017, 4:22 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This setting allows any state change, effectively telling agents to ignore the existing state and not to run any new tasks until the existing ones fit into the new provided resources.
> 
> 
> Diffs
> -----
> 
>   src/slave/compatibility.hpp 78b421a01abe5d2178c93832577577a7ba282b38 
>   src/slave/compatibility.cpp 4ead4a5b655f6f3d7812aa52d656830d7cff4598 
>   src/slave/flags.cpp d8764745e6aca81283d8b96388df1320c3465952 
>   src/slave/slave.cpp 49270013537356c8fe9150d757b064bc3bbae3cb 
> 
> 
> Diff: https://reviews.apache.org/r/64384/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 64384: Added new 'any' setting for reconfiguration_policy flag.

Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64384/#review202713
-----------------------------------------------------------




src/slave/slave.cpp
Lines 6373-6375 (original), 6377-6379 (patched)
<https://reviews.apache.org/r/64384/#comment284699>

    I noticed that we do not refresh the checkpointed information of agent, but simply proceed to run a different `AgentInfo` from what's left on the disk.
    
    Do we worry that this could cause a confusion in the future? I wonder whether we should augment the behavior for `any` to also flush out changed `AgentInfo` using `state::checkpoint()`.
    
    Thoughts?


- Zhitao Li


On Dec. 6, 2017, 8:22 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64384/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2017, 8:22 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This setting allows any state change, effectively telling agents to ignore the existing state and not to run any new tasks until the existing ones fit into the new provided resources.
> 
> 
> Diffs
> -----
> 
>   src/slave/compatibility.hpp 78b421a01abe5d2178c93832577577a7ba282b38 
>   src/slave/compatibility.cpp 4ead4a5b655f6f3d7812aa52d656830d7cff4598 
>   src/slave/flags.cpp d8764745e6aca81283d8b96388df1320c3465952 
>   src/slave/slave.cpp 49270013537356c8fe9150d757b064bc3bbae3cb 
> 
> 
> Diff: https://reviews.apache.org/r/64384/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>