You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2018/08/13 22:22:50 UTC

Review Request 68305: Increased and added flag for the master's authentication timeout.

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

Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
-------

There is not a lot of value in the master timing out a client's
authentication, other than releasing a small amount of resources.
We currently have a burned in 5 second timeout, which is largely
sufficient since most authenticators are implemented to use an
actor per session and avoid any head-of-line blocking.

Ideally, the master would know how long the client's timeout and
the master can use that for its own timeout. The current max backoff
for schedulers and agents is 1 minute, so this patch bumps the
master's timeout to be closer to that (15 seconds). We don't bump it
further because the vast majority of the timeout time is spent in
the initial trip through the master's queue, which occurs before
the master sets up its timeout.

This also adds a flag, both to allow users to tune this, as well
as to allow us to control timing in tests.


Diffs
-----

  src/master/constants.hpp f3b257a8a4c492593c701af442a0acc2b3c01762 
  src/master/flags.hpp 3929c297b45e1203e5b00454e88f86988a8b1058 
  src/master/flags.cpp 8fede0d2cf2254f34134329885f47c3c78dc5846 
  src/master/master.cpp 400a83e35451a3ee0ea42b5ca729357bf6c744e8 


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


Testing
-------

make check


Thanks,

Benjamin Mahler


Re: Review Request 68305: Increased and added flag for the master's authentication timeout.

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




src/master/flags.cpp
Lines 237-243 (patched)
<https://reviews.apache.org/r/68305/#comment290601>

    Please add this to authentication.md too.


- Gastón Kleiman


