mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gastón Kleiman <gas...@mesosphere.io>
Subject Re: Review Request 69978: Added garbage collection of terminated operations status update streams.
Date Thu, 21 Feb 2019 19:44:57 GMT


> On Feb. 20, 2019, 6:13 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Lines 7452 (patched)
> > <https://reviews.apache.org/r/69978/diff/3/?file=2126000#file2126000line7457>
> >
> >     Is it possible that the stream could still be present on disk, but the operation
would not be found in the checkpoint file?
> >     
> >     Since we use the directories found on disk to recover the SUM but we use the
recovered operation state when calling `addOperation()`, perhaps this call to `completedOperations.push_back(uuid);`
should be moved outside of this conditional, so that we GC the stream regardless of whether
or not the operation was present in the agent's checkpoint file?

This RR depends on https://reviews.apache.org/r/69977/ which makes `Slave::_recoverOperations()`
handle that case.


- Gastón


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69978/#review213008
-----------------------------------------------------------


On Feb. 20, 2019, 4:43 p.m., Gastón Kleiman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69978/
> -----------------------------------------------------------
> 
> (Updated Feb. 20, 2019, 4:43 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-9574
>     https://issues.apache.org/jira/browse/MESOS-9574
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Make the agent garbage collect an operation status update stream once
> a terminal status update is acknowledged.
> 
> This patch also improves the logging of acknowledgement failures and the
> readability of the `Slave::operationStatusAcknowledgement` method.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.cpp e3c2c005d865b5c333e92e50e49ef398fe06ad79 
> 
> 
> Diff: https://reviews.apache.org/r/69978/diff/3/
> 
> 
> Testing
> -------
> 
> Manual testing + existing tests still pass.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>


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