> On March 18, 2014, 9:39 p.m., Ben Mahler wrote: > > src/master/master.cpp, line 1114 > > <https://reviews.apache.org/r/19006/diff/2/?file=526925#file526925line1114> > > > > I applaud the effort here to clean this up but it looks like the > > meaning of the words used for slave states are inconsistent now: > > > > 1. The SlaveObserver "disconnects" a slave, which shuts the slave down, > > and marks it as "deactivated" once removed. > > > > 2. We have "deactivate", which marks the slave as "disconnected". > > > > Seems like we should either keep the terminology consistent or choose > > better terminology if we feel "activated" and "deactivated" were a poor > > choice in retrospect.
There's definitely some confusing language around various slave states (and framework states). In response to your comments, I made the following changes: 1. Renamed SlaveObserver.disconnect() (formerly deactivate()) to shutdown() since it calls what is now shutdownSlave() (was deactivateSlave). 2. Renamed deactivate(Slave) to disconnect(Slave). Now the only remaining [de]activate terminology associated with slaves are the slaves.activated/deactivated collections. I now identify the following 3 states for slaves: A. Connected: Slave exists in slaves.activated, slave.disconnected=false; disconnects when a checkpointing slave hits exited(). B. Disconnected: Slave exists in slaves.activated, slave.disconnected=true; reconnects on reregisterSlave. C. Shutdown: Slave removed from slaves.activated, pid added to slaves.deactivated; readded to slaves.activated on registerSlave. I propose that we rename slaves.activated/deactivated to slaves.running/shutdown to avoid confusion with the framework.active state and deactivateFramework message/action. The "deactivate" terminology for slaves is especially confusing because "disconnecting" a slave is analagous to "deactivating" a framework. Here are the framework states: A. Active: Framework exists in frameworks.activated, framework.active=true; goes inactive on exited(). B. Inactive: Framework exists in frameworks.activated, framework.active=false; reactivated on reregister (if before failoverTimeout). C. Completed: Framework moved to frameworks.completed; never goes back. I propose that we rename frameworks.activated to frameworks.running, because you shouldn't have an inactive slave in slaves.activated, but you could in slaves.running. Renaming slaves.activated/deactivated and frameworks.activated can happen in a subsequent review. - Adam ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19006/#review37684 ----------------------------------------------------------- On March 25, 2014, 3:22 a.m., Adam B wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19006/ > ----------------------------------------------------------- > > (Updated March 25, 2014, 3:22 a.m.) > > > Review request for mesos and Vinod Kone. > > > Bugs: MESOS-804 > https://issues.apache.org/jira/browse/MESOS-804 > > > Repository: mesos-git > > > Description > ------- > > Moved slave deactivation code out of Master::exited() into new > deactivate(Slave). > > > Diffs > ----- > > src/master/master.hpp a8ed5ec > src/master/master.cpp a951a7a > > Diff: https://reviews.apache.org/r/19006/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Adam B > >
