You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Alexander Rojas <al...@mesosphere.io> on 2015/01/20 14:39:37 UTC
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/
-----------------------------------------------------------
(Updated Jan. 20, 2015, 1:39 p.m.)
Review request for mesos, Niklas Nielsen and Till Toenshoff.
Bugs: mesos-2229
https://issues.apache.org/jira/browse/mesos-2229
Repository: mesos-git
Description
-------
Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
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.MetricsInStatsEndpoint` so it takes into account the new entry.
Diffs
-----
src/slave/http.cpp d1cf8a6
src/slave/slave.hpp 70bd8c1
src/slave/slave.cpp fca83b3
src/tests/slave_tests.cpp a85a028
Diff: https://reviews.apache.org/r/30074/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review68906
-----------------------------------------------------------
Patch looks great!
Reviews applied: [30074]
All tests passed.
- Mesos ReviewBot
On Jan. 21, 2015, 10:23 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2015, 10:23 a.m.)
>
>
> Review request for mesos, Niklas Nielsen and Till Toenshoff.
>
>
> Bugs: mesos-2229
> https://issues.apache.org/jira/browse/mesos-2229
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp d1cf8a6
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp fca83b3
> src/tests/slave_tests.cpp a85a028
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review68919
-----------------------------------------------------------
Patch looks great!
Reviews applied: [30074]
All tests passed.
- Mesos ReviewBot
On Jan. 21, 2015, 1:55 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2015, 1:55 p.m.)
>
>
> Review request for mesos, Niklas Nielsen and Till Toenshoff.
>
>
> Bugs: mesos-2229
> https://issues.apache.org/jira/browse/mesos-2229
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp d1cf8a6
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp fca83b3
> src/tests/slave_tests.cpp a85a028
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Alexander Rukletsov <al...@mesosphere.io>.
> On Jan. 28, 2015, 6:17 p.m., Niklas Nielsen wrote:
> > src/slave/slave.cpp, line 132
> > <https://reviews.apache.org/r/30074/diff/3/?file=828310#file828310line132>
> >
> > You don't have to initialize Option types with None. They will automatically be that :)
This is more explicit though. Do we have an agreement for such cases?
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review70029
-----------------------------------------------------------
On Jan. 21, 2015, 1:55 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2015, 1:55 p.m.)
>
>
> Review request for mesos, Niklas Nielsen and Till Toenshoff.
>
>
> Bugs: mesos-2229
> https://issues.apache.org/jira/browse/mesos-2229
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp d1cf8a6
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp fca83b3
> src/tests/slave_tests.cpp a85a028
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
> On Jan. 28, 2015, 7:17 p.m., Niklas Nielsen wrote:
> > src/slave/http.cpp, lines 273-276
> > <https://reviews.apache.org/r/30074/diff/3/?file=828308#file828308line273>
> >
> > Why stats and not state.json?
sorry, my bad.
> On Jan. 28, 2015, 7:17 p.m., Niklas Nielsen wrote:
> > src/slave/http.cpp, line 274
> > <https://reviews.apache.org/r/30074/diff/3/?file=828308#file828308line274>
> >
> > We should think about how to get the fact in, that it integer is a number of seconds.
> >
> > How about something like:
> >
> > max_allowed_directory_age_seconds = 42
> > or
> > max_allowed_directory_seconds = 42
> >
> > ?
Yeah, I was wondering about that too, I like that idea too. Too keep it consistant with https://github.com/apache/mesos/blob/master/src/slave/metrics.cpp#L35 I will use the `_secs` prefix
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review70029
-----------------------------------------------------------
On Jan. 21, 2015, 2:55 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2015, 2:55 p.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` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp d1cf8a6
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp fca83b3
> src/tests/slave_tests.cpp a85a028
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Niklas Nielsen <ni...@qni.dk>.
> On Jan. 28, 2015, 10:17 a.m., Niklas Nielsen wrote:
> > src/slave/slave.cpp, line 132
> > <https://reviews.apache.org/r/30074/diff/3/?file=828310#file828310line132>
> >
> > You don't have to initialize Option types with None. They will automatically be that :)
>
> Alexander Rukletsov wrote:
> This is more explicit though. Do we have an agreement for such cases?
Feel free to drop - think we do both: https://github.com/apache/mesos/blob/master/src/slave/slave.cpp#L127 vs https://github.com/apache/mesos/blob/master/src/slave/slave.hpp#L406
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review70029
-----------------------------------------------------------
On Jan. 21, 2015, 5:55 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2015, 5:55 a.m.)
>
>
> Review request for mesos, Niklas Nielsen and Till Toenshoff.
>
>
> Bugs: mesos-2229
> https://issues.apache.org/jira/browse/mesos-2229
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp d1cf8a6
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp fca83b3
> src/tests/slave_tests.cpp a85a028
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Alexander Rukletsov <al...@mesosphere.io>.
> On Jan. 28, 2015, 6:17 p.m., Niklas Nielsen wrote:
> > src/slave/http.cpp, line 274
> > <https://reviews.apache.org/r/30074/diff/3/?file=828308#file828308line274>
> >
> > We should think about how to get the fact in, that it integer is a number of seconds.
> >
> > How about something like:
> >
> > max_allowed_directory_age_seconds = 42
> > or
> > max_allowed_directory_seconds = 42
> >
> > ?
>
> Alexander Rojas wrote:
> Yeah, I was wondering about that too, I like that idea too. Too keep it consistant with https://github.com/apache/mesos/blob/master/src/slave/metrics.cpp#L35 I will use the `_secs` prefix
Feel free to drop this, but I do not like `_secs` prefix. There is no [such unit in SI](https://en.wikipedia.org/wiki/SI_base_unit#The_seven_SI_base_units). Note that we also use `_seconds` postfix.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review70029
-----------------------------------------------------------
On Feb. 4, 2015, 3:40 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2015, 3:40 p.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` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 0395d00
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp a8b2621
> src/tests/slave_tests.cpp e7e2af6
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review70029
-----------------------------------------------------------
Thanks for submitting this Alexander!
I have a few questions/suggestions below :)
src/slave/http.cpp
<https://reviews.apache.org/r/30074/#comment114961>
Why stats and not state.json?
src/slave/http.cpp
<https://reviews.apache.org/r/30074/#comment114967>
We should think about how to get the fact in, that it integer is a number of seconds.
How about something like:
max_allowed_directory_age_seconds = 42
or
max_allowed_directory_seconds = 42
?
src/slave/slave.hpp
<https://reviews.apache.org/r/30074/#comment114960>
We can make this a bit more precise. It is not just any files, but the executor directories/sandboxes.
How about having something like:
// Maximum age of executor directories. Will be recomputed periodically every flags.disk_watch_interval.
src/slave/slave.cpp
<https://reviews.apache.org/r/30074/#comment114966>
You don't have to initialize Option types with None. They will automatically be that :)
src/slave/slave.cpp
<https://reviews.apache.org/r/30074/#comment114955>
You don't need to use some() here. You can just set the value:
maxAllowedAge = age(usage.get());
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/30074/#comment114964>
Instead of setting this low (which would still cause a 1s+ test). How about forwarding the clock with flags.disk_watch_interval? That should trigger the timeout immediately
- Niklas Nielsen
On Jan. 21, 2015, 5:55 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2015, 5:55 a.m.)
>
>
> Review request for mesos, Niklas Nielsen and Till Toenshoff.
>
>
> Bugs: mesos-2229
> https://issues.apache.org/jira/browse/mesos-2229
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp d1cf8a6
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp fca83b3
> src/tests/slave_tests.cpp a85a028
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
> On Feb. 4, 2015, 4:50 p.m., Alexander Rukletsov wrote:
> >
I was specifically requested not to do so. If I decided to do these cleanups, I am supposed to provide a patch which does them.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review70969
-----------------------------------------------------------
On Feb. 4, 2015, 4:40 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2015, 4:40 p.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` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 0395d00
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp a8b2621
> src/tests/slave_tests.cpp e7e2af6
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review70969
-----------------------------------------------------------
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/30074/#comment116443>
Since you're editing this test, let's do a fly-by cleanup. s/> >/>>/
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/30074/#comment116444>
ditto
- Alexander Rukletsov
On Feb. 4, 2015, 3:40 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2015, 3:40 p.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` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 0395d00
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp a8b2621
> src/tests/slave_tests.cpp e7e2af6
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review70983
-----------------------------------------------------------
Patch looks great!
Reviews applied: [30074]
All tests passed.
- Mesos ReviewBot
On Feb. 4, 2015, 3:40 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2015, 3:40 p.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` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 0395d00
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp a8b2621
> src/tests/slave_tests.cpp e7e2af6
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Ben Mahler <be...@gmail.com>.
> On Feb. 13, 2015, 6:09 p.m., Ben Mahler wrote:
> > I'm curiuos, why not add a metric for this, instead of adding it to state.json?
The review subject says "stats.json" (which is deprecated in favor of metrics), but this was added to "state.json" in the diff?
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review72399
-----------------------------------------------------------
On Feb. 5, 2015, 3:12 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2015, 3:12 p.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` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 0395d00
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp a8b2621
> src/tests/slave_tests.cpp 956ce64
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Ben Mahler <be...@gmail.com>.
> On Feb. 13, 2015, 6:09 p.m., Ben Mahler wrote:
> > I'm curiuos, why not add a metric for this, instead of adding it to state.json?
>
> Ben Mahler wrote:
> The review subject says "stats.json" (which is deprecated in favor of metrics), but this was added to "state.json" in the diff?
>
> Alexander Rojas wrote:
> The question is where to add it. It is truth the original ticket request it to be on the `stats.json`, but as nnielsen pointed out, it is more a description of the server's state and as such it should be on the server. I will update the description of the review.
It can't go into stats.json because the stats.json endpoints are deprecated in favor of using `Metric`s, which get exposed on `/metrics/snapshot`. Feel free to help us fence it off more clearly in the code! :)
What do you mean by "be on the server"? Is stats.json not on the server? I'd like to understand this distinction as "max_allowed_gc_age_secs" seems as equally valid a Metric as "uptime_secs", "recovery_errors", etc. :)
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review72399
-----------------------------------------------------------
On Feb. 14, 2015, 11:20 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2015, 11:20 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/http.cpp 0395d00
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp a8b2621
> src/tests/slave_tests.cpp 956ce64
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
> On Feb. 13, 2015, 7:09 p.m., Ben Mahler wrote:
> > I'm curiuos, why not add a metric for this, instead of adding it to state.json?
>
> Ben Mahler wrote:
> The review subject says "stats.json" (which is deprecated in favor of metrics), but this was added to "state.json" in the diff?
The question is where to add it. It is truth the original ticket request it to be on the `stats.json`, but as nnielsen pointed out, it is more a description of the server's state and as such it should be on the server. I will update the description of the review.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review72399
-----------------------------------------------------------
On Feb. 14, 2015, 12:20 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2015, 12:20 p.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/http.cpp 0395d00
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp a8b2621
> src/tests/slave_tests.cpp 956ce64
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
> On Feb. 13, 2015, 7:09 p.m., Ben Mahler wrote:
> > I'm curiuos, why not add a metric for this, instead of adding it to state.json?
>
> Ben Mahler wrote:
> The review subject says "stats.json" (which is deprecated in favor of metrics), but this was added to "state.json" in the diff?
>
> Alexander Rojas wrote:
> The question is where to add it. It is truth the original ticket request it to be on the `stats.json`, but as nnielsen pointed out, it is more a description of the server's state and as such it should be on the server. I will update the description of the review.
>
> Ben Mahler wrote:
> It can't go into stats.json because the stats.json endpoints are deprecated in favor of using `Metric`s, which get exposed on `/metrics/snapshot`. Feel free to help us fence it off more clearly in the code! :)
>
> What do you mean by "be on the server"? Is stats.json not on the server? I'd like to understand this distinction as "max_allowed_gc_age_secs" seems as equally valid a Metric as "uptime_secs", "recovery_errors", etc. :)
Sorry, by server I meant master. Still, I am having problems understanding what goes into the metrics endpoint and a state endpoint. For what I see, everything seems to be being consolidated in the metrics endpoint. Am I right? can you help me understand to set the value on the right endpoint?
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review72399
-----------------------------------------------------------
On Feb. 14, 2015, 12:20 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2015, 12:20 p.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/http.cpp 0395d00
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp a8b2621
> src/tests/slave_tests.cpp 956ce64
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review72399
-----------------------------------------------------------
I'm curiuos, why not add a metric for this, instead of adding it to state.json?
- Ben Mahler
On Feb. 5, 2015, 3:12 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2015, 3:12 p.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` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 0395d00
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp a8b2621
> src/tests/slave_tests.cpp 956ce64
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review72373
-----------------------------------------------------------
Ship it!
Ship It!
- Till Toenshoff
On Feb. 5, 2015, 3:12 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2015, 3:12 p.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` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 0395d00
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp a8b2621
> src/tests/slave_tests.cpp 956ce64
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review72949
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Rukletsov
On Feb. 18, 2015, 10:36 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2015, 10:36 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/http.cpp ef6e64882282a9d7be252ccdc545ab96d6323c3b
> 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
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review72957
-----------------------------------------------------------
Patch looks great!
Reviews applied: [30074]
All tests passed.
- Mesos ReviewBot
On Feb. 18, 2015, 10:36 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2015, 10:36 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/http.cpp ef6e64882282a9d7be252ccdc545ab96d6323c3b
> 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
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review72968
-----------------------------------------------------------
Patch looks great!
Reviews applied: [30074]
All tests passed.
- Mesos ReviewBot
On Feb. 18, 2015, 3:09 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2015, 3:09 p.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
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
> 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?
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.
> 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
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)).
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review73003
-----------------------------------------------------------
On Feb. 18, 2015, 4:09 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2015, 4:09 p.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
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rukletsov <al...@mesosphere.io>.
> On Feb. 18, 2015, 8: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.
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
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review73003
-----------------------------------------------------------
On Feb. 18, 2015, 3:09 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2015, 3:09 p.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
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rukletsov <al...@mesosphere.io>.
> On Feb. 18, 2015, 8: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.
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
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review73003
-----------------------------------------------------------
On Feb. 18, 2015, 3:09 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2015, 3:09 p.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
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
> 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
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
> 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`?
That one sounds good too.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review73003
-----------------------------------------------------------
On Feb. 18, 2015, 4:09 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2015, 4:09 p.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
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review73003
-----------------------------------------------------------
The patch changed a bit since the first /state.json version. Holding back on the first given ship-it until we get the new version into a good shape
src/slave/slave.cpp
<https://reviews.apache.org/r/30074/#comment119130>
How about replacing age(usage.get()) here as well?
src/slave/slave.cpp
<https://reviews.apache.org/r/30074/#comment119129>
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
src/slave/slave.cpp
<https://reviews.apache.org/r/30074/#comment119132>
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.
- Niklas Nielsen
On Feb. 18, 2015, 7:09 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2015, 7:09 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
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review73126
-----------------------------------------------------------
Patch looks great!
Reviews applied: [30074]
All tests passed.
- Mesos ReviewBot
On Feb. 19, 2015, 9: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, 9: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
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review75261
-----------------------------------------------------------
Patch looks great!
Reviews applied: [30074]
All tests passed.
- Mesos ReviewBot
On March 4, 2015, 4:21 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated March 4, 2015, 4:21 p.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 0390461968112cac9ed84ec3cc23b264e8b4e9d7
> src/slave/metrics.cpp c32b4d8a1ff3bc5e5f5ca21eb78a036c9ea1a813
> src/slave/slave.hpp d476d850b195e917c5fcb5c82f69863b4f85e4b1
> src/slave/slave.cpp 2d52ea0c440fd530174b0e44a59c8ae68fa2616c
> src/tests/slave_tests.cpp 27b3410d25256951d685fa2a7f6d425b3e80709f
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review75873
-----------------------------------------------------------
Patch looks great!
Reviews applied: [30074]
All tests passed.
- Mesos ReviewBot
On March 10, 2015, 9:50 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated March 10, 2015, 9:50 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 0390461968112cac9ed84ec3cc23b264e8b4e9d7
> src/slave/metrics.cpp c32b4d8a1ff3bc5e5f5ca21eb78a036c9ea1a813
> src/slave/slave.hpp d476d850b195e917c5fcb5c82f69863b4f85e4b1
> src/slave/slave.cpp 364d911b086dfe1f15f76aa3888f99146aa8d876
> src/tests/slave_tests.cpp 27b3410d25256951d685fa2a7f6d425b3e80709f
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/
-----------------------------------------------------------
(Updated March 10, 2015, 10:50 a.m.)
Review request for mesos, Niklas Nielsen and Till Toenshoff.
Changes
-------
Since the metrics object doesn't allow optional values, the whole point of using an Optional value for the allowed age makes no sense. So now is just a duration with a default value of `age(0)`.
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 (updated)
-----
src/slave/metrics.hpp 0390461968112cac9ed84ec3cc23b264e8b4e9d7
src/slave/metrics.cpp c32b4d8a1ff3bc5e5f5ca21eb78a036c9ea1a813
src/slave/slave.hpp d476d850b195e917c5fcb5c82f69863b4f85e4b1
src/slave/slave.cpp 364d911b086dfe1f15f76aa3888f99146aa8d876
src/tests/slave_tests.cpp 27b3410d25256951d685fa2a7f6d425b3e80709f
Diff: https://reviews.apache.org/r/30074/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/
-----------------------------------------------------------
(Updated March 4, 2015, 5:21 p.m.)
Review request for mesos, Niklas Nielsen and Till Toenshoff.
Changes
-------
Changes from last reviewer.
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 (updated)
-----
src/slave/metrics.hpp 0390461968112cac9ed84ec3cc23b264e8b4e9d7
src/slave/metrics.cpp c32b4d8a1ff3bc5e5f5ca21eb78a036c9ea1a813
src/slave/slave.hpp d476d850b195e917c5fcb5c82f69863b4f85e4b1
src/slave/slave.cpp 2d52ea0c440fd530174b0e44a59c8ae68fa2616c
src/tests/slave_tests.cpp 27b3410d25256951d685fa2a7f6d425b3e80709f
Diff: https://reviews.apache.org/r/30074/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
> On March 3, 2015, 8:01 p.m., Niklas Nielsen wrote:
> > src/slave/slave.cpp, line 3850
> > <https://reviews.apache.org/r/30074/diff/9/?file=868894#file868894line3850>
> >
> > How about using Slave::age(0) i.e. Assume that tasks don't take up any space yet. That will be closer to the number users will get after it has been measured.
I used `Slave::age(0)` in a previous version of the patch, because it looked logical. However someone complained about how arbitrary that value was and requested to use the user given value. I did the change however, but the discussion may arise again.
> On March 3, 2015, 8:01 p.m., Niklas Nielsen wrote:
> > src/slave/slave.cpp, line 3452
> > <https://reviews.apache.org/r/30074/diff/9/?file=868894#file868894line3452>
> >
> > Even though age() returns a Duration, it's not clear from the calling code. We could make it explicit by introducing a CHECK()
> >
> > We could also have a local variable with the age which we set just before the delay so we don't have to .get().
> >
> > Bot no biggies, but to catch if someone changes the return type of age()
I'm not sure what you mean here. How would that check look? I did however added a local variable.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review75009
-----------------------------------------------------------
On March 4, 2015, 5:21 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated March 4, 2015, 5:21 p.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 0390461968112cac9ed84ec3cc23b264e8b4e9d7
> src/slave/metrics.cpp c32b4d8a1ff3bc5e5f5ca21eb78a036c9ea1a813
> src/slave/slave.hpp d476d850b195e917c5fcb5c82f69863b4f85e4b1
> src/slave/slave.cpp 2d52ea0c440fd530174b0e44a59c8ae68fa2616c
> src/tests/slave_tests.cpp 27b3410d25256951d685fa2a7f6d425b3e80709f
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review75009
-----------------------------------------------------------
src/slave/metrics.cpp
<https://reviews.apache.org/r/30074/#comment121931>
Let's preserve the ordering you have above :)
src/slave/metrics.cpp
<https://reviews.apache.org/r/30074/#comment121932>
Same here
src/slave/slave.hpp
<https://reviews.apache.org/r/30074/#comment121933>
Same as above.
src/slave/slave.cpp
<https://reviews.apache.org/r/30074/#comment121928>
Even though age() returns a Duration, it's not clear from the calling code. We could make it explicit by introducing a CHECK()
We could also have a local variable with the age which we set just before the delay so we don't have to .get().
Bot no biggies, but to catch if someone changes the return type of age()
src/slave/slave.cpp
<https://reviews.apache.org/r/30074/#comment121930>
How about using Slave::age(0) i.e. Assume that tasks don't take up any space yet. That will be closer to the number users will get after it has been measured.
src/slave/slave.cpp
<https://reviews.apache.org/r/30074/#comment121926>
No need for your else clause when you end the previous block with a return
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/30074/#comment121923>
Why this change?
- Niklas Nielsen
On Feb. 19, 2015, 1: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, 1: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
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
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.
Changes
-------
Sets `Flags::gc_delay` as the default value for the `max_allowed_age` entry.
Updates weirdly written comment, for a hopefully better one.
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 (updated)
-----
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
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/
-----------------------------------------------------------
(Updated Feb. 18, 2015, 4:09 p.m.)
Review request for mesos, Niklas Nielsen and Till Toenshoff.
Changes
-------
Better wording in comments.
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 (updated)
-----
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
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/
-----------------------------------------------------------
(Updated Feb. 18, 2015, 4:01 p.m.)
Review request for mesos, Niklas Nielsen and Till Toenshoff.
Changes
-------
Following Ben's and Till's suggestions, the value for `executor_directory_max_allowed_age_secs` now is part of metrics.
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 (updated)
-----
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
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/
-----------------------------------------------------------
(Updated Feb. 18, 2015, 11:36 a.m.)
Review request for mesos, Niklas Nielsen and Till Toenshoff.
Changes
-------
Sets the entry name to `executor_directory_max_allowed_age_secs`.
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 (updated)
-----
src/slave/http.cpp ef6e64882282a9d7be252ccdc545ab96d6323c3b
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
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review72855
-----------------------------------------------------------
Ship it!
src/slave/http.cpp
<https://reviews.apache.org/r/30074/#comment118973>
Which directory age? :) How about 'sandbox' or 'executor directory'. It's hard to interpret it's meaning only from the field name.
- Niklas Nielsen
On Feb. 14, 2015, 3:20 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2015, 3:20 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/http.cpp 0395d00
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp a8b2621
> src/tests/slave_tests.cpp 956ce64
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave state.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/
-----------------------------------------------------------
(Updated Feb. 14, 2015, 12:20 p.m.)
Review request for mesos, Niklas Nielsen and Till Toenshoff.
Changes
-------
Updated to show the endpoint where the entry is provided is not `stats.json` but `state.json`.
Summary (updated)
-----------------
Added "max allowed age" to Slave state.json endpoint
Bugs: mesos-2229
https://issues.apache.org/jira/browse/mesos-2229
Repository: mesos
Description (updated)
-------
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/http.cpp 0395d00
src/slave/slave.hpp 70bd8c1
src/slave/slave.cpp a8b2621
src/tests/slave_tests.cpp 956ce64
Diff: https://reviews.apache.org/r/30074/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review71224
-----------------------------------------------------------
Patch looks great!
Reviews applied: [30074]
All tests passed.
- Mesos ReviewBot
On Feb. 5, 2015, 3:12 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2015, 3:12 p.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` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 0395d00
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp a8b2621
> src/tests/slave_tests.cpp 956ce64
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/
-----------------------------------------------------------
(Updated Feb. 5, 2015, 4:12 p.m.)
Review request for mesos, Niklas Nielsen and Till Toenshoff.
Changes
-------
Rebases to latest master.
Bugs: mesos-2229
https://issues.apache.org/jira/browse/mesos-2229
Repository: mesos
Description
-------
Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
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.MetricsInStatsEndpoint` so it takes into account the new entry.
Diffs (updated)
-----
src/slave/http.cpp 0395d00
src/slave/slave.hpp 70bd8c1
src/slave/slave.cpp a8b2621
src/tests/slave_tests.cpp 956ce64
Diff: https://reviews.apache.org/r/30074/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/
-----------------------------------------------------------
(Updated Feb. 4, 2015, 4:40 p.m.)
Review request for mesos, Niklas Nielsen and Till Toenshoff.
Changes
-------
Makes changes based on reviews.
Bugs: mesos-2229
https://issues.apache.org/jira/browse/mesos-2229
Repository: mesos
Description
-------
Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
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.MetricsInStatsEndpoint` so it takes into account the new entry.
Diffs (updated)
-----
src/slave/http.cpp 0395d00
src/slave/slave.hpp 70bd8c1
src/slave/slave.cpp a8b2621
src/tests/slave_tests.cpp e7e2af6
Diff: https://reviews.apache.org/r/30074/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/
-----------------------------------------------------------
(Updated Jan. 21, 2015, 1:55 p.m.)
Review request for mesos, Niklas Nielsen and Till Toenshoff.
Changes
-------
The units of `max_allowed_directory_age` are always in seconds.
Bugs: mesos-2229
https://issues.apache.org/jira/browse/mesos-2229
Repository: mesos-git
Description
-------
Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
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.MetricsInStatsEndpoint` so it takes into account the new entry.
Diffs (updated)
-----
src/slave/http.cpp d1cf8a6
src/slave/slave.hpp 70bd8c1
src/slave/slave.cpp fca83b3
src/tests/slave_tests.cpp a85a028
Diff: https://reviews.apache.org/r/30074/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/
-----------------------------------------------------------
(Updated Jan. 21, 2015, 10:23 a.m.)
Review request for mesos, Niklas Nielsen and Till Toenshoff.
Changes
-------
Changes after revew by Alexander Rukletsov
Bugs: mesos-2229
https://issues.apache.org/jira/browse/mesos-2229
Repository: mesos-git
Description
-------
Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
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.MetricsInStatsEndpoint` so it takes into account the new entry.
Diffs (updated)
-----
src/slave/http.cpp d1cf8a6
src/slave/slave.hpp 70bd8c1
src/slave/slave.cpp fca83b3
src/tests/slave_tests.cpp a85a028
Diff: https://reviews.apache.org/r/30074/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
> On Jan. 20, 2015, 2:06 p.m., Alexander Rukletsov wrote:
> > src/slave/slave.cpp, line 3374
> > <https://reviews.apache.org/r/30074/diff/1/?file=826834#file826834line3374>
> >
> > It looks like at this point the future is ready and therefore usage is always available. Can we avoid `Option<>` then by providing a sane default until `maxAllowedAge` is loaded?
I like the idea, the question is, which value is a good default?
> On Jan. 20, 2015, 2:06 p.m., Alexander Rukletsov wrote:
> > src/tests/slave_tests.cpp, line 830
> > <https://reviews.apache.org/r/30074/diff/1/?file=826835#file826835line830>
> >
> > Why `3`? Can we use `Clock::advance` instead?
I think `FUTURE_DISPATCH` works better
> On Jan. 20, 2015, 2:06 p.m., Alexander Rukletsov wrote:
> > src/tests/slave_tests.cpp, line 828
> > <https://reviews.apache.org/r/30074/diff/1/?file=826835#file826835line828>
> >
> > What is `max_allowed_age`?
I changed the name to `max_allowed_directory_age` which I hope gives more information. It represents the maximum age of a directory before it is scheduled for deletion.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review68716
-----------------------------------------------------------
On Jan. 20, 2015, 1:39 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 1:39 p.m.)
>
>
> Review request for mesos, Niklas Nielsen and Till Toenshoff.
>
>
> Bugs: mesos-2229
> https://issues.apache.org/jira/browse/mesos-2229
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp d1cf8a6
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp fca83b3
> src/tests/slave_tests.cpp a85a028
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review68716
-----------------------------------------------------------
A high-level question: should `maxAllowedAge` be optional? Does it makes sense to have or provide a sane default?
src/slave/slave.cpp
<https://reviews.apache.org/r/30074/#comment113096>
Can we use simply `None()` instead?
src/slave/slave.cpp
<https://reviews.apache.org/r/30074/#comment113097>
It looks like at this point the future is ready and therefore usage is always available. Can we avoid `Option<>` then by providing a sane default until `maxAllowedAge` is loaded?
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/30074/#comment113098>
What is `max_allowed_age`?
src/tests/slave_tests.cpp
<https://reviews.apache.org/r/30074/#comment113099>
Why `3`? Can we use `Clock::advance` instead?
- Alexander Rukletsov
On Jan. 20, 2015, 1:39 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 1:39 p.m.)
>
>
> Review request for mesos, Niklas Nielsen and Till Toenshoff.
>
>
> Bugs: mesos-2229
> https://issues.apache.org/jira/browse/mesos-2229
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp d1cf8a6
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp fca83b3
> src/tests/slave_tests.cpp a85a028
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Alexander Rojas <al...@mesosphere.io>.
> On Jan. 21, 2015, 2:29 a.m., Sunil Abraham wrote:
> > I think the stats.json endpoint should report the max allowed age in type `double` not `double string`, to keep the stats.json key:value type as `string`:`double`. This could be achieved by having the value be either in seconds or milliseconds.
I like the idea on principle, but then it comes the question of how to make the units obvious. One example would be to have the name be something like `max_allowed_age_s` or `max_allowed_age_ms`. Another option to document somewhere with big letters the unit system, or (and I don't know about it) keep all units the same. I like the last one the most, but I don't know if we do that already.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review68848
-----------------------------------------------------------
On Jan. 21, 2015, 10:23 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2015, 10:23 a.m.)
>
>
> Review request for mesos, Niklas Nielsen and Till Toenshoff.
>
>
> Bugs: mesos-2229
> https://issues.apache.org/jira/browse/mesos-2229
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp d1cf8a6
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp fca83b3
> src/tests/slave_tests.cpp a85a028
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Sunil Abraham <s....@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review68848
-----------------------------------------------------------
I think the stats.json endpoint should report the max allowed age in type `double` not `double string`, to keep the stats.json key:value type as `string`:`double`. This could be achieved by having the value be either in seconds or milliseconds.
- Sunil Abraham
On Jan. 20, 2015, 1:39 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 1:39 p.m.)
>
>
> Review request for mesos, Niklas Nielsen and Till Toenshoff.
>
>
> Bugs: mesos-2229
> https://issues.apache.org/jira/browse/mesos-2229
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp d1cf8a6
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp fca83b3
> src/tests/slave_tests.cpp a85a028
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 30074: Added "max allowed age" to Slave stats.json
endpoint
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30074/#review68721
-----------------------------------------------------------
Patch looks great!
Reviews applied: [30074]
All tests passed.
- Mesos ReviewBot
On Jan. 20, 2015, 1:39 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30074/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 1:39 p.m.)
>
>
> Review request for mesos, Niklas Nielsen and Till Toenshoff.
>
>
> Bugs: mesos-2229
> https://issues.apache.org/jira/browse/mesos-2229
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds and entry `max_allowed_age` to the response of the stats.json endpoint in the slave. The format of the value is `{double}(days|mins|secs)`.
>
> 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.MetricsInStatsEndpoint` so it takes into account the new entry.
>
>
> Diffs
> -----
>
> src/slave/http.cpp d1cf8a6
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp fca83b3
> src/tests/slave_tests.cpp a85a028
>
> Diff: https://reviews.apache.org/r/30074/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>