> On June 30, 2014, 6:44 a.m., Adam B wrote: > > src/master/http.cpp, line 351 > > <https://reviews.apache.org/r/23147/diff/1/?file=620066#file620066line351> > > > > There could be upgrade issues for anyone using hardcoded keys to read > > from the stats endpoint. I'm not sure how important that is, but I can > > think of two approaches: > > 1) Just document the change. > > 2) Document the change, and store these stats at the old keys as well > > as the new ones. The old keys can be deprecated in a future release.
The second approach seems ok to me. The first one does not fix the upgrade issues you mentioned. > On June 30, 2014, 6:44 a.m., Adam B wrote: > > src/master/http.cpp, line 354 > > <https://reviews.apache.org/r/23147/diff/1/?file=620066#file620066line354> > > > > I wonder if "total_schedulers" is the correct term for this. Perhaps > > "registered_schedulers" would be more explicit? I'm not sure about that. Other opinions are welcomed. > On June 30, 2014, 6:44 a.m., Adam B wrote: > > src/master/http.cpp, lines 354-355 > > <https://reviews.apache.org/r/23147/diff/1/?file=620066#file620066line354> > > > > BTW, why are these "schedulers" and not "frameworks"? Should we rename > > them while we're at it? Well, maybe because each framework has it's own scheduler ?! Other opinions are welcomed. > On June 30, 2014, 6:44 a.m., Adam B wrote: > > src/master/master.hpp, line 746 > > <https://reviews.apache.org/r/23147/diff/1/?file=620067#file620067line746> > > > > active(true), Please see the comment for 'bool disconnected' @ master.hpp:854 > On June 30, 2014, 6:44 a.m., Adam B wrote: > > src/master/master.cpp, lines 768-770 > > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line768> > > > > } else if (slave->active) { > > // Checkpointing slaves can just be deactivated. > > deactivate(slave); Please see the comment for 'bool disconnected' @ master.hpp:854 > On June 30, 2014, 6:44 a.m., Adam B wrote: > > src/master/master.cpp, line 1598 > > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line1598> > > > > deactivate(Slave*) Please see the comment for 'bool disconnected' @ master.hpp:854 > On June 30, 2014, 6:44 a.m., Adam B wrote: > > src/master/master.cpp, line 1602 > > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line1602> > > > > Deactivating Please see the comment for 'bool disconnected' @ master.hpp:854 > On June 30, 2014, 6:44 a.m., Adam B wrote: > > src/master/master.cpp, line 1604 > > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line1604> > > > > deactivated Please see the comment for 'bool disconnected' @ master.hpp:854 > On June 30, 2014, 6:44 a.m., Adam B wrote: > > src/master/master.cpp, line 1605 > > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line1605> > > > > slave->active = false; Please see the comment for 'bool disconnected' @ master.hpp:854 > On June 30, 2014, 6:44 a.m., Adam B wrote: > > src/master/master.cpp, line 2746 > > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line2746> > > > > if (!slave->active) { Please see the comment for 'bool disconnected' @ master.hpp:854 > On June 30, 2014, 6:44 a.m., Adam B wrote: > > src/master/master.cpp, line 2747 > > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line2747> > > > > deactivated Please see the comment for 'bool disconnected' @ master.hpp:854 > On June 30, 2014, 6:44 a.m., Adam B wrote: > > src/master/master.cpp, line 2752 > > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line2752> > > > > deactivated Please see the comment for 'bool disconnected' @ master.hpp:854 > On June 30, 2014, 6:44 a.m., Adam B wrote: > > src/master/master.cpp, lines 2927-2932 > > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line2927> > > > > // If this is a deactivated slave... > > if (!slave->active) { > > slave->active = true; Please see the comment for 'bool disconnected' @ master.hpp:854 > On June 30, 2014, 6:44 a.m., Adam B wrote: > > src/master/master.cpp, lines 3390-3391 > > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line3390> > > > > I wonder if we should also be checking if > > (!slaves.registered[slaveId]->active) This is already done at line 3410 with a different log message. - Alexandra ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23147/#review46964 ----------------------------------------------------------- On July 7, 2014, 7:05 p.m., Alexandra Sava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23147/ > ----------------------------------------------------------- > > (Updated July 7, 2014, 7:05 p.m.) > > > Review request for mesos, Adam B and Ben Mahler. > > > Bugs: MESOS-1188 > https://issues.apache.org/jira/browse/MESOS-1188 > > > Repository: mesos-git > > > Description > ------- > > The existing terminology is confusing both for "slaves.deactivated" and > "frameworks.activated". Currently a deactivated slave actually represents a > removed/shutdown slave and "frameworks.activated" map holds both activated > and deactivated frameworks. > In order to make things look clear, rename the following: > * master.slaves.deactivated -> master.slaves.removed > * master.slaves.activated -> master.slaves.registered > * master.frameworks.activated -> master.frameworks.registered > * allocator.slaveDisconnect -> allocator.slaveDeactivate > * allocator.slaveReconnected -> allocator.slaveReactivated > > > Diffs > ----- > > src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7 > src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad > src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118 > src/master/hierarchical_allocator_process.hpp > 1765e7035bdda4c28e79d74c92e77dcc99759001 > src/master/http.cpp 5d869767cd3ed48aae1e702e8d014a37ef371123 > src/master/master.hpp 5fef35406c2ce2ad11e030aa7752eb691aab5857 > src/master/master.cpp 251b699ef971d69190bb2269ec8679067383e546 > src/tests/fault_tolerance_tests.cpp > ac65050bec5720b982f53d4dd6797cc3dee285dc > src/tests/master_authorization_tests.cpp > 0fdf464cc4a562afec276ec604205af3b56636de > src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb > src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e > src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827 > src/webui/master/static/js/controllers.js > 41a70a80442501a2bf7b217939dbe504662941d2 > > Diff: https://reviews.apache.org/r/23147/diff/ > > > Testing > ------- > > > Thanks, > > Alexandra Sava > >
