> On Nov. 21, 2017, 10:22 p.m., Jie Yu wrote: > > src/master/master.hpp > > Lines 2919 (patched) > > <https://reviews.apache.org/r/63842/diff/4/?file=1898995#file1898995line2919> > > > > `removeOfferOperation` might be used for old operations too. This will > > throw a CHECK failure. > > Benjamin Bannier wrote: > Dropping this as the code was removed. Now that `addOfferOperation` does > not modify resources anymore, neither should `removeOfferOperation`.
`addOfferOperation` does not convert `total`. It'll still be responsible for tracking 'used' resources. The reason we don't update `total` in `addOfferOperation` is because we want to support setting total to a snapshot when receiving update slave message. However, tracking 'used' should be orthogonal, and we should still rely on `addOfferOperation` to track used resources. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63842/#review191664 ----------------------------------------------------------- On Nov. 24, 2017, 2:52 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63842/ > ----------------------------------------------------------- > > (Updated Nov. 24, 2017, 2:52 p.m.) > > > Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht. > > > Repository: mesos > > > Description > ------- > > During reconcilation we might be required to remove non-terminal offer > operations from bookkeeping. > > > Diffs > ----- > > src/master/master.hpp 2a2e830354db4a2191fb8321beb8174b80f7ba7d > src/master/master.cpp 53263e499d88b906b6406c24c0dfb737e589e813 > src/slave/slave.cpp f93ff7b20815c3ccb274ce6990ee66a17b6ac51c > > > Diff: https://reviews.apache.org/r/63842/diff/6/ > > > Testing > ------- > > `make check`, tested as part of https://reviews.apache.org/r/63843/. > > This patch requires `protobuf::isSpeculativeOperation` from > https://reviews.apache.org/r/63751/ which is _not_ part of this review chain > (to avoid multiple dependent RRs). > > > Thanks, > > Benjamin Bannier > >
