> On Aug. 3, 2015, 8:07 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 1719-1721
> > <https://reviews.apache.org/r/37046/diff/1/?file=1027863#file1027863line1719>
> >
> >     Hm.. for later, I suppose we'll want to have a metric helper for calls 
> > inside receive. We currently aren't counting the number of call messages, 
> > right?

Filed https://issues.apache.org/jira/browse/MESOS-3195


> On Aug. 3, 2015, 8:07 p.m., Ben Mahler wrote:
> > src/master/master.cpp, line 1729
> > <https://reviews.apache.org/r/37046/diff/1/?file=1027863#file1027863line1729>
> >
> >     s/registration/subcribe/ ?

fixed.


> On Aug. 3, 2015, 8:07 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 1742-1745
> > <https://reviews.apache.org/r/37046/diff/1/?file=1027863#file1027863line1742>
> >
> >     Hm.. is the idea that we can remove this because we trust that the 
> > scheduler driver will not set this for a register message?

yes. but thinking more, i'll have these checks inside (re-)registerFramework() 
instead of killing them.


> On Aug. 3, 2015, 8:07 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 1754-1755
> > <https://reviews.apache.org/r/37046/diff/1/?file=1027863#file1027863line1754>
> >
> >     Hm.. if you're going to combine these mind at least wrapping so that 
> > the root checks are together?
> >     
> >     ```
> >     if (validationError.isNone() &&
> >         frameworkInfo.user() == "root" && !flags.root_submissions) {
> >     }
> >     ```

done.


> On Aug. 3, 2015, 8:07 p.m., Ben Mahler wrote:
> > src/master/master.cpp, line 1756
> > <https://reviews.apache.org/r/37046/diff/1/?file=1027863#file1027863line1756>
> >
> >     Indentation?

ty.


> On Aug. 3, 2015, 8:07 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 1854-1857
> > <https://reviews.apache.org/r/37046/diff/1/?file=1027863#file1027863line1854>
> >
> >     Do we want to log differently based on whether the framework id is set?

done.


> On Aug. 3, 2015, 8:07 p.m., Ben Mahler wrote:
> > src/master/master.cpp, line 2044
> > <https://reviews.apache.org/r/37046/diff/1/?file=1027863#file1027863line2044>
> >
> >     s/failover/force/ ?

done.


> On Aug. 3, 2015, 8:07 p.m., Ben Mahler wrote:
> > src/master/master.cpp, line 2058
> > <https://reviews.apache.org/r/37046/diff/1/?file=1027863#file1027863line2058>
> >
> >     subscription?

done.


> On Aug. 3, 2015, 8:07 p.m., Ben Mahler wrote:
> > src/master/master.cpp, line 2077
> > <https://reviews.apache.org/r/37046/diff/1/?file=1027863#file1027863line2077>
> >
> >     Just an observation, it seems not that intuitive that we call 
> > failoverFramework when the pids are the same, might benefit from a comment.

i think this was originally done in the days when the driver pid wasn't unique. 
now that it is unique, we can simplify this a bit. i'll add a TODO for now.


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37046/#review93955
-----------------------------------------------------------


On Aug. 3, 2015, 7:26 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37046/
> -----------------------------------------------------------
> 
> (Updated Aug. 3, 2015, 7:26 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-3182
>     https://issues.apache.org/jira/browse/MESOS-3182
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Merged registerFramework() and reregisterFramework() (and their corresponding 
> continuations) into subscribe() (and _subscribe()).
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc 
>   src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 
> 
> Diff: https://reviews.apache.org/r/37046/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Reply via email to