---
On April 9, 2015, 12:30 a.m., Ben Mahler wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32996
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32998/#review79457
---
On April 9, 2015, 12:30 a.m., Ben Mahler wrote
On April 22, 2015, 9:24 a.m., Alexander Rukletsov wrote:
Let's not repeat summary in description. How about the following:
StatusUpdateStream is not a template, hence we can reduce compilation time
by moving method definitions into a `.cpp` file. As a drive-by change
headers are
/32509/#comment131519
Hm.. schedulers can't use Update as a replacement for this Message, since
Message is scheduler - executor but Update is executor - scheduler..
- Ben Mahler
On April 22, 2015, 9:44 p.m., Vinod Kone wrote
/ here? Could you do a grep just in case anything
else was missed?
src/tests/scheduler_tests.cpp
https://reviews.apache.org/r/32845/#comment131492
Hm.. can you add a comment above this as to why it's needed?
- Ben Mahler
On April 22, 2015, 9:43 p.m., Vinod Kone wrote
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33491/#review81534
---
Ship it!
Ship It!
- Ben Mahler
On April 23, 2015, 8:16 p.m
On April 28, 2015, 2:08 a.m., Ben Mahler wrote:
docs/mesos-c++-style-guide.md, line 162
https://reviews.apache.org/r/33558/diff/3/?file=942044#file942044line162
When would we use capture by reference? Seems prone to mistakes?
Michael Park wrote:
When would we use capture
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32838/#review82200
---
Ship it!
Ship It!
- Ben Mahler
On April 30, 2015, 4:22 p.m
/#comment132903
It seems a bit odd to put the top-level state in the middle, before one can
see the tree structure through the order they're listed in the header. Why not
just forward declare above?
- Ben Mahler
On April 30, 2015, 4:22 p.m., Joris Van Remoortere wrote
a `followSymlinks` boolean? Just thinking of how
to provide a complete and consistent interface that is simple to reason about.
- Ben Mahler
On March 11, 2015, 5:06 p.m., Bernd Mathiske wrote:
---
This is an automatically generated e
/thread/7mu6lzkv3b6hngww
- Ben Mahler
On April 22, 2015, 8:36 p.m., Marco Massenzio wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33448
On April 29, 2015, 8:24 p.m., Joris Van Remoortere wrote:
I was surprised to see new functionality in this patch since the summary was
code movement :)
Just a quick note on DRY below.
On April 29, 2015, 8:24 p.m., Joris Van Remoortere wrote:
src/master/framework.cpp, line 187
some food for thought. :)
- Ben Mahler
On May 6, 2015, 10:35 p.m., Jie Yu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33918
.
```
--oversubscription=disable -- this for disabling
--oversubscription=conservative
--oversubscription=bayesian
--oversubscription=custom
```
Did you have something in mind for exposing this?
- Ben Mahler
On May 6, 2015, 10:35 p.m., Jie Yu wrote
of the
motivation for the new endpoint and its format. A bit tough to tell what a
sample response looks like without reading through this diff carefully. :)
- Ben Mahler
On May 5, 2015, 9:51 p.m., Joris Van Remoortere wrote
.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-328
https://issues.apache.org/jira/browse/MESOS-328
Repository: mesos
Description
---
Add InsensitiveHashMap.
Diffs
-
3rdparty/libprocess/3rdparty/stout/include/Makefile.am
On April 28, 2015, 2:08 a.m., Ben Mahler wrote:
docs/mesos-c++-style-guide.md, lines 206-208
https://reviews.apache.org/r/33558/diff/3/?file=942044#file942044line206
What does it mean to pass a lambda to `socket.send`? Doesn't look like
this is functionality provided
, 1:46 a.m., Ben Mahler wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33153/
---
(Updated April 14, 2015, 1:46 a.m
/#comment134222
unique_ptr is not a POD, so this will still try to run the destructor of
the function during exit of the program.
Can you just use a raw pointer here and leak it? (Not ideal but we use this
in many places)
- Ben Mahler
On May 9, 2015, 5:07 p.m., haosdent huang wrote
of this file
- Ben Mahler
On May 9, 2015, 3:36 p.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33792
e-mail. To reply, visit:
https://reviews.apache.org/r/33154/#review80444
---
On April 14, 2015, 1:46 a.m., Ben Mahler wrote:
---
This is an automatically generated e-mail. To reply
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33155/#review80397
---
On April 14, 2015, 1:46 a.m., Ben Mahler wrote
?
src/tests/sorter_tests.cpp
https://reviews.apache.org/r/31667/#comment134701
Ditto here, let's flip the arguments.
- Ben Mahler
On May 10, 2015, 12:21 a.m., Michael Park wrote:
---
This is an automatically generated e-mail
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33154/#review83463
---
On May 12, 2015, 2:51 a.m., Ben Mahler wrote:
---
This is an automatically
Diff: https://reviews.apache.org/r/33154/diff/
Testing
---
make check
Thanks,
Ben Mahler
/tests/master_tests.cpp 75ffadae64ece4e3ff53abeefa5f6e8e3690d402
src/tests/partition_tests.cpp 1018e479a77a6b533f2dd392fedbdccb80e3ac1f
src/tests/slave_tests.cpp 04e79ec061e767a82d40242be7884967942c50b6
Diff: https://reviews.apache.org/r/33155/diff/
Testing
---
make check
Thanks,
Ben
check
Thanks,
Ben Mahler
://reviews.apache.org/r/33153/diff/
Testing
---
make check
Thanks,
Ben Mahler
considered,
because there is no way to be able to write just `http::Response` otherwise, is
there?
Seems quite verbose to write process::http everywhere, and on the otherhand
just having `Request` or `Response` seems to miss the context of it being http,
thoughts?
- Ben Mahler
On May 18, 2015, 11
-
src/master/master.hpp da0a83510784f4f7dbd933e666ac12c04c413a62
Diff: https://reviews.apache.org/r/34387/diff/
Testing
---
make check
Thanks,
Ben Mahler
da0a83510784f4f7dbd933e666ac12c04c413a62
src/master/master.cpp eaea79df2c693d15087d70b3c9b988e57c894f8e
src/master/validation.cpp 20a6ac833ec5dc437f80159a9234c7b94d86ba29
Diff: https://reviews.apache.org/r/34389/diff/
Testing
---
make check
Thanks,
Ben Mahler
/
Testing
---
See markdown in: https://reviews.apache.org/r/34295/
Thanks,
Ben Mahler
in the latter).
- Ben
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34295/#review84007
---
On May 15, 2015, 10:25 p.m., Ben Mahler
(including the
commit message in a comment), if applicable?
- Ben Mahler
On May 19, 2015, 5:40 p.m., Jie Yu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34420
for you.
3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
https://reviews.apache.org/r/34438/#comment135629
Let's move the * here to be in the right place.
- Ben Mahler
On May 19, 2015, 9:44 p.m., Chi Zhang wrote
, so flip the order of the
arguments here. Also, these should fit on one line, ditto below.
3rdparty/libprocess/src/tests/http_tests.cpp
https://reviews.apache.org/r/33793/#comment135597
Ditto here. Don't bother with stringify.
- Ben Mahler
On May 9, 2015, 3:37 p.m., haosdent huang
, it's ok to continue checking things.
Can you re-organize this a bit, it's hard to follow that you're testing the
things we care about. Also, how about just sticking with 'put' and 'get',
rather than also using '[]' and 'contains' here?
- Ben Mahler
On May 12, 2015, 12:56 a.m
/diff/
Testing (updated)
---
The existing no executor framework test picks this up.
Since we do not have an non-zero estimator yet, I modified the slave to send
revocable resources in order to manually test the revocable case.
Thanks,
Ben Mahler
://reviews.apache.org/r/33271/#comment138833
Do we really want to encourage taking a const of a POD type? In general we
have not been doing this, so it seems pretty inconsistent to put it in this
example.
Also, looks like we need a space before the openening parenthesis.
- Ben Mahler
On June 2
, rules.disabled_endpoints().paths()) {
paths.insert(path);
}
```
src/slave/main.cpp
https://reviews.apache.org/r/33296/#comment139396
Don't you need a utility include for move?
- Ben Mahler
On June 8, 2015, 12:11 p.m., Alexander Rojas wrote
/process_tests.cpp
https://reviews.apache.org/r/33295/#comment139429
Is it possible to just pass `{}` here for empty vector?
- Ben Mahler
On June 8, 2015, 10:09 a.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail
On June 5, 2015, 9:58 p.m., Vinod Kone wrote:
src/master/master.cpp, line 3462
https://reviews.apache.org/r/35118/diff/1/?file=980131#file980131line3462
woah. didn't realize this was handled automagically by the install
handler.
Yeah, we didn't do this for framework provided
/#comment139436
By the way, I removed the std:: prefix here.
docs/mesos-c++-style-guide.md
https://reviews.apache.org/r/35120/#comment139437
Also I added const to the 'values', was that not the intent?
- Ben Mahler
On June 5, 2015, 9:34 a.m., Joris Van Remoortere wrote
Do you still need the fraction on 1024.0 with this? Maybe a note is
needed:
```
// Ensure bytes is treated as floating point for the math below.
bytes = float(bytes)
...
```
- Ben Mahler
On June 2, 2015, 3:28 a.m., weitao zhou wrote
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35120/#review87111
---
Ship it!
Ship It!
- Ben Mahler
On June 5, 2015, 9:34 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32999/#review86224
---
On April 23, 2015, 11:15 p.m., Ben Mahler wrote
estimator yet, I modified the slave to send
revocable resources in order to manually test the revocable case.
Thanks,
Ben Mahler
.
- Ben
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34970/#review86767
---
On June 5, 2015, 5:52 a.m., Ben Mahler wrote
I can't tell why slave id is being passed here, is there something subtle
going on?
- Ben Mahler
On May 21, 2015, 4:05 p.m., Bernd Mathiske wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
---
On June 5, 2015, 7:56 p.m., Ben Mahler wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34970/
---
(Updated
to be able to
declare when an argument is required to avoid these manual checks entirely. :)
- Ben Mahler
On May 29, 2015, 1:10 a.m., Marco Massenzio wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
On June 2, 2015, 5:52 p.m., Ben Mahler wrote:
I'm seeing the following pattern:
```
if (flags.master.isNone()) {
cerr flags.usage(Missing required option --master) endl;
return EXIT_FAILURE;
}
```
Why not leverage EXIT here?
```
if (flags.master.isNone
with the master?
Would help debugging, as I was just tripped up by forgetting to set it.
Also, might be easy to fold in to printing the 'checkpoint' capability (maybe
also at some point folding it in to the Capabilities enum).
- Ben Mahler
On May 21, 2015, 12:42 a.m., Vinod Kone wrote
/#comment138292
Is the lack of a delay() in this case a bug? Doesn't this break the
`forwardOversubscribedResources` loop? Is there something else going on that
ensures the loop continues..?
- Ben Mahler
On May 29, 2015, 12:30 a.m., Vinod Kone wrote
inconsistent?
- Ben Mahler
On June 2, 2015, 10:04 p.m., Jie Yu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34956
0x0047f06a in main ()
```
Thanks,
Ben Mahler
src/examples/persistent_volume_framework.cpp
6a0c0cb61a06d1a0e7608fe2447167c18111ea43
Diff: https://reviews.apache.org/r/34969/diff/
Testing
---
make check
Thanks,
Ben Mahler
difficult to make further changes without breaking
people. For example, if we add 'total_resources' as done here, but then we want
to show the role, how do we do that without breaking people's code that exactly
matches cpus ?
- Ben Mahler
On June 9, 2015, 12:38 a.m., Jiang Yan Xu wrote
On June 9, 2015, 5:36 p.m., Niklas Nielsen wrote:
src/master/master.cpp, lines 3513-3514
https://reviews.apache.org/r/35118/diff/1/?file=980131#file980131line3513
Does it make sense to point to some documentation (if it exists
already, or inline) about how this resource math will
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34921/#review87336
---
Ship it!
Ship It!
- Ben Mahler
On June 9, 2015, 2:58 a.m
are inconcistent on the Test suffix, e.g. MasterTest vs
CRAMMD5Authentication. See two options:
(1) We can use Test to signify that there is a fixture, if that's helpful. So
remove any Test suffixes from non-fixture tests.
(2) We can use Test as a suffix for all tests.
Thoughts?
- Ben Mahler
On June 20
is that some tests are
Process and some tests are named JsonTest. The latter tends to be forced
when we have a test fixture (e.g. MasterTest).
- Ben Mahler
On June 20, 2015, 7:14 p.m., Michael Park wrote:
---
This is an automatically
not contain
explicit pre-conditions, it seems a little non-idiomatic to return 412..?
- Ben Mahler
On June 22, 2015, 5:08 a.m., Michael Park wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35663/#review88588
---
Ship it!
Ship It!
- Ben Mahler
On June 19, 2015, 7:57 p.m., Jie
/sorter_tests.cpp (lines 396 - 398)
https://reviews.apache.org/r/35664/#comment141183
Hm.. wonder if we can do the following now in these tests?
```
EXPECT_EQ({a, b}, sorter.sort());
```
- Ben Mahler
On June 19, 2015, 10:21 p.m., Jie Yu wrote
();
}
if (total 0.0) {
...
}
}
```
Ditto for computing `allocation` below. Seems a bit non-intuitive that the
call to `get` is aggregating across `Resource` objects. One would guess that it
is just pulling out the scalar by that name.
- Ben Mahler
On June 19, 2015, 9:09
On June 25, 2015, 12:38 a.m., Vinod Kone wrote:
src/tests/master_contender_detector_tests.cpp, line 877
https://reviews.apache.org/r/35815/diff/3/?file=991386#file991386line877
s/EXPECT/ASSERT/
as this is the last expectation in the test.
Marco Massenzio wrote:
://reviews.apache.org/r/35849/#comment141852
Mind leaving a note for posterity as to why it's disabled? E.g.
```
// NOTE: This is disabled due to MESOS-1187.
```
- Ben Mahler
On June 24, 2015, 10:10 p.m., Jie Yu wrote
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35849/#review89261
---
Ship it!
Ship It!
- Ben Mahler
On June 24, 2015, 10:10 p.m
();
});
```
Thoughts?
Thoughts?
- Ben Mahler
On June 18, 2015, 3:34 p.m., Benjamin Hindman wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34943
/master/allocator/mesos/allocator.hpp
include/mesos/master/allocator.hpp
- Ben Mahler
On June 24, 2015, 6:16 p.m., Jie Yu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35837
we might want to express
over-allocation, see:
[MESOS-2930](https://issues.apache.org/jira/browse/MESOS-2930).
- Ben Mahler
On June 24, 2015, 3:35 p.m., Michael Park wrote:
---
This is an automatically generated e-mail. To reply, visit
this interact with the
zookeeper session timeout?
- Ben Mahler
On June 22, 2015, 10:03 a.m., Adam B wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29507
if there is more of both revocable and non-revocable resources.
- Ben Mahler
On June 24, 2015, 6:17 p.m., Jie Yu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35838
.
Perhaps for this particular case, since it resembles a literal constant, we
could keep 'const' and call it VALUES. Some food for thought.
- Ben Mahler
On June 25, 2015, 12:29 a.m., Jojy Varghese wrote:
---
This is an automatically generated
what we've
found.
src/slave/slave.cpp (lines 3554 - 3560)
https://reviews.apache.org/r/35635/#comment141030
This is a detraction from the existing style IMO
- Ben Mahler
On June 19, 2015, 12:30 a.m., Michael Park wrote
)
https://reviews.apache.org/r/35631/#comment141036
Maybe a 1000 and 50,000 as well?
src/tests/hierarchical_allocator_tests.cpp (lines 991 - 993)
https://reviews.apache.org/r/35631/#comment141034
Doesn't this need to be an atomic?
- Ben Mahler
On June 19, 2015, 1:11 a.m., Jie Yu wrote
On June 17, 2015, 8:23 p.m., Till Toenshoff wrote:
3rdparty/libprocess/src/subprocess.cpp, line 332
https://reviews.apache.org/r/35561/diff/1/?file=986458#file986458line332
Aren't we leaking this one in the parent process?
Benjamin Hindman wrote:
We'll be exec'ing, so
On June 18, 2015, 1:08 a.m., Ben Mahler wrote:
src/master/detector.cpp, lines 444-447
https://reviews.apache.org/r/35571/diff/4/?file=986626#file986626line444
Our logging messages do not end with periods, please do a sweep :)
Ditto for aligning on the operator
IMO, can you remove it?
- Ben Mahler
On June 18, 2015, 3:31 p.m., Marco Massenzio wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35571
://reviews.apache.org/r/35561/#comment140931
As till pointed out, you're leaking this memory in the parent process.
- Ben Mahler
On June 17, 2015, 2:28 p.m., Benjamin Hindman wrote:
---
This is an automatically generated e-mail
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35699/#review88836
---
Ship it!
Thanks for taking care of this!
- Ben Mahler
On June
question about the Test suffix, curious to hear your
thoughts on it.
- Ben Mahler
On June 20, 2015, 7:14 p.m., Michael Park wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35695
' to be clear that this is a role
breakdown, rather than say, a type, name, etc breakdown?
If the type was `hashmapRole, Resources` we could just call the argument
`resources`, but in-lieu of this, `roleResources` helps the reader more than
`resourcesMap`. :)
- Ben Mahler
On June 22, 2015
-
src/tests/reconciliation_tests.cpp 6042d8c02d86f486e0c4d82d5a70666d7ac9019b
Diff: https://reviews.apache.org/r/35935/diff/
Testing
---
make check + ran the benchmark (per MESOS-2940)
Thanks,
Ben Mahler
---
See above.
Diffs
-
src/common/protobuf_utils.cpp bd6996159e73bf63bb7c2fa3a28def6a2be92b1b
Diff: https://reviews.apache.org/r/35943/diff/
Testing
---
N/A
Thanks,
Ben Mahler
,
Ben Mahler
930dda91068dc6ccbab848b5723ce1568f042779
Diff: https://reviews.apache.org/r/35910/diff/
Testing
---
make check
Thanks,
Ben Mahler
/slave/status_update_manager.cpp 0ad24500c3007202263794fd094fa60535d8f58c
Diff: https://reviews.apache.org/r/35911/diff/
Testing
---
make check
Thanks,
Ben Mahler
://issues.apache.org/jira/browse/MESOS-1988
Repository: mesos
Description
---
Per the thread on the mailing list recently.
Diffs
-
src/sched/sched.cpp bc76c71ae9d44b291049223366e38cb0fd0c
Diff: https://reviews.apache.org/r/35914/diff/
Testing
---
N/A
Thanks,
Ben Mahler
---
See above.
Diffs
-
src/slave/slave.cpp b3e1ccc28d2698d8bc91829d70787dc2fc17b80d
Diff: https://reviews.apache.org/r/35909/diff/
Testing
---
make check
Thanks,
Ben Mahler
/35632/diff/
Testing
---
Added to the existing master / registrar metrics test.
Thanks,
Ben Mahler
/35752/#comment141695
Drop the message when event.has_error() is not true.
- Ben Mahler
On June 23, 2015, 7:26 p.m., Ben Mahler wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r
On June 25, 2015, 1:41 a.m., Ben Mahler wrote:
Actually, we should think about one more thing, how does this interact with
the zookeeper session timeout?
Adam B wrote:
The hardcoded individual ping timeout (15secs) was previously longer than
the default zk session timeout (10secs
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35910/#review89565
---
On June 27, 2015, 12:36 a.m., Ben Mahler wrote
1efc6fb351e49deaa8f626823592bc9155f5137b
src/slave/slave.cpp b3e1ccc28d2698d8bc91829d70787dc2fc17b80d
src/slave/status_update_manager.cpp 0ad24500c3007202263794fd094fa60535d8f58c
Diff: https://reviews.apache.org/r/35911/diff/
Testing
---
make check
Thanks,
Ben Mahler
/fault_tolerance_tests.cpp fb28e2afd19d9e9141596767b1d48363e6cce5dc
src/tests/partition_tests.cpp f7ee3abe321e9a44913ce5418c9ad9fda0cb3974
Diff: https://reviews.apache.org/r/35912/diff/
Testing
---
make check
Ran the benchmark per MESOS-2940.
Thanks,
Ben Mahler
Diff: https://reviews.apache.org/r/35935/diff/
Testing
---
make check + ran the benchmark (per MESOS-2940)
Thanks,
Ben Mahler
for this in a subsequent patch.
Diffs (updated)
-
src/exec/exec.cpp 930dda91068dc6ccbab848b5723ce1568f042779
Diff: https://reviews.apache.org/r/35910/diff/
Testing
---
make check
Thanks,
Ben Mahler
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35910/#review89573
---
On June 26, 2015, 9:11 p.m., Ben Mahler wrote
1 - 100 of 815 matches
Mail list logo