On Aug. 13, 2018, 3:22 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68305/
> -----------------------------------------------------------
> 
> (Updated Aug. 13, 2018, 3:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9145
>     https://issues.apache.org/jira/browse/MESOS-9145
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> There is not a lot of value in the master timing out a client's
> authentication, other than releasing a small amount of resources.
> We currently have a burned in 5 second timeout, which is largely
> sufficient since most authenticators are implemented to use an
> actor per session and avoid any head-of-line blocking.
> 
> Ideally, the master would know how long the client's timeout and
> the master can use that for its own timeout. The current max backoff
> for schedulers and agents is 1 minute, so this patch bumps the
> master's timeout to be closer to that (15 seconds). We don't bump it
> further because the vast majority of the timeout time is spent in
> the initial trip through the master's queue, which occurs before
> the master sets up its timeout.
> 
> This also adds a flag, both to allow users to tune this, as well
> as to allow us to control timing in tests.
> 
> 
> Diffs
> -----
> 
>   src/master/constants.hpp f3b257a8a4c492593c701af442a0acc2b3c01762 
>   src/master/flags.hpp 3929c297b45e1203e5b00454e88f86988a8b1058 
>   src/master/flags.cpp 8fede0d2cf2254f34134329885f47c3c78dc5846 
>   src/master/master.cpp 400a83e35451a3ee0ea42b5ca729357bf6c744e8 
> 
> 
> Diff: https://reviews.apache.org/r/68305/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 68305: Increased and added flag for the master's authentication timeout.

Posted by Benjamin Mahler <bm...@apache.org>.

> On Aug. 13, 2018, 11:08 p.m., Benjamin Mahler wrote:
> > src/master/flags.cpp
> > Lines 237-243 (patched)
> > <https://reviews.apache.org/r/68305/diff/1/?file=2072115#file2072115line237>
> >
> >     Note to self: add this to configuration.md

Also, added to authentication.md per Gaston's suggestion.

Renamed to v0_authentication_timeout -> authentication_v0_timeout


- Benjamin


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


On Aug. 13, 2018, 10:22 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68305/
> -----------------------------------------------------------
> 
> (Updated Aug. 13, 2018, 10:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9145
>     https://issues.apache.org/jira/browse/MESOS-9145
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> There is not a lot of value in the master timing out a client's
> authentication, other than releasing a small amount of resources.
> We currently have a burned in 5 second timeout, which is largely
> sufficient since most authenticators are implemented to use an
> actor per session and avoid any head-of-line blocking.
> 
> Ideally, the master would know how long the client's timeout and
> the master can use that for its own timeout. The current max backoff
> for schedulers and agents is 1 minute, so this patch bumps the
> master's timeout to be closer to that (15 seconds). We don't bump it
> further because the vast majority of the timeout time is spent in
> the initial trip through the master's queue, which occurs before
> the master sets up its timeout.
> 
> This also adds a flag, both to allow users to tune this, as well
> as to allow us to control timing in tests.
> 
> 
> Diffs
> -----
> 
>   src/master/constants.hpp f3b257a8a4c492593c701af442a0acc2b3c01762 
>   src/master/flags.hpp 3929c297b45e1203e5b00454e88f86988a8b1058 
>   src/master/flags.cpp 8fede0d2cf2254f34134329885f47c3c78dc5846 
>   src/master/master.cpp 400a83e35451a3ee0ea42b5ca729357bf6c744e8 
> 
> 
> Diff: https://reviews.apache.org/r/68305/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 68305: Increased and added flag for the master's authentication timeout.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68305/#review207176
-----------------------------------------------------------




src/master/flags.cpp
Lines 237-243 (patched)
<https://reviews.apache.org/r/68305/#comment290472>

    Note to self: add this to configuration.md


- Benjamin Mahler


On Aug. 13, 2018, 10:22 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68305/
> -----------------------------------------------------------
> 
> (Updated Aug. 13, 2018, 10:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9145
>     https://issues.apache.org/jira/browse/MESOS-9145
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> There is not a lot of value in the master timing out a client's
> authentication, other than releasing a small amount of resources.
> We currently have a burned in 5 second timeout, which is largely
> sufficient since most authenticators are implemented to use an
> actor per session and avoid any head-of-line blocking.
> 
> Ideally, the master would know how long the client's timeout and
> the master can use that for its own timeout. The current max backoff
> for schedulers and agents is 1 minute, so this patch bumps the
> master's timeout to be closer to that (15 seconds). We don't bump it
> further because the vast majority of the timeout time is spent in
> the initial trip through the master's queue, which occurs before
> the master sets up its timeout.
> 
> This also adds a flag, both to allow users to tune this, as well
> as to allow us to control timing in tests.
> 
> 
> Diffs
> -----
> 
>   src/master/constants.hpp f3b257a8a4c492593c701af442a0acc2b3c01762 
>   src/master/flags.hpp 3929c297b45e1203e5b00454e88f86988a8b1058 
>   src/master/flags.cpp 8fede0d2cf2254f34134329885f47c3c78dc5846 
>   src/master/master.cpp 400a83e35451a3ee0ea42b5ca729357bf6c744e8 
> 
> 
> Diff: https://reviews.apache.org/r/68305/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 68305: Increased and added flag for the master's authentication timeout.

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


Fix it, then Ship it!





src/master/flags.cpp
Lines 240-242 (patched)
<https://reviews.apache.org/r/68305/#comment290594>

    I'd remove the note. We could add it later once we have a concrete plan for deprecating v0 authentication, in the meantime I don't think it really helps operators.


- Gastón Kleiman


On Aug. 13, 2018, 3:22 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68305/
> -----------------------------------------------------------
> 
> (Updated Aug. 13, 2018, 3:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9145
>     https://issues.apache.org/jira/browse/MESOS-9145
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> There is not a lot of value in the master timing out a client's
> authentication, other than releasing a small amount of resources.
> We currently have a burned in 5 second timeout, which is largely
> sufficient since most authenticators are implemented to use an
> actor per session and avoid any head-of-line blocking.
> 
> Ideally, the master would know how long the client's timeout and
> the master can use that for its own timeout. The current max backoff
> for schedulers and agents is 1 minute, so this patch bumps the
> master's timeout to be closer to that (15 seconds). We don't bump it
> further because the vast majority of the timeout time is spent in
> the initial trip through the master's queue, which occurs before
> the master sets up its timeout.
> 
> This also adds a flag, both to allow users to tune this, as well
> as to allow us to control timing in tests.
> 
> 
> Diffs
> -----
> 
>   src/master/constants.hpp f3b257a8a4c492593c701af442a0acc2b3c01762 
>   src/master/flags.hpp 3929c297b45e1203e5b00454e88f86988a8b1058 
>   src/master/flags.cpp 8fede0d2cf2254f34134329885f47c3c78dc5846 
>   src/master/master.cpp 400a83e35451a3ee0ea42b5ca729357bf6c744e8 
> 
> 
> Diff: https://reviews.apache.org/r/68305/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 68305: Increased and added flag for the master's authentication timeout.

Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68305/#review207307
-----------------------------------------------------------


Ship it!




Ship It!

- Alexander Rojas


On Aug. 14, 2018, 12:22 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68305/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2018, 12:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9145
>     https://issues.apache.org/jira/browse/MESOS-9145
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> There is not a lot of value in the master timing out a client's
> authentication, other than releasing a small amount of resources.
> We currently have a burned in 5 second timeout, which is largely
> sufficient since most authenticators are implemented to use an
> actor per session and avoid any head-of-line blocking.
> 
> Ideally, the master would know how long the client's timeout and
> the master can use that for its own timeout. The current max backoff
> for schedulers and agents is 1 minute, so this patch bumps the
> master's timeout to be closer to that (15 seconds). We don't bump it
> further because the vast majority of the timeout time is spent in
> the initial trip through the master's queue, which occurs before
> the master sets up its timeout.
> 
> This also adds a flag, both to allow users to tune this, as well
> as to allow us to control timing in tests.
> 
> 
> Diffs
> -----
> 
>   docs/authentication.md ab3791b7d0c52826213f735b4acf5627b2925330 
>   docs/configuration/master.md 2090090894211ee7f5b7ffce58256f1b678b375a 
>   src/master/constants.hpp f3b257a8a4c492593c701af442a0acc2b3c01762 
>   src/master/flags.hpp 3929c297b45e1203e5b00454e88f86988a8b1058 
>   src/master/flags.cpp 8fede0d2cf2254f34134329885f47c3c78dc5846 
>   src/master/master.cpp 400a83e35451a3ee0ea42b5ca729357bf6c744e8 
> 
> 
> Diff: https://reviews.apache.org/r/68305/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 68305: Increased and added flag for the master's authentication timeout.

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




src/master/flags.cpp
Lines 237-242 (patched)
<https://reviews.apache.org/r/68305/#comment290691>

    We don't make much use of the 'v0' name for the unversioned APIs in the documentation and flags. 
    
    I might say something like "The timeout within which non-HTTP authentication must complete. This applies to agents and SchedulerDriver-based frameworks."
    
    It might be OK to just call this flag 'authentication_timeout', since the doc string specifies the cases to which it's relevant?


- Greg Mann


On Aug. 13, 2018, 10:22 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68305/
> -----------------------------------------------------------
> 
> (Updated Aug. 13, 2018, 10:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9145
>     https://issues.apache.org/jira/browse/MESOS-9145
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> There is not a lot of value in the master timing out a client's
> authentication, other than releasing a small amount of resources.
> We currently have a burned in 5 second timeout, which is largely
> sufficient since most authenticators are implemented to use an
> actor per session and avoid any head-of-line blocking.
> 
> Ideally, the master would know how long the client's timeout and
> the master can use that for its own timeout. The current max backoff
> for schedulers and agents is 1 minute, so this patch bumps the
> master's timeout to be closer to that (15 seconds). We don't bump it
> further because the vast majority of the timeout time is spent in
> the initial trip through the master's queue, which occurs before
> the master sets up its timeout.
> 
> This also adds a flag, both to allow users to tune this, as well
> as to allow us to control timing in tests.
> 
> 
> Diffs
> -----
> 
>   docs/authentication.md ab3791b7d0c52826213f735b4acf5627b2925330 
>   docs/configuration/master.md 2090090894211ee7f5b7ffce58256f1b678b375a 
>   src/master/constants.hpp f3b257a8a4c492593c701af442a0acc2b3c01762 
>   src/master/flags.hpp 3929c297b45e1203e5b00454e88f86988a8b1058 
>   src/master/flags.cpp 8fede0d2cf2254f34134329885f47c3c78dc5846 
>   src/master/master.cpp 400a83e35451a3ee0ea42b5ca729357bf6c744e8 
> 
> 
> Diff: https://reviews.apache.org/r/68305/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>