and will get this
committed shortly.
- Ben Mahler
On March 21, 2016, 6:14 a.m., Abhishek Dasgupta wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache
verResources(
allocation->frameworkId,
agent.id(),
allocation->resources.get(agent.id()).get(),
offerFilter);
```
- Ben Mahler
On March 24, 2016, 1:36 p.m., Benjamin Bannier wrote:
>
>
://issues.apache.org/jira/browse/MESOS-5021
Repository: mesos
Description
---
See summary.
Diffs
-
3rdparty/libprocess/src/subprocess.cpp
16c327db6fde1a1bac92d9e8f7cc80e57b028c1a
Diff: https://reviews.apache.org/r/45267/diff/
Testing
---
make check
Thanks,
Ben Mahler
/#comment187976>
It is the executor's responsiblitity to enforce kill policies
- Ben Mahler
On March 23, 2016, 11:26 p.m., Alexander Rukletsov wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit
out to impact the kill timeout since no shutdown is occuring, so that might
surprise others and its worth saying why we do this.
- Ben Mahler
On March 23, 2016, 11:25 p.m., Alexander Rukletsov wrote:
>
> ---
> This is an automati
)
<https://reviews.apache.org/r/44994/#comment187967>
s/TimedOut/Timeout/
src/tests/slave_tests.cpp (lines 3308 - 3309)
<https://reviews.apache.org/r/44994/#comment187968>
AWAIT_EXPECT_EQ
- Ben Mahler
On March 23, 2016, 11:22 p.m., Alexander Ruk
p (line 631)
<https://reviews.apache.org/r/44660/#comment186855>
removing flags is also difficult :)
- Ben Mahler
On March 23, 2016, 11:25 p.m., Alexander Rukletsov wrote:
>
> ---
&g
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45242/#review125127
---
Ship it!
Ship It!
- Ben Mahler
On March 23, 2016, 7:52 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45240/#review125125
---
Ship it!
Ship It!
- Ben Mahler
On March 23, 2016, 7:50 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45241/#review125126
---
Ship it!
Ship It!
- Ben Mahler
On March 23, 2016, 7:50 p.m
> On March 18, 2016, 11:24 p.m., Ben Mahler wrote:
> > src/tests/slave_tests.cpp, line 3264
> > <https://reviews.apache.org/r/44994/diff/2/?file=1306236#file1306236line3264>
> >
> > You don't need a settle here, AWAIT_READY will settle if the clock is
/master_validation_tests.cpp (lines 1175 - 1176)
<https://reviews.apache.org/r/44854/#comment187833>
Why the explicit detector?
- Ben Mahler
On March 22, 2016, 5:05 p.m., Alexander Rukletsov wrote:
>
> ---
> This is a
d2ab6ed187ec0d0e5dbac92607b612bb55b1a682
Diff: https://reviews.apache.org/r/45151/diff/
Testing
---
make check
manually validated the upgrade / backwards compatibility of enum fields
Thanks,
Ben Mahler
I would have rather we did a
consistent change across the tests, but not your fault, we can stick with agent
here since this file seems to use it in many places.
src/tests/hierarchical_allocator_tests.cpp (lines 2736 - 2737)
<https://reviews.apache.org/r/43882/#comment187705>
Try to
would be consistent
with the comments I made on the previous patches.
Also, why not use your allocations variable? s/0/allocations/
- Ben Mahler
On March 21, 2016, 11:08 a.m., Benjamin Bannier wrote:
>
> ---
&
{"allocator/mesos/resources/mem/offered_or_allocated", 0},
{"allocator/mesos/resources/disk/offered_or_allocated", 0},
};
metrics = Metrics();
EXPECT_TRUE(metrics.contains(expected));
}
```
- Ben Mahler
On March 21, 2016, 11:08 a.m., Benjamin Bannier wrote:
>
>
esis
src/master/allocator/mesos/metrics.cpp (line 103)
<https://reviews.apache.org/r/43884/#comment187546>
you can omit the 'allocator' here to use a deferred context where
libprocess manages the execution, see here:
https://github.
> On March 22, 2016, 5:50 p.m., Ben Mahler wrote:
> > The changelog update would be great as well here, I'll take care of that
> > before committing. Thanks!
I'll also update the existing test:
```
$ grep -R allocator/event_queue_dispatches src/tests
src/tests/master
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44852/#review124843
---
Ship it!
Ship It!
- Ben Mahler
On March 18, 2016, 4:08 p.m
well here, I'll take care of that before
committing. Thanks!
docs/upgrades.md (line 157)
<https://reviews.apache.org/r/44851/#comment187527>
Looks like this is missing the "-x" from your href above?
- Ben Mahler
On March 22, 2016, 5:32 p.m., B
/
Testing
---
make check
manually validated the upgrade / backwards compatibility of enum fields
Thanks,
Ben Mahler
tps://reviews.apache.org/r/45134/#comment187343>
Take a look at environment.cpp for our approach to test filtering, the
"curl" filter is a good example:
https://github.com/apache/mesos/blob/0.28.0/src/tests/environment.cpp#L229-L251
- Ben Mahler
On March 22, 2016, 12:14 a.m., To
27;t we be focusing on
[MESOS-2537](https://issues.apache.org/jira/browse/MESOS-2537)?
- Ben Mahler
On March 16, 2016, 5:44 p.m., Yong Tang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https:
org/r/44379/#comment186282>
Hm.. this comment is really hard for me to understand, if OS vendors
enhance the format, how did you know that they only use this particular format
and not others?
- Ben Mahler
On March 10, 2016, 3:26 a.m., fan du
> On March 15, 2016, 6:39 p.m., Ben Mahler wrote:
> > src/exec/exec.cpp, line 113
> > <https://reviews.apache.org/r/44654/diff/4/?file=1299793#file1299793line113>
> >
> > If you'd like to add the `private` qualifier, why isn't `kill` left as
&g
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44850/#review124054
---
Ship it!
Ship It!
- Ben Mahler
On March 15, 2016, 2:51 p.m
tps://reviews.apache.org/r/44654/#comment186749>
for backwards compatibility: ...
- Ben Mahler
On March 17, 2016, 11:30 p.m., Alexander Rukletsov wrote:
>
> ---
> This is an automatically generated e-mail. To reply,
> On March 17, 2016, 9:07 p.m., Ben Mahler wrote:
> > docs/monitoring.md, lines 876-883
> > <https://reviews.apache.org/r/43884/diff/16/?file=1299847#file1299847line876>
> >
> > Thanks! How about:
> >
> > ```
> >
>
nted here, unless this isn't overriding the
interface? Looks like this is just an implementation of the interface function,
where it is already documented.
- Ben Mahler
On March 17, 2016, 11:47 p.m., Alexander Rukletsov wrote:
>
> ---
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44653/#review124271
---
Ship it!
Ship It!
- Ben Mahler
On March 17, 2016, 11:28 p.m
> On March 17, 2016, 9:07 p.m., Ben Mahler wrote:
> > docs/monitoring.md, lines 876-883
> > <https://reviews.apache.org/r/43884/diff/16/?file=1299847#file1299847line876>
> >
> > Thanks! How about:
> >
> > ```
> >
>
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44912/#review123911
---
Ship it!
Ship It!
- Ben Mahler
On March 16, 2016, 3:47 p.m
> On March 15, 2016, 11:21 p.m., Ben Mahler wrote:
> > src/docker/executor.cpp, lines 235-239
> > <https://reviews.apache.org/r/44660/diff/4/?file=1299829#file1299829line235>
> >
> > Ditto earlier comments. I would expect the buffer to be in addition to
>
> On March 18, 2016, 8:36 p.m., Ben Mahler wrote:
> >
s/executor library/executor driver/
- Ben
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44654/#rev
org/r/43884/#comment186406>
s/metricKey/metric/ would be more consistent
Is it possible to use the JSON contains helper here to make the test a bit
easier to read? Any reason you avoided checking the values of these?
```
JSON::Object expected = {
{"allocato
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44651/#review124270
---
Ship it!
Ship It!
- Ben Mahler
On March 17, 2016, 11:27 p.m
master/event_queue_dispatches
Number of dispatches in the event queue
Gauge
```
- Ben Mahler
On March 15, 2016, 2:51 p.m., Benjamin Bannier wrote:
>
> ---
> This is an automatically ge
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44991/#review124293
---
Ship it!
Ship It!
- Ben Mahler
On March 17, 2016, 11:46 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44655/#review124289
---
Ship it!
Ship It!
- Ben Mahler
On March 17, 2016, 11:33 p.m
.
Ideally there could be a stateless validateExecutorInfo, in addition to the one
that needs the master state. In the absence of this, you can write a test that
spins up a framework and launches a task with a negative shutdown duration.
- Ben Mahler
On March 17, 2016, 11:34 p.m., Alexander
tps://reviews.apache.org/r/44851/#comment186400>
Can you clarify here what the deprecation cycle is? It would be helpful to
say that the new one was added in 0.29.0.
Could you also update the CHANGELOG with this deprecation and point users
towards using the new metric name?
- Ben
tps://reviews.apache.org/r/44657/#comment186862>
I added this TODO for killTask, can you help by clarifying that the
shutdown arrives after a kill task?
- Ben Mahler
On March 18, 2016, 5:19 p.m., Alexander Rukletsov wrote:
>
> ---
t for the argument to arrive. In
general we try to avoid SaveArg since it doesn't indicate when the value
arrives.
src/tests/slave_tests.cpp (line 3474)
<https://reviews.apache.org/r/45040/#comment186836>
Please sweep for s/.get()./->/
src/tests/slave_tests.cpp (line 34
44657/#comment186844>
Can you pull Seconds(1) down to the next line so that each component of the
sum is on a separate line?
- Ben Mahler
On March 18, 2016, 5:19 p.m., Alexander Rukletsov wrote:
>
> ---
> This is an automati
> On March 15, 2016, 10:25 p.m., Ben Mahler wrote:
> > src/launcher/executor.cpp, line 499
> > <https://reviews.apache.org/r/44657/diff/4/?file=1299823#file1299823line499>
> >
> > I'd expect that the buffer is in addition to dealing with the reap
> &
hange, can you please
provide context?
- Ben Mahler
On March 17, 2016, 11:47 p.m., Alexander Rukletsov wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.a
// shutdown more quickly by the agent, or failures / forcible
// terminations may occur.
```
However, you already have a NOTE above, so I'd rather not call it out again
here.
- Ben Mahler
On March 18, 2016, 5:14 p.m., Alexande
unit test within
master_validation_tests.cpp.
- Ben Mahler
On March 15, 2016, 3:47 p.m., Alexander Rukletsov wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache
org/r/44655/#comment186822>
Could we say `"must not assume that it will *always* be alloted the full
grace period"` in all of these?
- Ben Mahler
On March 17, 2016, 11:33 p.m., Alexander Rukletsov wrote:
>
> --
is is 2x the agent flag).
```
src/tests/slave_tests.cpp (lines 3353 - 3354)
<https://reviews.apache.org/r/44994/#comment186806>
Remember that we have the -> operator now :)
src/tests/slave_tests.cpp (line 3357)
<https://reviews.apache.org/r/44994/#comment186808>
Why do yo
> On March 18, 2016, 9:33 p.m., Ben Mahler wrote:
> > The change looks good but please add a test for this.
> >
> > Ideally there could be a stateless validateExecutorInfo, in addition to the
> > one that needs the master state. In the absence of this, you can write
> On March 15, 2016, 11:21 p.m., Ben Mahler wrote:
> > src/docker/executor.cpp, lines 657-663
> > <https://reviews.apache.org/r/44660/diff/4/?file=1299829#file1299829line657>
> >
> > Hm.. ideally we could ignore the docker stop flag if a kill policy was
&
esos.proto for a
description of Filters).
```
- Ben Mahler
On March 18, 2016, 5:13 p.m., Alexander Rukletsov wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https:
> On March 19, 2016, 2:16 a.m., Ben Mahler wrote:
> > Ok I misunderstood what the intent was here. Generally when we say
> > "deprecated" we maintain the old behavior and we plan to remove the support
> > in a future version. This doesn't maintain the old be
> On March 15, 2016, 8:35 p.m., Ben Mahler wrote:
> > Could you add a corresponding test? Having it in the header should make
> > this really easy :)
>
> Alexander Rukletsov wrote:
> Sure, but I'll write a more complex test than just unit testing the
&g
> On March 18, 2016, 9:33 p.m., Ben Mahler wrote:
> > The change looks good but please add a test for this.
> >
> > Ideally there could be a stateless validateExecutorInfo, in addition to the
> > one that needs the master state. In the absence of this, you can write
doesn't this break the existing users'
expectations? It seems easier to do a clean break after the deprecation cycle.
- Ben Mahler
On March 18, 2016, 5:23 p.m., Alexander Rukletsov wrote:
>
> ---
> This is an
tps://reviews.apache.org/r/44657/#comment186857>
can you do s/timeout/gracePeriod/ here?
I still feel that this would be clearer if separated between killTask and
shutdown, but I'm ok with this approach.
- Ben Mahler
On March 18, 2016, 5:19 p.m., Alexander Rukl
ringify(json.get()) +
"'"
" is not a valid string");
}
map[key] = value.as().value;
}
```
- Ben Mahler
On March 15, 2016, 3:52 a.m., haosdent huang wrote:
>
> ---
>
In the
future, we may allow more policy to be specified (e.g. htting an HTTP endpoint,
running a command, etc).
CHANGELOG (lines 9 - 12)
<https://reviews.apache.org/r/44662/#comment186056>
Also best-effort, worth mentioning that and why that it must not be relied
on for correctness.
.org/r/44709/
> ---
>
> (Updated March 15, 2016, 4:09 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Repository: mesos
>
>
> Description
> ---
>
> We allow unknown flags to ensure that when a new flag is added
> in Mesos version N, agent of
rsion?
This information would be great to have in the CHANGELOG deprecation
message I suggested above.
src/slave/flags.cpp (line 532)
<https://reviews.apache.org/r/44661/#comment186051>
poly?
- Ben Mahler
could ignore the docker stop flag if a kill policy was set,
because the user is being explicit about how much time they need. This is the
same as how we ignore the executor shutdown grace period flag if the executor
explicitly sets one.
- Ben Mahler
On March 15, 2016, 4:04 p.m., Alexander Rukletso
)
<https://reviews.apache.org/r/44708/#comment186028>
We tend to use size_t for things that are a count or a size. Otherwise,
we'll tend to use `unsigned int` rather than uint32_t.
- Ben Mahler
On March 15, 2016, 2:28 p.m., Alexander Rukl
tps://reviews.apache.org/r/44659/#comment186027>
Thanks, the original comment was pretty confusing, could you also reach out
to tim to ask him if there was anything else that we should call out here?
- Ben Mahler
On March 15, 2016, 2:28 p.m., Alexander Rukletsov
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44658/#review123771
---
Ship it!
Ship It!
- Ben Mahler
On March 15, 2016, 2:28 p.m
GRACE_PERIOD': " <<
parse.error();
```
We've found this style makes it clearer that no spaces were forgotten, if
you think about it, it's not the previous line that should be responsible for
adding a space, it is the line that adds more
should make this
really easy :)
- Ben Mahler
On March 15, 2016, 3:47 p.m., Alexander Rukletsov wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache
, it is the executor's responsibility to
provide the health checking and kill policy.
Does that seem clearer for the users?
include/mesos/v1/mesos.proto (lines 359 - 360)
<https://reviews.apache.org/r/44656/#comment185984>
This is missing the NOTE from the non-v1 version?
-
for
example:
```
// If the executor specifies shutdown grace period,
// pass it instead of the default.
```
- Ben Mahler
On March 15, 2016, 3:49 p.m., Alexander Rukletsov wrote:
>
>
) and above set this, so that we don't have to do a deeper investigation
later if we want to figure out which agent version sets this.
- Ben Mahler
On March 15, 2016, 2:15 p.m., Alexander Rukletsov wrote:
>
> ---
> This
--
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44652/
> -----------
>
> (Updated March 14, 2016, 5:48 p.m.)
>
>
> Review request for mesos, Ben Mahler and Joerg Schad.
>
>
651/#comment185812>
Can you rebase this without the `/* */` changes? I can't apply this patch
as is.
- Ben Mahler
On March 14, 2016, 5:45 p.m., Alexander Rukletsov wrote:
>
> ---
> This is an automatically genera
650/#comment185811>
This doesn't seem like the style we use for unused arguments..?
- Ben Mahler
On March 14, 2016, 5:45 p.m., Alexander Rukletsov wrote:
>
> ---
> This is an automatically generated e-mail. To r
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43763/#review123586
---
Ship it!
s/library/driver/
- Ben Mahler
On March 14, 2016
> On March 15, 2016, 3:13 a.m., Ben Mahler wrote:
> > Just a minor tweak that I'll apply.
We refer to this as the executor driver rather than the executor library, I'll
use the following commit message:
Minor environment variable parsing cleanup in execu
< "Failed to parse value '" << value.get() << "'"
<< " of 'MESOS_RECOVERY_TIMEOUT': " << parse.error();
```
- Ben Mahler
On March 14, 2016, 5:45 p.m., Alexander Rukletsov wrote:
>
> --
> On March 15, 2016, 3:08 a.m., Ben Mahler wrote:
> > Ship It!
I'll update the description to mention that this is consistent with the
executor driver's log format.
- Ben
---
This is an automatically generated e-ma
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44634/#review123582
---
Ship it!
Ship It!
- Ben Mahler
On March 14, 2016, 5:45 p.m
ents I had written for the
'shutdown_grace_period' in the ExecutorInfo changes?
- Ben Mahler
On March 14, 2016, 5:45 p.m., Alexander Rukletsov wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
&
reviews, for example:
`EXECUTOR_SHUTDOWN_GRACE_PERIOD` is now a flag default and is no longer intended
to be used as a hard-coded constant used in the source. The general pattern in
the code is to name a flag default as `DEFAULT_X` rather than just `X`.
- Ben Mahler
On March 14, 2016, 5:44 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44628/#review123570
---
Ship it!
Ship It!
- Ben Mahler
On March 14, 2016, 5:44 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44629/#review123571
---
Ship it!
Ship It!
- Ben Mahler
On March 14, 2016, 5:44 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44627/#review123569
---
Ship it!
Ship It!
- Ben Mahler
On March 14, 2016, 5:44 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44626/#review123568
---
Ship it!
Ship It!
- Ben Mahler
On March 14, 2016, 5:44 p.m
efore committing.
include/mesos/resources.hpp (lines 224 - 229)
<https://reviews.apache.org/r/42590/#comment185471>
Would be great to update the documentation here to reflect the changes.
- Ben Mahler
On March 12, 2016, 2:47 a.m., Guangy
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42590/#review123265
---
Ship it!
Ship It!
- Ben Mahler
On March 12, 2016, 2:47 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44721/#review123262
---
Ship it!
Ship It!
- Ben Mahler
On March 11, 2016, 6:32 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44720/#review123261
---
Ship it!
Ship It!
- Ben Mahler
On March 11, 2016, 6:32 p.m
> On March 12, 2016, 2:28 a.m., Ben Mahler wrote:
> >
I'll make sure the commit description provides the similar motivation as was
provided in https://reviews.apache.org/r/44191/ (although referring to the
latest project style guidelines rather than perform
tps://reviews.apache.org/r/44719/#comment185467>
Can this be in-line or is there a circular dependency issue that warrants
the .cpp file?
- Ben Mahler
On March 11, 2016, 6:32 p.m., Neil Conway wrote:
>
> ---
> This is a
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44718/#review123257
---
Ship it!
Ship It!
- Ben Mahler
On March 11, 2016, 6:32 p.m
191/#comment185463>
Any reason this Duration constant wasn't changed to constexpr?
- Ben Mahler
On March 11, 2016, 6:31 p.m., Neil Conway wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44190/#review123253
---
Ship it!
Ship It!
- Ben Mahler
On March 11, 2016, 6:30 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44717/#review123254
---
Ship it!
Ship It!
- Ben Mahler
On March 11, 2016, 6:32 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44690/#review123085
---
Ship it!
Ship It!
- Ben Mahler
On March 11, 2016, 3:11 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44693/#review123084
---
Ship it!
Ship It!
- Ben Mahler
On March 11, 2016, 1:40 a.m
166)
<https://reviews.apache.org/r/44692/#comment185213>
2 spaces here?
- Ben Mahler
On March 11, 2016, 1:39 a.m., Kevin Klues wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://
--
>
> (Updated March 11, 2016, 1:38 a.m.)
>
>
> Review request for mesos, Ben Mahler and Neil Conway.
>
>
> Bugs: MESOS-4787
> https://issues.apache.org/jira/browse/MESOS-4787
>
>
> Repository: mesos
>
>
> Description
> -
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44691/#review123080
---
Ship it!
Ship It!
- Ben Mahler
On March 11, 2016, 1:39 a.m
101 - 200 of 964 matches
Mail list logo
