On Oct. 3, 2014, 5:25 p.m., Niklas Nielsen wrote:
src/exec/exec.cpp, lines 75-76
https://reviews.apache.org/r/25434/diff/4/?file=709150#file709150line75
Why Java executors? I am not sure I understand this comment.
This is what BenH pointed me at. Since Java has garbage collection,
On Oct. 2, 2014, 7:49 a.m., Timothy Chen wrote:
src/exec/exec.cpp, line 739
https://reviews.apache.org/r/25434/diff/4/?file=709150#file709150line739
I'm not sure this should be at the WARNING level, as it's not really
expected to have it set all the time. IMHO it should be INFO
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/
---
(Updated Oct. 7, 2014, 3:54 p.m.)
Review request for mesos, Benjamin Hindman,
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/#review55662
---
Patch looks great!
Reviews applied: [25434]
All tests passed.
-
On Oct. 2, 2014, 7:49 a.m., Timothy Chen wrote:
src/tests/slave_tests.cpp, line 1023
https://reviews.apache.org/r/25434/diff/4/?file=709164#file709164line1023
Times(1) is default, can ignore the call.
Alexander Rukletsov wrote:
To the best of my knowledge, we always
On Oct. 2, 2014, 12:49 a.m., Timothy Chen wrote:
src/slave/utils.cpp, line 39
https://reviews.apache.org/r/25434/diff/4/?file=709161#file709161line39
Log the timeout value
Both the one that is too small and the effective one.
- Niklas
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/#review55348
---
Hi Alex,
Sorry for the tardy reply on this review. I have left a
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/
---
(Updated Oct. 2, 2014, 7:17 a.m.)
Review request for mesos, Benjamin Hindman,
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/#review55196
---
src/exec/exec.cpp
https://reviews.apache.org/r/25434/#comment95590
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/
---
(Updated Sept. 30, 2014, 2:16 p.m.)
Review request for mesos, Benjamin
On Sept. 11, 2014, 3:45 p.m., Timothy St. Clair wrote:
src/exec/exec.cpp, line 82
https://reviews.apache.org/r/25434/diff/2/?file=683945#file683945line82
Maybe I'm missing something, but is there a reason we don't check
before a delay? If ShutdownProcess is spawned, it's
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/
---
(Updated Sept. 18, 2014, 11:03 a.m.)
Review request for mesos, Benjamin
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/#review53816
---
Patch looks great!
Reviews applied: [25434]
All tests passed.
-
On Sept. 9, 2014, 4:39 p.m., Niklas Nielsen wrote:
src/exec/exec.cpp, lines 723-729
https://reviews.apache.org/r/25434/diff/2/?file=683945#file683945line723
Small nit: We try to keep the variable names short and concise. I would
have dropped the 'mesos' prefix.
Alexander
On Sept. 9, 2014, 4:21 p.m., Timothy Chen wrote:
src/exec/exec.cpp, line 738
https://reviews.apache.org/r/25434/diff/2/?file=683945#file683945line738
Why divide by 2 here? It seems like no matter what you're shrinking the
timeout?
Alexander Rukletsov wrote:
I'm not
On Sept. 9, 2014, 5:50 p.m., Benjamin Hindman wrote:
src/launcher/executor.cpp, lines 654-656
https://reviews.apache.org/r/25434/diff/2/?file=683946#file683946line654
I'd rather not introduce namespace aliases, can we not put all of this
in mesos::internal?
Alexander Rukletsov
On Sept. 11, 2014, 3:45 p.m., Timothy St. Clair wrote:
src/tests/containerizer.cpp, line 113
https://reviews.apache.org/r/25434/diff/2/?file=683955#file683955line113
Manifest constant please.
Alexander Rukletsov wrote:
Ok.
Use default.
- Alexander
On Sept. 11, 2014, 3:45 p.m., Timothy St. Clair wrote:
src/exec/exec.cpp, line 82
https://reviews.apache.org/r/25434/diff/2/?file=683945#file683945line82
Maybe I'm missing something, but is there a reason we don't check
before a delay? If ShutdownProcess is spawned, it's
On Sept. 11, 2014, 3:45 p.m., Timothy St. Clair wrote:
src/exec/exec.cpp, line 82
https://reviews.apache.org/r/25434/diff/2/?file=683945#file683945line82
Maybe I'm missing something, but is there a reason we don't check
before a delay? If ShutdownProcess is spawned, it's
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/#review53038
---
src/exec/exec.cpp
https://reviews.apache.org/r/25434/#comment92392
On Sept. 9, 2014, 5:50 p.m., Benjamin Hindman wrote:
src/slave/constants.hpp, line 53
https://reviews.apache.org/r/25434/diff/2/?file=683947#file683947line53
What is the 'base executor' versus the 'command executor'?
Alexander Rukletsov wrote:
We have Executor (lives in
On Sept. 9, 2014, 5:50 p.m., Benjamin Hindman wrote:
src/slave/constants.hpp, line 53
https://reviews.apache.org/r/25434/diff/2/?file=683947#file683947line53
What is the 'base executor' versus the 'command executor'?
Alexander Rukletsov wrote:
We have Executor (lives in
On Sept. 9, 2014, 9:33 a.m., Till Toenshoff wrote:
src/slave/constants.hpp, lines 51-57
https://reviews.apache.org/r/25434/diff/1/?file=682522#file682522line51
I really like this approach. Just wanted to make sure we do not create
timeout's (using the /2 or /3 arithmetics) that
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/#review52727
---
Patch looks great!
Reviews applied: [25434]
All tests passed.
-
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/#review52729
---
src/exec/exec.cpp
https://reviews.apache.org/r/25434/#comment91709
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/#review52726
---
I think this will work (and good job on style btw!). The biggest
On Sept. 9, 2014, 9:21 a.m., Timothy Chen wrote:
src/launcher/executor.cpp, line 664
https://reviews.apache.org/r/25434/diff/2/?file=683946#file683946line664
I'm not sure we've all agreed to start using C++11 yet?
Good catch - no auto's yet.
- Niklas
On Sept. 9, 2014, 4:21 p.m., Timothy Chen wrote:
src/launcher/executor.cpp, line 682
https://reviews.apache.org/r/25434/diff/2/?file=683946#file683946line682
I'm not sure I'm really following the logic, I know the levels but I
don't know why there is the divide by 2/3 on each
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/#review52740
---
src/exec/exec.cpp
https://reviews.apache.org/r/25434/#comment91728
On Sept. 9, 2014, 4:39 p.m., Niklas Nielsen wrote:
I think this will work (and good job on style btw!). The biggest piece that
I find missing is testing; we need tests to verify that the new escalation
logic works as we expect.
Tests in case of long (more than 3 deltas) and short
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/#review52750
---
Great seeing new folks submitting reviews! Just a drive by here,
On Sept. 9, 2014, 4:21 p.m., Timothy Chen wrote:
src/launcher/executor.cpp, line 682
https://reviews.apache.org/r/25434/diff/2/?file=683946#file683946line682
I'm not sure I'm really following the logic, I know the levels but I
don't know why there is the divide by 2/3 on each
On Sept. 9, 2014, 4:39 p.m., Niklas Nielsen wrote:
src/tests/containerizer.cpp, line 113
https://reviews.apache.org/r/25434/diff/2/?file=683955#file683955line113
Why 3 seconds?
Alexander Rukletsov wrote:
Though default is 5, I decide to set it to 3 (which is the lower
On Sept. 9, 2014, 5:50 p.m., Benjamin Hindman wrote:
src/slave/constants.hpp, line 53
https://reviews.apache.org/r/25434/diff/2/?file=683947#file683947line53
What is the 'base executor' versus the 'command executor'?
We have Executor (lives in src/exec/exec.cpp) and CommandExecutor
On Sept. 9, 2014, 5:50 p.m., Benjamin Hindman wrote:
src/slave/constants.hpp, line 53
https://reviews.apache.org/r/25434/diff/2/?file=683947#file683947line53
What is the 'base executor' versus the 'command executor'?
Alexander Rukletsov wrote:
We have Executor (lives in
On Sept. 9, 2014, 5:50 p.m., Benjamin Hindman wrote:
src/slave/constants.hpp, line 53
https://reviews.apache.org/r/25434/diff/2/?file=683947#file683947line53
What is the 'base executor' versus the 'command executor'?
Alexander Rukletsov wrote:
We have Executor (lives in
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/
---
(Updated Sept. 8, 2014, 8:04 p.m.)
Review request for mesos, Niklas Nielsen,
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25434/#review52666
---
Patch looks great!
Reviews applied: [25434]
All tests passed.
-
38 matches
Mail list logo
