----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66050/#review200055 -----------------------------------------------------------
src/common/resources_utils.cpp Lines 203-205 (patched) <https://reviews.apache.org/r/66050/#comment280665> This is already done in the validation code - do we need to do it again here? Ditto below. src/common/resources_utils.cpp Lines 207-208 (patched) <https://reviews.apache.org/r/66050/#comment280667> Suggestion: "To grow a persistent volume, we consume the original volume and the additional resource and convert into a single volume with the new size." src/common/resources_utils.cpp Lines 223 (patched) <https://reviews.apache.org/r/66050/#comment280671> s/consume/consume the/ src/common/resources_utils.cpp Lines 232-237 (patched) <https://reviews.apache.org/r/66050/#comment280672> Could you provide a comment explaining the two cases here? src/common/resources_utils.cpp Lines 243 (patched) <https://reviews.apache.org/r/66050/#comment280674> s/shrinked/shrunk/ src/common/resources_utils.cpp Lines 696-698 (patched) <https://reviews.apache.org/r/66050/#comment280675> No period at the end of error messages. Here and below. src/master/master.cpp Lines 4925-4928 (patched) <https://reviews.apache.org/r/66050/#comment280694> Not indented far enough. src/master/master.cpp Lines 4928 (patched) <https://reviews.apache.org/r/66050/#comment280696> s/have/have the/ Here and below. src/master/master.cpp Lines 4945-4946 (patched) <https://reviews.apache.org/r/66050/#comment280699> Indented too far. src/master/master.cpp Lines 4949-4953 (patched) <https://reviews.apache.org/r/66050/#comment280695> Could you update the indentation on these `drop()` calls to be consistent? I'm fine with either ``` drop( framework, etc... ``` or ``` drop(framework, etc... ``` src/master/master.cpp Lines 4951 (patched) <https://reviews.apache.org/r/66050/#comment280698> s/Operation/operation/ Here and below. src/master/master.cpp Lines 4973-4974 (patched) <https://reviews.apache.org/r/66050/#comment280700> Fits on one line. src/master/master.cpp Lines 5010 (patched) <https://reviews.apache.org/r/66050/#comment280702> s/GROW_VOLUME/SHRINK_VOLUME/ src/master/master.cpp Lines 5012 (patched) <https://reviews.apache.org/r/66050/#comment280703> s/with subtract/subtracting/ src/master/validation.cpp Lines 2341 (patched) <https://reviews.apache.org/r/66050/#comment280685> "Addition in grow_volume must be greater than zero" src/master/validation.cpp Lines 2345-2346 (patched) <https://reviews.apache.org/r/66050/#comment280687> Fits on one line. src/master/validation.cpp Lines 2355-2356 (patched) <https://reviews.apache.org/r/66050/#comment280686> Fits on one line. src/master/validation.cpp Lines 2382 (patched) <https://reviews.apache.org/r/66050/#comment280688> Is this error message accurate? Looks like it should be something like "Not a valid resource: " src/master/validation.cpp Lines 2387 (patched) <https://reviews.apache.org/r/66050/#comment280689> "The 'subtract' field in 'shrink_volume' must be greater than zero" src/master/validation.cpp Lines 2392 (patched) <https://reviews.apache.org/r/66050/#comment280690> "The 'subtract' field in 'shrink_volume' must be smaller than the persistent volume's size" src/master/validation.cpp Lines 2395 (patched) <https://reviews.apache.org/r/66050/#comment280692> Is this test necessary, given the above check for `shrink.volume().scalar() <= shrink.subtract()`? src/master/validation.cpp Lines 2411-2412 (patched) <https://reviews.apache.org/r/66050/#comment280693> Fits on one line. - Greg Mann On March 27, 2018, 6:30 a.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66050/ > ----------------------------------------------------------- > > (Updated March 27, 2018, 6:30 a.m.) > > > Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann. > > > Bugs: MESOS-4965 > https://issues.apache.org/jira/browse/MESOS-4965 > > > Repository: mesos > > > Description > ------- > > The new offer operations are implemented as non-speculative operations, > which means they cannot be chained with previous offer operations which > depend on results of each other. > > > Diffs > ----- > > src/common/protobuf_utils.cpp 2acdc798200e82d14651b1edcc5f83e174477d53 > src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 > src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 > src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf > src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 > src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb > src/tests/mesos.hpp 01fd1c2bf21ef96dbab0769a1c1c92d7e3b60032 > src/tests/persistent_volume_tests.cpp > 924d8458e54e34a49c99593482b5908c5f7c7a48 > src/tests/reservation_tests.cpp 8d8e9c8390e65187269bd194bb322bbdff88e0bd > > > Diff: https://reviews.apache.org/r/66050/diff/6/ > > > Testing > ------- > > > Thanks, > > Zhitao Li > >
