> On Dec. 12, 2017, 1:08 a.m., Jie Yu wrote: > > src/slave/slave.cpp > > Lines 7296-7298 (patched) > > <https://reviews.apache.org/r/64505/diff/1/?file=1912670#file1912670line7296> > > > > Wondering if we should do this check in `updateOfferOperation`? > > > > What's the equality check for `OfferOperationStatus`? does the order in > > `Resources` matter? > > > > Also, wondering what we do for Tasks in this situation?
Yea I considered putting this in `updateOfferOperation` instead. In the absence of other motivations, I opted for what seems like better readability: the function `updateOfferOperation` is used to update the agent's representation of an offer operation, and if we don't need to update, then we don't call it at all. LMK what you think. Regarding equality, I can update the patch to just check the `status_uuid`. Regarding tasks, I'll quote here from our offline discussion: "in the task case, the `Slave::forward` function is only responsible for sending the update, not updating the agent’s internal task state - the agent’s state is updated in the `statusUpdate` code path (which also calls `taskStatusUpdateManager->update()`). in the offer operation case, the LRP’s forwarding function submits an agent call, and then the agent’s handler is responsible for _both_ sending the update message and updating the agent’s internal state. thus, in the task case we don’t have this issue - the `statusUpdate` code path is only executed once for each update, whereas for offer operations the code path which updates the agent’s state is executed every time we forward an update, including retries." - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64505/#review193476 ----------------------------------------------------------- On Dec. 11, 2017, 8:17 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64505/ > ----------------------------------------------------------- > > (Updated Dec. 11, 2017, 8:17 p.m.) > > > Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht. > > > Repository: mesos > > > Description > ------- > > Local resource providers send all offer operation status updates > using the reosurce provider API. This patch makes the agent's > resource provider message handler skip operation updates when an > update is a retry. > > > Diffs > ----- > > src/slave/slave.cpp 373e393ca1e7c0c30c3474cc9e630e25ad92f235 > > > Diff: https://reviews.apache.org/r/64505/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Greg Mann > >
