> On Dec. 13, 2017, 12:20 p.m., Benjamin Bannier wrote: > > src/slave/slave.cpp > > Line 6992 (original) > > <https://reviews.apache.org/r/64561/diff/1/?file=1914886#file1914886line6998> > > > > One motivation for such helper functions can be that it becomes clearer > > than `UpdateSlaveMessage` should not be constructed manually due to > > possible complexity. By removing one of them this is not as apparent > > anymore. > > > > I'd suggest to keep this helper and revert changes at the callers so an > > up-to-date `oversubscribedResources` is found.
I changed that because merging `update_oversubscribed_resources` field with one set to true and one set to false is weird. There is actually a bug previously that in generateResourceProviderMessage, we don't set `update_oversubscribed_resources`, with leads to an update to the oversubscribed resources when people are using resource provider. I'll stick to the existing code. I'll think about a cleanup to generate those messages without looking into member fields (instead, making them a pure helper). - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64561/#review193671 ----------------------------------------------------------- On Dec. 13, 2017, 12:56 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64561/ > ----------------------------------------------------------- > > (Updated Dec. 13, 2017, 12:56 a.m.) > > > Review request for mesos, Benjamin Bannier and Jan Schlicht. > > > Repository: mesos > > > Description > ------- > > Given that now we use `UpdateSlaveMessage` to send resource provider > information directly, having resource categories in the message is > unnecessary and misleading. > > Instead, this patch introduced a single optional boolean to indicate if > oversubscribed resources need to be updated or not. > > > Diffs > ----- > > src/master/master.hpp 232cc3758f240db626c4fdaf852163fa48af4dd7 > src/master/master.cpp efe8b8f1704b314e6e6a4d5632718cab2854e38f > src/messages/messages.proto e680cd5e4d5a93c3c77309f327844f55fbb239a1 > src/slave/slave.hpp 7c40fc71b49057fea0cfd85290931fbd0f6a9d62 > src/slave/slave.cpp 5869e73ca1c14c99e580da9d7375181da2073ec5 > src/tests/oversubscription_tests.cpp > 3f57ce105e24e9f9cd681d8d984dbe242aa51f75 > > > Diff: https://reviews.apache.org/r/64561/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
