> 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 > >
