----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30694/#review71497 -----------------------------------------------------------
Great stuff, looking much better! include/mesos/resources.hpp <https://reviews.apache.org/r/30694/#comment117241> "checkpointing on the slave", right? I think going forward we'll want to pull out our custom filters from the API, checkpointing seems like an internal filter not relevant to frameworks. Mind adding a TODO to pull it out? src/master/master.cpp <https://reviews.apache.org/r/30694/#comment117242> Can we get the allocator call, slave->apply, and message sending all inside the helper? Might want to rename it to `applyOfferOperation` at that point? src/tests/persistent_volume_tests.cpp <https://reviews.apache.org/r/30694/#comment117244> Why is this needed? src/tests/persistent_volume_tests.cpp <https://reviews.apache.org/r/30694/#comment117245> Why is this needed? Looks like setting the `--resources` flag with `"role1"` in the test would be more obvious? src/tests/persistent_volume_tests.cpp <https://reviews.apache.org/r/30694/#comment117247> camelCase on these? Unless you want these to be structs that are implicitly convertible to Operation and Resource? src/tests/persistent_volume_tests.cpp <https://reviews.apache.org/r/30694/#comment117246> It's not obvious that there's enough disk for the 64MB and 128MB volumes below, why not just set `--resources` here? - Ben Mahler On Feb. 6, 2015, 7:47 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30694/ > ----------------------------------------------------------- > > (Updated Feb. 6, 2015, 7:47 p.m.) > > > Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone. > > > Bugs: MESOS-2100 > https://issues.apache.org/jira/browse/MESOS-2100 > > > Repository: mesos > > > Description > ------- > > Fixed bugs in CREATE/DESTROY operation handlers and added tests. > > > Diffs > ----- > > include/mesos/resources.hpp 3b57568c10233a0c692787de6464f21af5eaadf4 > src/Makefile.am 93537d17d3c7604a8532ee1453e405630c481ddc > src/master/master.hpp dcfd38ae2fa9e1bd0b477e9719724dba37114d30 > src/master/master.cpp 234bbecc4205036d790b026abd59100eb188f913 > src/tests/persistent_volume_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/30694/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
