> On Nov. 23, 2014, 11:49 p.m., Benjamin Hindman wrote: > > src/watcher/whitelist_watcher.hpp, lines 34-35 > > <https://reviews.apache.org/r/28230/diff/1/?file=769747#file769747line34> > > > > Why aren't you just using master/constants.hpp and then > > WHITELIST_WATCH_INTERVAL? Otherwise, I'd expect that you're moving it here > > but I didn't see that deleted in this review. > > Niklas Nielsen wrote: > yeah - along the same lines: Can we make this a parameter instead and > grab the constants from src/{master,slave}/constants.{hpp,cpp} instead?
I don't reuse the const from `master/constants.hpp` because I'll be using WhitelistWatcher in slave and I don't want to pull master include into slave. Good catch about removing the constant from master sources, thanks! > On Nov. 23, 2014, 11:49 p.m., Benjamin Hindman wrote: > > src/master/master.cpp, lines 523-526 > > <https://reviews.apache.org/r/28230/diff/1/?file=769746#file769746line523> > > > > Would you mind adding a comment here that explains that passing a > > callback (via the lambda::bind) is safe because we make sure to shutdown > > the 'whitelistWatcher' in Master::finalize so the WhitelistWatcher should > > not be making any callbacks after the master is terminated/destructed so > > there isn't any risk of calling into an allocator that has been cleaned up. > > Thanks! Very good point, thanks for bringing this up. I have also added the comment in the WhitelistWatcher class. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28230/#review62745 ----------------------------------------------------------- On Nov. 20, 2014, 4:42 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28230/ > ----------------------------------------------------------- > > (Updated Nov. 20, 2014, 4:42 p.m.) > > > Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, > and Vinod Kone. > > > Bugs: MESOS-2114 > https://issues.apache.org/jira/browse/MESOS-2114 > > > Repository: mesos-git > > > Description > ------- > > WhitelistWatcher can be used for tracking changes to all kind of whitelists. > In order to reuse the functionality, extract the code from master sources. > > > Diffs > ----- > > src/Makefile.am 0fe7dd0 > src/master/hierarchical_allocator_process.hpp 31dfb2c > src/master/master.hpp ece36c3 > src/master/master.cpp 9e2d768 > src/watcher/whitelist_watcher.hpp PRE-CREATION > src/watcher/whitelist_watcher.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/28230/diff/ > > > Testing > ------- > > make check (Mac OS 10.9.4, Ubuntu 14.04) > > > Thanks, > > Alexander Rukletsov > >
