mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kapil Arya" <ka...@mesosphere.io>
Subject Re: Review Request 28401: Introduced Hooks API for Master
Date Mon, 24 Nov 2014 22:31:11 GMT


> On Nov. 24, 2014, 5:12 p.m., Alexander Rukletsov wrote:
> > src/hook/manager.hpp, lines 22-29
> > <https://reviews.apache.org/r/28401/diff/1/?file=774582#file774582line22>
> >
> >     Does it make sense to move <pthread.h>, <vector> and "common/lock.hpp"
into `.cpp`?

Thanks for catching this.


> On Nov. 24, 2014, 5:12 p.m., Alexander Rukletsov wrote:
> > src/hook/manager.cpp, lines 51-62
> > <https://reviews.apache.org/r/28401/diff/1/?file=774583#file774583line51>
> >
> >     This code loads hooks one by one and gives up once the next hook cannot be found,
right? This looks a bit strange for me. I would propose one of the two alternatives:
> >     1. Optimistic: load all loadable hooks and ignore those that cannot be found.
> >     2. Pessimistic, but transactional: load hooks only if all given hooks can be
found & loaded.

This is in alignment with the module design philosophy. If something that was specified on
the command line is not correct/valid, we should fail there instead of going forward with
partial success.

So, if a single hook fails to load, we fail write there.  The user can then either fix the
problem or remove the hook from the command-line flag.


> On Nov. 24, 2014, 5:12 p.m., Alexander Rukletsov wrote:
> > src/hook/manager.cpp, lines 74-78
> > <https://reviews.apache.org/r/28401/diff/1/?file=774583#file774583line74>
> >
> >     Is my understanding correct, that if we have plenty of hook modules loaded,
we will search through all of them each time a hook is invoked (e.g. authorizeTasks())? If
yes, then together with the hook's payload it can lead to performance issues.

This is a good question.  There are two parts to the performance penalty:

1. Iterating through all the available modules -- for this case, we can try keeping a separate
hook list for each "kind" of hook.
2. Hook overhead --  As with the design of hooks, there is (yet) no guarantee that a hook
won't misbehave.


> On Nov. 24, 2014, 5:12 p.m., Alexander Rukletsov wrote:
> > src/master/main.cpp, lines 159-164
> > <https://reviews.apache.org/r/28401/diff/1/?file=774585#file774585line159>
> >
> >     See my comment above. The message is a bit misleading: there can be a situation,
when this error is printed, but several hooks are loaded.

The error message is supposed to inform the user of a failed hook loading.  Is there a alternative
message that you suggest?


- Kapil


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


On Nov. 24, 2014, 3:07 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28401/
> -----------------------------------------------------------
> 
> (Updated Nov. 24, 2014, 3:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2060
>     https://issues.apache.org/jira/browse/MESOS-2060
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Hooks can be specified on the command line by specifying the --hooks flags.  Hooks are
implemented as Mesos modules that have to be specified via the --modules flag which can also
be used to specify any hook-specific command line parameters.
> 
> This is still a PoC based on the discussions on the dev mailing list.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 1d4ba1c8335eb8106cbccf903eaf3d9fdebdcda2 
>   src/hook/hook.hpp PRE-CREATION 
>   src/hook/manager.hpp PRE-CREATION 
>   src/hook/manager.cpp PRE-CREATION 
>   src/master/flags.hpp 1cea50c02f3ad7de1e1ae91d65d1accdb9af7b03 
>   src/master/main.cpp 193d53f13d8b14638b311cc290b5a5802ea56299 
>   src/master/master.cpp de42f8eb7c3c4ed64fb7fea9f4977e276f4a9043 
>   src/module/hook.hpp PRE-CREATION 
>   src/module/manager.cpp b15b0fc3f056fe29bd4d1acca508d75805ef2e0b 
> 
> Diff: https://reviews.apache.org/r/28401/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message