----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69858/#review212547 -----------------------------------------------------------
src/resource_provider/storage/provider.cpp Lines 2889 (patched) <https://reviews.apache.org/r/69858/#comment298357> Why is this always true? Probably a good idea to document this here if we make this assertion. I'd suggest to not make this assertion and instead simplify the control flow, see below. src/resource_provider/storage/provider.cpp Lines 2888-2892 (original), 2898-2910 (patched) <https://reviews.apache.org/r/69858/#comment298358> What do you think about constructing a single update outside of any condition (like was done previously), and then only have branching depending on whether we want to use the status update manager or send unreliably? I think this would read much better if we'd: * gather all required input to create the status update in variables, * construct the status update, and * a single `if/else` for the sending. * update metrics in a single place src/resource_provider/storage/provider.cpp Lines 2923 (patched) <https://reviews.apache.org/r/69858/#comment298359> We could introduce a variable for this. src/resource_provider/storage/provider_process.hpp Line 298 (original), 298-300 (patched) <https://reviews.apache.org/r/69858/#comment298360> It isn't clear from the second sentence whether you document behavior or give a prescription on how to use this function. - Benjamin Bannier On Jan. 30, 2019, 11:35 p.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69858/ > ----------------------------------------------------------- > > (Updated Jan. 30, 2019, 11:35 p.m.) > > > Review request for mesos, Benjamin Bannier, Gastón Kleiman, and Greg Mann. > > > Bugs: MESOS-9537 > https://issues.apache.org/jira/browse/MESOS-9537 > > > Repository: mesos > > > Description > ------- > > If an operation is dropped intentionally (e.g., because of a resource > version mismatch), the operation should be persisted so no conflicting > status update would be generated for operation reconciliation. > > > Diffs > ----- > > src/resource_provider/storage/provider.cpp > d6e20a549ede189c757ae3ae922ab7cb86d2be2c > src/resource_provider/storage/provider_process.hpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/69858/diff/3/ > > > Testing > ------- > > `make check` > > Additional test MESOS-9537 is added later in chain. > > > Thanks, > > Chun-Hung Hsiao > >
