mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chun-Hung Hsiao <chhs...@apache.org>
Subject Re: Review Request 69858: Persisted intentionally dropped operations in SLRP.
Date Tue, 05 Feb 2019 23:52:04 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message