----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66545/#review200903 -----------------------------------------------------------
src/resource_provider/manager.cpp Lines 668 (patched) <https://reviews.apache.org/r/66545/#comment281802> Shouldn't this be `false` here? If there's no registrar to admit a resource provider to, a resource provider cannot be admitted. src/resource_provider/manager.cpp Lines 679-683 (patched) <https://reviews.apache.org/r/66545/#comment281803> We should set this event handler before setting `admitResourceProvider` to any value, i.e. set this directly after instanciation. src/resource_provider/manager.cpp Line 662 (original), 691-697 (patched) <https://reviews.apache.org/r/66545/#comment281805> This is only checking the `FAILED` state, but not other failure states, e.g. `DISCARDED`. How about checking for `!admitResourceProvider.isReady()` here? - Jan Schlicht On April 11, 2018, 11:23 a.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66545/ > ----------------------------------------------------------- > > (Updated April 11, 2018, 11:23 a.m.) > > > Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jan Schlicht. > > > Bugs: MESOS-8402 > https://issues.apache.org/jira/browse/MESOS-8402 > > > Repository: mesos > > > Description > ------- > > Added admitted resource providers to the manager's registry. > > > Diffs > ----- > > src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a > src/resource_provider/registrar.hpp > 39f45b0a2a7c35bfe72a9305f5248409e4a3ef45 > src/resource_provider/registrar.cpp > 92ef9aecb1e93d10f46e53984391558f9901a60b > src/resource_provider/registry.proto > 14bd433ef056f8891e9a38153fdb06c39cee8f62 > src/tests/resource_provider_manager_tests.cpp > c52541bf130ccf4795b989b53331176a64a097ea > > > Diff: https://reviews.apache.org/r/66545/diff/1/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
