> On Feb. 5, 2019, 12:08 p.m., Benjamin Bannier wrote: > > src/resource_provider/storage/provider.cpp > > Lines 2889 (patched) > > <https://reviews.apache.org/r/69858/diff/3/?file=2123315#file2123315line2889> > > > > 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.
Hmm good point. If we support operator API in the future than this assertion won't hold. > On Feb. 5, 2019, 12:08 p.m., Benjamin Bannier wrote: > > src/resource_provider/storage/provider.cpp > > Lines 2888-2892 (original), 2898-2910 (patched) > > <https://reviews.apache.org/r/69858/diff/3/?file=2123315#file2123315line2898> > > > > 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 I come up with a slightly different refactor. Please give feedback if it's still not readable :) Leaving this open for now. > On Feb. 5, 2019, 12:08 p.m., Benjamin Bannier wrote: > > src/resource_provider/storage/provider.cpp > > Lines 2923 (patched) > > <https://reviews.apache.org/r/69858/diff/3/?file=2123315#file2123315line2923> > > > > We could introduce a variable for this. Resolved through a different refactor. Closing. - Chun-Hung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69858/#review212547 ----------------------------------------------------------- On Jan. 30, 2019, 10: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, 10: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 > >
