----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22754/#review48314 -----------------------------------------------------------
Operators will likely want a way to see which slaves are activated/deactivated. Perhaps we'll want to expose additional endpoints like: Write (POST): /slaves/deactivate (this review) /slaves/activate (follow up?) Read (GET): /slaves/deactivated /slaves/activated This means that the master will need to know the activation state of the slave, or will need to ask the allocator. src/master/http.cpp <https://reviews.apache.org/r/22754/#comment84756> Let's also mention that deactivation means that we stop making resource offers for these slaves, which means frameworks cannot launch new tasks on deactivated slaves. src/master/http.cpp <https://reviews.apache.org/r/22754/#comment84763> The observe endpoint that uses HOSTS_KEY is not functional at the current time, I think we want a different thing called "HOSTNAMES_KEY" which is equal to "hostnames", since that is the terminology in the SlaveInfo and in your description of this parameter. Can you end with a period? src/master/http.cpp <https://reviews.apache.org/r/22754/#comment84764> Remove these std:: prefixes in the cpp files. src/master/http.cpp <https://reviews.apache.org/r/22754/#comment84801> It looks like you'll want to leverage getFormValue as was done in Http::observe. The request body won't be a JSON object, it will be post parameters. src/master/http.cpp <https://reviews.apache.org/r/22754/#comment84802> Since it's currently all-or-nothing, can we just send back 200 OK with some response string like "Deactivated 301 hosts."? src/master/master.cpp <https://reviews.apache.org/r/22754/#comment84804> What about just having a single deactivateSlaves method that takes the hostnames? Not sure if we'll need the one that takes slaveIDs just yet, so might be better to keep it simple with one method. Also, s/hosts/hostnames/ src/master/master.cpp <https://reviews.apache.org/r/22754/#comment84806> You can keep a set of invalid hostnames instead of a string, and then use stringify() to print them in your failure message. src/master/master.cpp <https://reviews.apache.org/r/22754/#comment84807> How about taking a set of hostnames as the input to this method, that way you can loop over the slaves a single time, doing lookups against the host set. We need to be a bit careful here with N^2 behavior for large clusters, if we have 30,000 slaves then we might be looping O(30,000 * 30,000) times here! src/master/master.cpp <https://reviews.apache.org/r/22754/#comment84805> space before ( src/master/master.cpp <https://reviews.apache.org/r/22754/#comment84769> Please add your name in the TODO for posterity. src/master/master.cpp <https://reviews.apache.org/r/22754/#comment84803> We should have a note that deactivating a slave in the allocator is an idempotent operation. - Ben Mahler On July 21, 2014, 2:09 p.m., Alexandra Sava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22754/ > ----------------------------------------------------------- > > (Updated July 21, 2014, 2:09 p.m.) > > > Review request for mesos and Ben Mahler. > > > Repository: mesos-git > > > Description > ------- > > This is the first part of the 'deactivate slave' mechanism. > > This part consists of creating an HTTP endpoint which receives HTTP posts. > Each post contains a JSON object with a list of host names. > The list is further passed to the master which checks which slaves run on > those hosts and marks them as 'deactivated' in the allocator. The final > result is that the master will no longer send resource offers belonging to > the deactivates slaves. > > The TODO for the second part is to make the list of deactivated slaves > persistent, by storing it into the Registry. > > > Diffs > ----- > > src/master/http.cpp f2ca6599eb165c4c1bc4580175fa439f797c832b > src/master/master.hpp 7e7a75bd7e0fafc084ad2663c894e76e5fb35edd > src/master/master.cpp 896be5e8db45e819ce9f3a4e24c4017605283f12 > > Diff: https://reviews.apache.org/r/22754/diff/ > > > Testing > ------- > > > Thanks, > > Alexandra Sava > >
