mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexander Rojas" <alexan...@mesosphere.io>
Subject Re: Review Request 30074: Added "max allowed age" to Slave state.json endpoint
Date Thu, 19 Feb 2015 09:03:12 GMT


> On Feb. 18, 2015, 9:16 p.m., Niklas Nielsen wrote:
> > src/slave/slave.cpp, lines 3852-3854
> > <https://reviews.apache.org/r/30074/diff/8/?file=867823#file867823line3852>
> >
> >     Can you help me understand the second part of this. What does 'as if ... were
using no disk'? Think we need to sharpen this comment a little bit
> 
> Alexander Rojas wrote:
>     Funny, I rewrote that comment many times. The problem here is, if usage is not set
a default value is needed, so I asume `usage=0` and get the max allowed age of that case (which
for all x >= 0 age(x) <= age(0)).

I just updated the comment, but I would like to see what you think about it now.


> On Feb. 18, 2015, 9:16 p.m., Niklas Nielsen wrote:
> > src/slave/slave.cpp, line 3855
> > <https://reviews.apache.org/r/30074/diff/8/?file=867823#file867823line3855>
> >
> >     Max age of 0 seconds is hard to tell whether is a very unhealthy system or "no
value present". Wonder if we should leave a todo if the metrics system doesn't support it.
> 
> Alexander Rukletsov wrote:
>     Observing the new approach, I would like to circle back with my autistic suggestion:
shall we introduce a sane default for `executorDirectoryMaxAllowedAge` and do not make it
optional?
> 
> Alexander Rojas wrote:
>     Actually, now we need it, because `metrics` requires it. I though age(0).secs() would
be a good default value, but it is open to sugestions.
> 
> Alexander Rukletsov wrote:
>     It looks like we do not use optional fields in Metric structures, hence a default
is probably a good option. How about `Flags::gc_delay`?
> 
> Alexander Rojas wrote:
>     That one sounds good too.

So I follow Alex's suggestion of using `Flags::gc_delay`. But if someone has any idea of what
the value should be, still open to comments.


- Alexander


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


On Feb. 19, 2015, 10:03 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2015, 10:03 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Till Toenshoff.
> 
> 
> Bugs: mesos-2229
>     https://issues.apache.org/jira/browse/mesos-2229
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Adds and entry `max_allowed_age_secs` to the response of the `state.json` endpoint in
the slave. 
> 
> The max allowed age value is available only if it is set, which depends on the flag `--disk_watch_interval`
which by default runs every minute (So it will only be available after the first minute running).
> 
> Modifies the unit test `SlaveTest.StateEndpoint` so it takes into account the new entry.
> 
> 
> Diffs
> -----
> 
>   src/slave/metrics.hpp e2ce845ec2229e1e6e71c710022551bec9d15029 
>   src/slave/metrics.cpp ee610b8a7d352c20e8a91bb86c8aa2c1f011e910 
>   src/slave/slave.hpp 7a399f6df50c69b7e1e12d74f076fa57b6edb1b3 
>   src/slave/slave.cpp ec7ec1356e745bb07484ae1755c9183b038043b3 
>   src/tests/slave_tests.cpp 21709e2858d91a2746e0343786f73dec22735da6 
> 
> Diff: https://reviews.apache.org/r/30074/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


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