> On July 22, 2014, 12:40 a.m., Ben Mahler wrote: > > 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.
A will create a review for the activate endpoint(POST) as soon as this one receives 'shipit'. This is because the activate endpoint is very similar to this one so each issue you would found for the deactivate part it would also be in the activate part (so duplicate amount of comments and problems). For the GET endpoints I will create a separate review. > On July 22, 2014, 12:40 a.m., Ben Mahler wrote: > > src/master/http.cpp, lines 312-313 > > <https://reviews.apache.org/r/22754/diff/4/?file=636774#file636774line312> > > > > 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. ok. > On July 22, 2014, 12:40 a.m., Ben Mahler wrote: > > src/master/http.cpp, line 316 > > <https://reviews.apache.org/r/22754/diff/4/?file=636774#file636774line316> > > > > 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? ok. > On July 22, 2014, 12:40 a.m., Ben Mahler wrote: > > src/master/http.cpp, line 319 > > <https://reviews.apache.org/r/22754/diff/4/?file=636774#file636774line319> > > > > Remove these std:: prefixes in the cpp files. ok. > On July 22, 2014, 12:40 a.m., Ben Mahler wrote: > > src/master/http.cpp, lines 347-356 > > <https://reviews.apache.org/r/22754/diff/4/?file=636774#file636774line347> > > > > Since it's currently all-or-nothing, can we just send back 200 OK with > > some response string like "Deactivated 301 hosts."? ok. > On July 22, 2014, 12:40 a.m., Ben Mahler wrote: > > src/master/master.cpp, line 1631 > > <https://reviews.apache.org/r/22754/diff/4/?file=636776#file636776line1631> > > > > 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/ ok. > On July 22, 2014, 12:40 a.m., Ben Mahler wrote: > > src/master/master.cpp, line 1634 > > <https://reviews.apache.org/r/22754/diff/4/?file=636776#file636776line1634> > > > > You can keep a set of invalid hostnames instead of a string, and then > > use stringify() to print them in your failure message. ok. > On July 22, 2014, 12:40 a.m., Ben Mahler wrote: > > src/master/master.cpp, line 1638 > > <https://reviews.apache.org/r/22754/diff/4/?file=636776#file636776line1638> > > > > space before ( ok. > On July 22, 2014, 12:40 a.m., Ben Mahler wrote: > > src/master/master.cpp, line 1664 > > <https://reviews.apache.org/r/22754/diff/4/?file=636776#file636776line1664> > > > > Please add your name in the TODO for posterity. ok. > On July 22, 2014, 12:40 a.m., Ben Mahler wrote: > > src/master/master.cpp, line 1668 > > <https://reviews.apache.org/r/22754/diff/4/?file=636776#file636776line1668> > > > > We should have a note that deactivating a slave in the allocator is an > > idempotent operation. ok. > On July 22, 2014, 12:40 a.m., Ben Mahler wrote: > > src/master/master.cpp, lines 1637-1653 > > <https://reviews.apache.org/r/22754/diff/4/?file=636776#file636776line1637> > > > > 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! True, this is O(n^2). - Alexandra ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22754/#review48314 ----------------------------------------------------------- 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 > >
