----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64557/#review193733 -----------------------------------------------------------
Fix it, then Ship it! src/resource_provider/manager.cpp Lines 613-614 (patched) <https://reviews.apache.org/r/64557/#comment272361> We could use aggregate initialization here, ResourceProviderMessage::Disconnect disconnect{resourceProviderId}; src/tests/resource_provider_manager_tests.cpp Lines 1246 (patched) <https://reviews.apache.org/r/64557/#comment272358> I would suggest to add an assertion that `disk` is contained in the total resources of the resource provider in this update (i.e., a check similar to the one on the second update with inverted contains expectation). This makes sure that we actually detect a change when checking below expectations. src/tests/resource_provider_manager_tests.cpp Lines 1255 (patched) <https://reviews.apache.org/r/64557/#comment272354> Replace `1u` with just `1` here. Protobuf tries to be nice and safe us from integer conversion surprises by using a signed integer for sizes. src/tests/resource_provider_manager_tests.cpp Lines 1257 (patched) <https://reviews.apache.org/r/64557/#comment272355> `const Resources&` src/tests/resource_provider_manager_tests.cpp Lines 1260 (patched) <https://reviews.apache.org/r/64557/#comment272357> This expecation is always met as `Resource`s in `totalResources` will have a provider ID set, but `disk` has not. We need to create an updated `disk` after the resource provider has subscribed, e.g., after l.1245 add ASSERT_TRUE(resourceProvider->info.has_id()); disk.mutable_provider_id()->CopyFrom(resourceProvider->info.id()); - Benjamin Bannier On Dec. 12, 2017, 11:54 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64557/ > ----------------------------------------------------------- > > (Updated Dec. 12, 2017, 11:54 p.m.) > > > Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Greg Mann. > > > Repository: mesos > > > Description > ------- > > If an RP is disconnected, we'll shrink its total resources to zero so > that no offer will be made on this RP until it reconnects. This prevents > frameworks from sending operations to the disconnected RP. > > > Diffs > ----- > > src/resource_provider/manager.cpp fd138b9914d925b5be7a11255dd632921c107dba > src/resource_provider/message.hpp eab90cffd6aab9e38207dcf109cc737171ed3953 > src/slave/slave.cpp 5869e73ca1c14c99e580da9d7375181da2073ec5 > src/tests/resource_provider_manager_tests.cpp > e37a53ac6a03e2ea58dd6580fc8a399a1398d950 > > > Diff: https://reviews.apache.org/r/64557/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
