Hi,
BenM, BenH, MPark and I had a sync today discussing reservations
(dynamic+static) in Mesos. It was a very productive meeting and here are
some notes I took from the meeting. I think all of us are on the same page
now and are happy with the current design.
- Jie
1) we still keep
and mesos ones)
- Jie Yu
On Feb. 21, 2015, 12:02 a.m., Evelina Dumitrescu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31000
+1
It does not allow me to continue to see the second page if I don't make
choices on the first one.
- Jie
On Fri, Feb 20, 2015 at 4:54 PM, Benjamin Mahler benjamin.mah...@gmail.com
wrote:
Great to see so many proposals!
Is it intentional that we have to review them in small subsets? It's
, 2015, 2:54 a.m.)
Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and
Vinod Kone.
Bugs: MESOS-2348
https://issues.apache.org/jira/browse/MESOS-2348
Repository: mesos
Description
---
See [JIRA Ticket](https://issues.apache.org/jira/browse/MESOS
On Feb. 20, 2015, 6:30 p.m., Jie Yu wrote:
include/mesos/resources.hpp, lines 148-150
https://reviews.apache.org/r/30911/diff/3/?file=868705#file868705line148
We don't use snake_case for function names. Please
```
s/persitent_volumes/persistentVolumes
```
I am
/r/30545/#comment119669
We don't use std:: prefix in cpp files. Please remove them (do a sweep in
this file).
- Jie Yu
On Feb. 20, 2015, 9:37 p.m., Chi Zhang wrote:
---
This is an automatically generated e-mail. To reply, visit
On Feb. 18, 2015, 1:28 a.m., Jie Yu wrote:
Glad to see this being worked on, Thanks for all the effort!
This is a partial review. Let's address the following comments first and
then I'll take a second pass.
One high level comment is that we should separate this giant patch. Many
://reviews.apache.org/r/30545/#comment119094
Kill those functions.
src/linux/cgroups.cpp
https://reviews.apache.org/r/30545/#comment119096
s/read/listen
src/linux/cgroups.cpp
https://reviews.apache.org/r/30545/#comment119097
s/read/listen/
- Jie Yu
On Feb. 17, 2015, 8:12 p.m
---
On Feb. 18, 2015, 12:01 a.m., Jie Yu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31024
://reviews.apache.org/r/31162/#comment119185
We don't do using namespace in headers.
- Jie Yu
On Feb. 18, 2015, 10:27 p.m., Kapil Arya wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31162
On Feb. 18, 2015, 10:42 p.m., Jie Yu wrote:
include/mesos/authentication/authentication.hpp, lines 25-27
https://reviews.apache.org/r/31162/diff/1/?file=868264#file868264line25
We don't do using namespace in headers.
Kapil Arya wrote:
Yes, but this was to avoid changing
such cases and fix them.
src/tests/state_tests.cpp
https://reviews.apache.org/r/31162/#comment119262
typedef Registry::Slaves Slaves;
typedef Registry::Slave Slave;
- Jie Yu
On Feb. 19, 2015, 12:59 a.m., Kapil Arya wrote
/#comment118987
This is still buggy right? You call fd.get() even if fd is an error?
src/linux/cgroups.cpp
https://reviews.apache.org/r/31008/#comment118988
Please update the comments.
src/linux/cgroups.cpp
https://reviews.apache.org/r/31008/#comment118986
Indent.
- Jie Yu
On Feb. 18
---
On Feb. 13, 2015, 10:43 p.m., Jie Yu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31024
/31008/#comment118934
One more blank line
- Jie Yu
On Feb. 17, 2015, 8:10 p.m., Chi Zhang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31008
On Feb. 16, 2015, 11:43 p.m., Jie Yu wrote:
include/mesos/resources.hpp, line 61
https://reviews.apache.org/r/30911/diff/2/?file=863975#file863975line61
We avoid using typedefs as much as possible trying to be more explicit.
Alexander Rukletsov wrote:
I think it's good place
/diff/
Testing
---
make check
Thanks,
Jie Yu
/r/31024/diff/
Testing
---
make check
Thanks,
Jie Yu
that the above is always enforced in our code
(especially in tests).
- Jie Yu
On Feb. 13, 2015, 10:37 p.m., Vinod Kone wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30906
in a separate
patch.
- Jie Yu
On Feb. 12, 2015, 8:05 p.m., Evelina Dumitrescu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29288
On Feb. 16, 2015, 10:48 p.m., Jie Yu wrote:
src/linux/cgroups.cpp, lines 1204-1210
https://reviews.apache.org/r/31008/diff/1/?file=863519#file863519line1204
To be consistent and symetric, putting `registerNotifier` in
`initialize` is better than putting it here. What
already?
- Jie Yu
On Feb. 16, 2015, 8:53 a.m., Isabel Jimenez wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30952
://reviews.apache.org/r/30906/#comment118684
Any reason not place this metrics at top level (i.e., in
`MesosContainerizerProcess::__destroy`)? In that way, you don't need to do
`++metrics.destroy_error` multiple times, thoughts?
- Jie Yu
On Feb. 13, 2015, 10:37 p.m., Vinod Kone wrote
it?
That being said, I want to understand the motivation for this in a JIRA.
- Jie Yu
On Feb. 13, 2015, 6:24 p.m., Chi Zhang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30958
/#comment118706
Why do you need a OwnedPromise here? Can it simply be:
```
OptionPromiseuint64 promsie;
```
src/linux/cgroups.cpp
https://reviews.apache.org/r/31008/#comment118711
You should not call a method in a process directly. Please use dispatch!
- Jie Yu
On Feb. 13
://reviews.apache.org/r/29742/#comment118712
The indent here should be 4 spaces.
- Jie Yu
On Feb. 13, 2015, 10:59 p.m., Michael Park wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org
to make filter seamlessly work for
RepeatedPtrField. Let's punt that for now and leave a TODO. For the above case,
simply do the following for now (i.e., still keep `reserved` and `unreserved`).
```
Resources(slave2.resourecs()).reserved(role2);
```
- Jie Yu
On Feb. 14, 2015, 12
30d3cd8b1e7d56fbb326c9feccd4c3cbae2015de
src/tests/scheduler_tests.cpp 080ec7236246794de6c8cc7356a06331c5e48cd5
src/tests/slave_tests.cpp a02e335576bf68b449a6286fa5cf5093b1b7182a
Diff: https://reviews.apache.org/r/31024/diff/
Testing
---
make check
Thanks,
Jie Yu
, 2015, 1:34 a.m.)
Review request for mesos, Ian Downes and Jie Yu.
Bugs: MESOS-1690
https://issues.apache.org/jira/browse/MESOS-1690
Repository: mesos
Description
---
See summary.
Diffs
-
src/slave/containerizer/mesos/containerizer.hpp
f39a876cdd6b580a7a75fd053e6923761bee7635
Diff: https://reviews.apache.org/r/30945/diff/
Testing
---
make check
Thanks,
Jie Yu
happens
between `_runTask` and `__runTask`). We could end up with the situation where
the executor receives an KillTaskMessage with an unknown task since
RunTaskMessage hasn't reached the executor yet. :(
- Jie Yu
On Feb. 12, 2015, 7:30 p.m., Jie Yu wrote
B, Benjamin Hindman, Ben Mahler, Jie Yu, and
Vinod Kone.
Repository: mesos
Description
---
# Motivation
The main motivation for introducing these functions is to capture the
definition of various identification of resources. With these functions
capturing various
On Feb. 12, 2015, 7:47 p.m., Jie Yu wrote:
Just realized that there still a race here with killTask (killTask happens
between `_runTask` and `__runTask`). We could end up with the situation
where the executor receives an KillTaskMessage with an unknown task since
RunTaskMessage
/diff/
Testing
---
make check
Thanks,
Jie Yu
Diff: https://reviews.apache.org/r/30850/diff/
Testing
---
make check
Thanks,
Jie Yu
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30812/#review71886
---
On Feb. 10, 2015, 1:35 a.m., Jie Yu wrote
check
Thanks,
Jie Yu
Thanks,
Jie Yu
/diff/
Testing
---
make check
Thanks,
Jie Yu
Thanks,
Jie Yu
---
Updated the allocator in Master::addSlave using slave-totalResources.
Diffs
-
src/master/master.cpp 7c3aa220e72fdc156fb9a0998dd68beb7c464256
Diff: https://reviews.apache.org/r/30812/diff/
Testing
---
make check
Thanks,
Jie Yu
src/tests/persistent_volume_tests.cpp
ffbaedddbd21d0eb7964be0bff4368384389b0a0
Diff: https://reviews.apache.org/r/28809/diff/
Testing
---
make check
Thanks,
Jie Yu
9adee17cb94a72f0e1e139b3fd8978a9a1ff6237
src/slave/slave.cpp fff2d725fe49eee984d9151cfb2131202c47994f
Diff: https://reviews.apache.org/r/30508/diff/
Testing
---
make check.
Thanks,
Jie Yu
/tests/persistent_volume_tests.cpp
ffbaedddbd21d0eb7964be0bff4368384389b0a0
Diff: https://reviews.apache.org/r/28809/diff/
Testing
---
make check
Thanks,
Jie Yu
/
Testing
---
make check
Thanks,
Jie Yu
(updated)
-
src/slave/containerizer/mesos/containerizer.hpp
b3aafe4efa9f3469d7a3fd39243ad66b46d6a54d
src/slave/containerizer/mesos/containerizer.cpp
fa40d47aee7803833bcde6cce1e86a21d7cf27d0
Diff: https://reviews.apache.org/r/30510/diff/
Testing
---
make check
Thanks,
Jie Yu
of this functionality be in one place!?
Jie Yu wrote:
For now, in master, we will only allow relative non-nested container path.
Revised the comments.
- Jie
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30510/#review71690
---
On Feb. 9, 2015, 6 p.m., Jie Yu wrote
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30508/#review71689
---
On Feb. 9, 2015, 6 p.m., Jie Yu wrote
task/executor.
Diffs (updated)
-
src/slave/slave.cpp 7a29f86bf873141759957a6ef5502f6a3b6ab269
src/tests/persistent_volume_tests.cpp
e3a8d33b6ae2b6afe5f4ab7c029b2f37a861f112
Diff: https://reviews.apache.org/r/30508/diff/
Testing
---
make check.
Thanks,
Jie Yu
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30509/#review71118
---
On Feb. 9, 2015, 6 p.m., Jie Yu wrote
e3a8d33b6ae2b6afe5f4ab7c029b2f37a861f112
Diff: https://reviews.apache.org/r/30510/diff/
Testing
---
make check
Thanks,
Jie Yu
://reviews.apache.org/r/30694/diff/
Testing
---
make check
Thanks,
Jie Yu
://reviews.apache.org/r/30694/diff/
Testing
---
make check
Thanks,
Jie Yu
://reviews.apache.org/r/30694/diff/
Testing
---
make check
Thanks,
Jie Yu
generated e-mail. To reply, visit:
https://reviews.apache.org/r/30694/#review71316
---
On Feb. 6, 2015, 7:47 p.m., Jie Yu wrote:
---
This is an automatically generated e-mail
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28809/#review71279
---
On Feb. 4, 2015, 7:13 p.m., Jie Yu wrote
f92808ad9b1623cea0c35ec735c53a3d6457bdbe
src/tests/persistent_volume_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/28809/diff/
Testing
---
make check
Thanks,
Jie Yu
---
Changed default disk quota check interval to be 15 seconds.
Diffs
-
src/slave/flags.hpp facf6d3e68e412897a1381d7bbdcceaf5d1fbb94
Diff: https://reviews.apache.org/r/30682/diff/
Testing
---
make check
Thanks,
Jie Yu
/30624/#comment116478
2 lines apart here please:)
src/tests/paths_tests.cpp
https://reviews.apache.org/r/30624/#comment116480
s/Pid/LibprocessPid/
Because we also have forked pid.
- Jie Yu
On Feb. 4, 2015, 5:43 p.m., Dominic Hamon wrote
On Feb. 4, 2015, 1:32 a.m., Jie Yu wrote:
src/slave/paths.cpp, line 453
https://reviews.apache.org/r/30531/diff/1/?file=844643#file844643line453
const string directory
Dominic Hamon wrote:
the function returns a string by value. there's no improvement here using
a const
/#comment116470
Ditto.
- Jie Yu
On Feb. 4, 2015, 5:22 p.m., Dominic Hamon wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30531
On Feb. 4, 2015, 1:32 a.m., Jie Yu wrote:
src/slave/paths.cpp, line 453
https://reviews.apache.org/r/30531/diff/1/?file=844643#file844643line453
const string directory
Dominic Hamon wrote:
the function returns a string by value. there's no improvement here using
a const
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30628/#review70999
---
Ship it!
- Jie Yu
On Feb. 4, 2015, 6:07 p.m., Ian Downes wrote
On Jan. 23, 2015, 6:44 p.m., Jie Yu wrote:
src/slave/state.hpp, line 120
https://reviews.apache.org/r/29918/diff/9/?file=829596#file829596line120
One more thought:
Can we introduce a third parameter for this function to let users opt
out the temp + rename (we can use
:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30514/
---
(Updated Feb. 4, 2015, 1:51 a.m.)
Review request for mesos, Ben Mahler, David Robinson, and Jie Yu.
Bugs: MESOS-1148
https
check
Thanks,
Jie Yu
();
}
```
Jie Yu wrote:
Having both 'future' and 'shuttingDown' in 'shutdown' method looks
confusing to me:) You need to comment about why you need that future. Or, I
think having dup code is not a too bad thing in this case as the logic is
more clear IMO.
Another option to avoid dup code
/30635/diff/
Testing
---
make check
Thanks,
Jie Yu
();
}
```
Jie Yu wrote:
Having both 'future' and 'shuttingDown' in 'shutdown' method looks
confusing to me:) You need to comment about why you need that future. Or, I
think having dup code is not a too bad thing in this case as the logic is
more clear IMO.
Jie Yu wrote:
Another option
On Feb. 4, 2015, 8:31 p.m., Ben Mahler wrote:
Is there a test of this function?
Might be nice to add a NOTE or TODO about how to address the issue of
leaving dangling files when we failover at the wrong time?
Jie Yu wrote:
Yes, SlaveStateTest should catch the regression
.
- Jie
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30386/#review71040
---
On Feb. 4, 2015, 1:09 a.m., Jie Yu wrote
---
On Feb. 4, 2015, 7:38 p.m., Jie Yu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30635
();
}
```
Jie Yu wrote:
Having both 'future' and 'shuttingDown' in 'shutdown' method looks
confusing to me:) You need to comment about why you need that future. Or, I
think having dup code is not a too bad thing in this case as the logic is
more clear IMO.
Jie Yu wrote:
Another option
(updated)
-
src/slave/containerizer/mesos/containerizer.hpp
569119fc591ecb118ecff51b8b19d0a1b8090cb5
src/slave/containerizer/mesos/containerizer.cpp
d712278428889ebdfd598537690138329d8464f0
Diff: https://reviews.apache.org/r/30510/diff/
Testing
---
make check
Thanks,
Jie Yu
failed future for read() if the flag is set).
- Jie Yu
On Feb. 4, 2015, 7:50 p.m., Chi Zhang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30545
81dc7eedb23ccfd61666280f6cea511cef7eb4ff
src/master/validation.cpp 550782ee7f13271f050a6374a33d36133da10f03
src/tests/master_validation_tests.cpp
12773110a61c4478fa968a07a969bd3127d73e5e
Diff: https://reviews.apache.org/r/30642/diff/
Testing
---
make check
Thanks,
Jie Yu
/
---
(Updated Feb. 4, 2015, 10:43 p.m.)
Review request for mesos, Ben Mahler, David Robinson, and Jie Yu.
Bugs: MESOS-1148
https://issues.apache.org/jira/browse/MESOS-1148
Repository: mesos
Description
---
The algorithm is simple. All the slave
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30459/#review70429
---
On Jan. 31, 2015, 2:09 a.m., Jie Yu wrote
/#comment116201
indent. align the ``
src/tests/slave_tests.cpp
https://reviews.apache.org/r/30514/#comment116202
Kill process:: prefix by using namespace process?
- Jie Yu
On Feb. 3, 2015, 2:39 a.m., Vinod Kone wrote
.
- Jie Yu
On Feb. 3, 2015, 2:39 a.m., Vinod Kone wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30514/
---
(Updated Feb
---
On Feb. 3, 2015, 11:15 p.m., Jie Yu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30513
PRE-CREATION
src/tests/resource_offers_tests.cpp 24a7eabd40b11f4bbbf30b8251561e73683eef9a
Diff: https://reviews.apache.org/r/30513/diff/
Testing
---
make check
Thanks,
Jie Yu
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30513/#review70815
---
On Feb. 3, 2015, 11:15 p.m., Jie Yu wrote
/resource_offers_tests.cpp 24a7eabd40b11f4bbbf30b8251561e73683eef9a
Diff: https://reviews.apache.org/r/30513/diff/
Testing
---
make check
Thanks,
Jie Yu
---
On Feb. 3, 2015, 11:15 p.m., Jie Yu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30513/
---
(Updated
(slave) below)?
- Jie Yu
On Feb. 3, 2015, 10:36 p.m., Vinod Kone wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30584
On Feb. 3, 2015, 11:37 p.m., Jie Yu wrote:
src/master/master.cpp, line 3908
https://reviews.apache.org/r/30584/diff/1/?file=846680#file846680line3908
This doesn't seem to necessary since we have master/slave_removals
already (which will be incremented in removeSlave(slave) below
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30386/#review70144
---
On Jan. 29, 2015, 12:12 a.m., Jie Yu wrote
d04b2c4041d8fe8978b877f07579a6f907903e1b
Diff: https://reviews.apache.org/r/30386/diff/
Testing
---
make check
Thanks,
Jie Yu
src/tests/resource_offers_tests.cpp 24a7eabd40b11f4bbbf30b8251561e73683eef9a
Diff: https://reviews.apache.org/r/30513/diff/
Testing
---
make check
Thanks,
Jie Yu
string
- Jie Yu
On Feb. 2, 2015, 11:40 p.m., Dominic Hamon wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30531
, 2015, 6:37 p.m., Jie Yu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30509/
---
(Updated Feb. 2, 2015, 6:37 p.m.)
Review
shutdown is called.
```
src/tests/slave_tests.cpp
https://reviews.apache.org/r/30514/#comment115852
indent?
src/tests/slave_tests.cpp
https://reviews.apache.org/r/30514/#comment115851
indent?
- Jie Yu
On Feb. 2, 2015, 6:51 p.m., Vinod Kone wrote
---
On Feb. 2, 2015, 7:48 p.m., Jie Yu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30512
discrepency between ext3 and ext4 (for meta data) shouldn't affect the
expections.
Diffs
-
src/tests/disk_quota_tests.cpp 83a98447644a5fa46bffdc7f2ed73bb8411841f5
Diff: https://reviews.apache.org/r/30512/diff/
Testing
---
make check on both ext3 and ext4 systems.
Thanks,
Jie Yu
Diff: https://reviews.apache.org/r/30509/diff/
Testing
---
make check
Thanks,
Jie Yu
that in the containerizer).
- Jie Yu
On Dec. 2, 2014, 10:01 p.m., Jie Yu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28608
generated e-mail. To reply, visit:
https://reviews.apache.org/r/30510/#review70617
---
On Feb. 2, 2015, 6:36 p.m., Jie Yu wrote:
---
This is an automatically generated e-mail. To reply
337e00aa46ea127f3667e3383d631c3fb8e22f30
src/master/master.cpp 10056861b95ed9453c971787982db7d09f09f323
src/messages/messages.proto c609f500e5e999a7587feb8acfb62aa637824c0a
Diff: https://reviews.apache.org/r/30536/diff/
Testing
---
make check
Thanks,
Jie Yu
501 - 600 of 2138 matches
Mail list logo
