----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32502/#review79358 -----------------------------------------------------------
include/mesos/mesos.proto <https://reviews.apache.org/r/32502/#comment128618> For consistency, mind adding a comment in `Slave::statusUpdate()` that we should ensure `SlaveID` is set? Or even log warning in case it's not, so we start preparing framework authors for the upcoming change now. Also, maybe we need a JIRA for this. include/mesos/scheduler/scheduler.proto <https://reviews.apache.org/r/32502/#comment128620> I see your point in making `SlaveID` required in `TaskStatus` and it makes sense to me. But does this mean it should be required here as well? My understanding is that if the master knows `TaskID` it can always deduce `SlaveID`; if the task is unknown to the master, then the task is lost in all cases except when there are transitionary slaves. Having `SlaveID` in reconciliation query *just* speeds up reconciliation for unknown tasks, therefore making it required feels too much for me. Maybe Ben Mahler can provide more background and validate my thoughts. If you opt for making it required, I would then suggest to add checks in reconciliation algorithm, so that `SlaveID` of known (to the master) tasks matches `SlaveID` from the query. Similar to what you do in a subsequent patch for `killTask`. I'll also leave a comment on MESOS-1127. src/master/master.cpp <https://reviews.apache.org/r/32502/#comment128621> I would suggest we put the implementation after `Master::accept()` call for consistency. src/master/master.cpp <https://reviews.apache.org/r/32502/#comment128633> Do you want to update metrics here? `++metrics->messages_reconcile_tasks;` - Alexander Rukletsov On April 3, 2015, 11:36 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32502/ > ----------------------------------------------------------- > > (Updated April 3, 2015, 11:36 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-1127 > https://issues.apache.org/jira/browse/MESOS-1127 > > > Repository: mesos > > > Description > ------- > > Having a slave id will help us do better reconciliation. So added it to the > 'Reconcile' call. > > Also updated the master to receive the Reconcile call directly. > > > Diffs > ----- > > include/mesos/mesos.proto 3a8e8bf303e0576c212951f6028af77e54d93537 > include/mesos/scheduler/scheduler.proto > 783a63ad1cc0edd86605d638046fb959cb6e97e8 > src/master/master.hpp 05be911734b8d70c9fa5f3b4a275e8b610621fc5 > src/master/master.cpp 618db68ee4163b06e479cf3413eda4b63c9c5a4b > src/scheduler/scheduler.cpp 584b042e32865fdf875bf41ebcfb7f9c327d882a > src/tests/scheduler_tests.cpp 4a89a7a88b50bb8c254f5076661ce07ac9fc7657 > > Diff: https://reviews.apache.org/r/32502/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
