Re: Review Request 33159: Pump updateFramework through Allocator from Master.

2015-04-13 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33159/#review79982
---


Patch looks great!

Reviews applied: [32961, 33159]

All tests passed.

- Mesos ReviewBot


On April 14, 2015, 2:57 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33159/
> ---
> 
> (Updated April 14, 2015, 2:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2615 and MESOS-703
> https://issues.apache.org/jira/browse/MESOS-2615
> https://issues.apache.org/jira/browse/MESOS-703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Follow up of 32961 where we add the 'updateFramework' function to the 
> allocator.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/allocator.hpp 91f80501aa9bc733fd53f9cb1ac0f15949a74964 
>   src/master/allocator/mesos/allocator.hpp 
> fb898f1175b61b442204e6e38c69ccc2838a646f 
>   src/master/allocator/mesos/hierarchical.hpp 
> 9f9a631ee52a3ab194e678f9be139e8dabc7f3db 
>   src/master/master.cpp 44b0a0147f5354824d86332a67b30018634c9a36 
>   src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4 
> 
> Diff: https://reviews.apache.org/r/33159/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Review Request 33159: Pump updateFramework through Allocator from Master.

2015-04-13 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33159/
---

Review request for mesos, Benjamin Hindman and Niklas Nielsen.


Bugs: MESOS-2615 and MESOS-703
https://issues.apache.org/jira/browse/MESOS-2615
https://issues.apache.org/jira/browse/MESOS-703


Repository: mesos


Description
---

Follow up of 32961 where we add the 'updateFramework' function to the allocator.


Diffs
-

  src/master/allocator/allocator.hpp 91f80501aa9bc733fd53f9cb1ac0f15949a74964 
  src/master/allocator/mesos/allocator.hpp 
fb898f1175b61b442204e6e38c69ccc2838a646f 
  src/master/allocator/mesos/hierarchical.hpp 
9f9a631ee52a3ab194e678f9be139e8dabc7f3db 
  src/master/master.cpp 44b0a0147f5354824d86332a67b30018634c9a36 
  src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4 

Diff: https://reviews.apache.org/r/33159/diff/


Testing
---

make check


Thanks,

Joris Van Remoortere



Re: Review Request 32961: WIP: Allow framework re-registeration to update master http fields.

2015-04-13 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32961/
---

(Updated April 14, 2015, 2:57 a.m.)


Review request for mesos, Benjamin Hindman and Niklas Nielsen.


Changes
---

Factor out allocator change.
Rebase on master.


Bugs: MESOS-1218, MESOS-2614 and MESOS-703
https://issues.apache.org/jira/browse/MESOS-1218
https://issues.apache.org/jira/browse/MESOS-2614
https://issues.apache.org/jira/browse/MESOS-703


Repository: mesos


Description
---

Fields: 'name', 'hostname', 'failover_timeout', 'webui_url'


Diffs (updated)
-

  src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
  src/master/master.cpp 44b0a0147f5354824d86332a67b30018634c9a36 

Diff: https://reviews.apache.org/r/32961/diff/


Testing
---

make check.
re-registered no_executor_framework with different 'name', 'hostname', 
'failover_timeout', and 'webui_url'


Thanks,

Joris Van Remoortere



Re: Review Request 33155: Added commented-out tests for slave removal metrics.

2015-04-13 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33155/#review79981
---


Patch looks great!

Reviews applied: [33152, 33153, 33154, 33155]

All tests passed.

- Mesos ReviewBot


On April 14, 2015, 1:46 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33155/
> ---
> 
> (Updated April 14, 2015, 1:46 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2485
> https://issues.apache.org/jira/browse/MESOS-2485
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> I updated the existing tests to validate the metrics. However, it blocks the 
> tests due to the slave metrics never getting satisfied. Added a TODO.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 32b1e9bb58d8046e5363fafe2ab8f662b6c9a666 
>   src/tests/partition_tests.cpp 1018e479a77a6b533f2dd392fedbdccb80e3ac1f 
>   src/tests/slave_tests.cpp b826000e0a4221690f956ea51f49ad4c99d5e188 
> 
> Diff: https://reviews.apache.org/r/33155/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Review Request 33154: Added reason metrics for slave removals.

2015-04-13 Thread Ben Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33154/
---

Review request for mesos and Vinod Kone.


Bugs: MESOS-2485
https://issues.apache.org/jira/browse/MESOS-2485


Repository: mesos


Description
---

See [MESOS-2485](https://issues.apache.org/jira/browse/MESOS-2485).


Diffs
-

  include/mesos/mesos.proto 3a8e8bf303e0576c212951f6028af77e54d93537 
  src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
  src/master/master.cpp 44b0a0147f5354824d86332a67b30018634c9a36 
  src/master/metrics.hpp 52a83289cfe7e6b6fd8d5bff0774ebe5ce51d0ed 
  src/master/metrics.cpp 14486bf7130250f561c9fb7a43c95f3fc1e76a4b 

Diff: https://reviews.apache.org/r/33154/diff/


Testing
---

make check


Thanks,

Ben Mahler



Review Request 33152: Moved the slave shutdown test into slave_tests.cpp.

2015-04-13 Thread Ben Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33152/
---

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/fault_tolerance_tests.cpp a637c32f004638a110390b22cf5b626e904097cf 
  src/tests/slave_tests.cpp b826000e0a4221690f956ea51f49ad4c99d5e188 

Diff: https://reviews.apache.org/r/33152/diff/


Testing
---

make check


Thanks,

Ben Mahler



Review Request 33153: Moved a partition test into partition_tests.cpp.

2015-04-13 Thread Ben Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33153/
---

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/fault_tolerance_tests.cpp a637c32f004638a110390b22cf5b626e904097cf 
  src/tests/partition_tests.cpp 1018e479a77a6b533f2dd392fedbdccb80e3ac1f 

Diff: https://reviews.apache.org/r/33153/diff/


Testing
---

make check


Thanks,

Ben Mahler



Review Request 33155: Added commented-out tests for slave removal metrics.

2015-04-13 Thread Ben Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33155/
---

Review request for mesos and Vinod Kone.


Bugs: MESOS-2485
https://issues.apache.org/jira/browse/MESOS-2485


Repository: mesos


Description
---

I updated the existing tests to validate the metrics. However, it blocks the 
tests due to the slave metrics never getting satisfied. Added a TODO.


Diffs
-

  src/tests/master_tests.cpp 32b1e9bb58d8046e5363fafe2ab8f662b6c9a666 
  src/tests/partition_tests.cpp 1018e479a77a6b533f2dd392fedbdccb80e3ac1f 
  src/tests/slave_tests.cpp b826000e0a4221690f956ea51f49ad4c99d5e188 

Diff: https://reviews.apache.org/r/33155/diff/


Testing
---

make check


Thanks,

Ben Mahler



Re: Review Request 31985: Mesos container ID available to the executor through an environment variable.

2015-04-13 Thread Timothy Chen


> On March 14, 2015, 12:49 a.m., Timothy Chen wrote:
> > The change looks good, but I'm not sure how exposing the container id is 
> > the right thing to do overall yet. Container id as I know of is meant to be 
> > a internal id that is used only in mesos, and I believe the whole 
> > motivation was for users to be able to guess the docker container name from 
> > the container id. However, container id -> docker container name mapping 
> > might change since we manage that, and I'm progress changing it to include 
> > the slave id.
> > 
> > I personally think we should think about exposing container specific 
> > information.
> > 
> > Do you know if there is any other use case to know the container id?
> 
> Alexander Rojas wrote:
> 1. I am not aware of any other use case. However I asked benh about this 
> and he mentioned it was ok to have this as extra info on the update, as long 
> as it was no docker specific data.
> 2. After asking some questions to the original reporter, the idea was 
> more to be able to group tasks assigned to the same container, but not as a 
> way to extract the specific container.
> 3. I am not sure, then, what would be considered mesos private info and 
> info which can be shared. For example, why can the framework id and the 
> executor id be shared but no the container id?
> 
> Vinod Kone wrote:
> "group tasks assigned to the same container " .. What does this mean? 
> IIUC, our docker containerizer only supports single task containers.
> 
> regarding why framework id and executor id are exposed: framework id is 
> needed by frameworks to reregister with master. executor id (and task id) is 
> generated by the framework and not mesos.
> 
> Alexander Rojas wrote:
> So I guess then, I will discard this patch and set the issue to won't fix?
> 
> Vinod Kone wrote:
> I think we should try to understand the root of the issue that the 
> reporter is having before jumping onto a specific implementation.
> 
> Alexander Rojas wrote:
> Well, I felt like I had it clear but apparently not. Can you please ask 
> the questions in the Jira entry vinod?
> 
> I was also wondering, As mentioned above, we want to keep the 
> `containerId` private within mesos, but patch 
> [32426](https://reviews.apache.org/r/32426) effectively makes it public.
> 
> Alexander Rojas wrote:
> I got it now… it is still quite private. Forget my question on the second 
> paragraph.
> 
> Alexander Rojas wrote:
> One more thing Vinod, you wrote one container per task, however if you 
> check the code (Slave.cpp at `Executor* 
> Framework::launchExecutor(ExecutorInfo&, TaskInfo&)`) what we have is one 
> container per executor.

Docker containerizer supports multiple tasks only if the container is a custom 
executor, so you get grouping naturally through that. 
>From what I understand most people wants the container ID so they can get 
>docker specific information like name, IP, etc. I think by sending this all 
>back through the docker executor we shouldn't need this anymore.


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31985/#review76470
---


On March 20, 2015, 9:27 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31985/
> ---
> 
> (Updated March 20, 2015, 9:27 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Isabel Jimenez, Joerg Schad, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-2191
> https://issues.apache.org/jira/browse/MESOS-2191
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the executor is created, the container ID where it runs is made 
> accesible through an environment variable.
> 
> 
> Diffs
> -
> 
>   src/exec/exec.cpp d678f0682d803b0b080c3a6c296067ac9ab5dbf8 
>   src/slave/containerizer/containerizer.hpp 
> 129e60f20835f5d151701e934330b81825887af1 
>   src/slave/containerizer/containerizer.cpp 
> 4d66e767de1f877cb66b37826ba7c9d00639a7c0 
>   src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88 
>   src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 
>   src/slave/containerizer/external_containerizer.cpp 
> 42c67f548caf7bddbe131e0dfa7d74227d8c2593 
>   src/slave/containerizer/mesos/containerizer.cpp 
> fbd1c0a0e5f4f227adb022f0baaa6d2c7e3ad748 
>   src/tests/containerizer.cpp 26b87ac6b16dfeaf84888e80296ef540697bd775 
>   src/tests/slave_tests.cpp a975305430097a8295b4b155e8448572c12bde22 
> 
> Diff: https://reviews.apache.org/r/31985/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 32978: Add os::stat::inode to stout.

2015-04-13 Thread Jie Yu

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32978/#review79961
---

Ship it!


Ship It!

- Jie Yu


On April 8, 2015, 3:24 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32978/
> ---
> 
> (Updated April 8, 2015, 3:24 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Jie Yu, and Paul Brett.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add os::stat::inode to stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/stat.hpp 
> af940a48b161c194f2efb529b3d589c543b12f61 
> 
> Diff: https://reviews.apache.org/r/32978/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>



Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.

2015-04-13 Thread Jie Yu

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31444/#review79948
---


Nice test!


src/linux/fs.cpp


s/typede/types/



src/linux/fs.cpp


Remember to move '{' to the next line.



src/linux/fs.cpp


Ditto.



src/slave/containerizer/mesos/launch.cpp


Why do you need this include? Seems that list is not used.



src/slave/containerizer/mesos/launch.cpp


Ditto.



src/slave/containerizer/mesos/launch.cpp


Could you change the flag help string for --directory stating that if 
--rootfs is used, the 'directory' should be relative to the new root?



src/tests/launch_tests.cpp


Maybe add a comment about this method? What does it do?



src/tests/launch_tests.cpp


```
if (os::system("...") != 0) {
  return ErrnoError("...");
}
```



src/tests/launch_tests.cpp


Ditto. Also, do you also wanna copy /lib as well? Looking at my ubuntu 
trusty 64:

```
vagrant@vagrant-ubuntu-trusty-64:~$ uname -a
Linux vagrant-ubuntu-trusty-64 3.13.0-32-generic #57-Ubuntu SMP Tue Jul 15 
03:51:08 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
vagrant@vagrant-ubuntu-trusty-64:~$ ls /
bin  boot  dev  etc  home  initrd.img  lib  lib64  lost+found  media  mnt  
opt  proc  root  run  sbin  srv  sys  tmp  usr  vagrant  var  vmlinuz  workspace
vagrant@vagrant-ubuntu-trusty-64:~$ ls /lib64
ld-linux-x86-64.so.2
vagrant@vagrant-ubuntu-trusty-64:~$ ldd /bin/sh
linux-vdso.so.1 =>  (0x7fff23331000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x7f8c8e24)
/lib64/ld-linux-x86-64.so.2 (0x7f8c8e831000)
```



src/tests/launch_tests.cpp


Why slave mount? Shouldn't this be a SHARED mount?



src/tests/launch_tests.cpp


Please refine the comments here;)



src/tests/launch_tests.cpp


Why `// XXX` at the end of the line?



src/tests/launch_tests.cpp


Do you need stdin? Or you can just use PATH("/dev/null")?



src/tests/launch_tests.cpp


Any reason you want to do this?



src/tests/launch_tests.cpp


s/stack/Stack/



src/tests/launch_tests.cpp


Do you still need this?



src/tests/launch_tests.cpp


Add one more blank line above.


- Jie Yu


On April 8, 2015, 3:24 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31444/
> ---
> 
> (Updated April 8, 2015, 3:24 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Dominic Hamon, Jay Buffington, Jie Yu, 
> and James Peach.
> 
> 
> Bugs: MESOS-2350
> https://issues.apache.org/jira/browse/MESOS-2350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Optionally take a path that the launch helper should chroot to before 
> exec'ing the executor. It is assumed that the work directory is mounted to 
> the appropriate location under the chroot. In particular, the path to the 
> executor must be relative to the chroot.
> 
> Configuration that should be private to the chroot is done during the launch, 
> e.g. mounting proc and statically configuring basic devices. It is assumed 
> that other configuration, e.g., preparing the image, mounting in volumes or 
> persistent resources, is done by the caller.
> 
> Mounts can be made to the chroot (e.g., updating the volumes or persistent 
> resources) and they will propagate in to the container but mounts made inside 
> the container will not propagate out to the host.
> 
> It currently assumes that at least {{chroot}}/tmp is writeable and that mount 
> points {{chroot}}/{tmp,dev,proc,sys} exist in the chroot.
> 
> This is specific to Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8 
>   src/linux/fs.cpp 1c9cf3f2ffead37148e4f6a81cefdbb97f679b09 
>   src/slave/containerizer/mesos/launch.hpp 
> 7c8b535746b5ce9add00afef86fdb6faefb5620e 
>   src/slave/containe

Re: Review Request 32749: Add -Wno-unused-local-typedef for clang 3.6

2015-04-13 Thread Cody Maloney


> On April 13, 2015, 10:20 p.m., Till Toenshoff wrote:
> > configure.ac, line 579
> > 
> >
> > This is a bit unusual. We haven't marked our branches that way so far 
> > but I kinda like it.

Mesos does actually do something similar for #ifdef / #endif header include 
guards as well as namespaces, although those are both in a little different 
format.


- Cody


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32749/#review79954
---


On April 13, 2015, 9:01 p.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32749/
> ---
> 
> (Updated April 13, 2015, 9:01 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-2550
> https://issues.apache.org/jira/browse/MESOS-2550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add -Wno-unused-local-typedef for clang 3.6
> 
> 
> Diffs
> -
> 
>   configure.ac 868c0413a2e2307ae8ab2211f56874595759b139 
> 
> Diff: https://reviews.apache.org/r/32749/diff/
> 
> 
> Testing
> ---
> 
> For series:
> 
> ../configure --disable-java --disable-python CC=clang CXX=clang++
> `make check` passed on ArchLinux with clang version 3.6 as host compiler. 
> CXXFLAGS contains the typedef. Didn't build without patches.
> `make check` on ArchLinux with GCC 4.9
> 
> ../configure
> `make distcheck` on CentOS 7 (GCC 4.8)
> 
> ../configure
> `make distcheck` with OS X (Clang 3.5). Verified no 
> '-Wno-unused-local-typedef'.
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>



Jenkins build is back to normal : Mesos-Trunk-Ubuntu-Build-Out-Of-Src-Disable-Java-Disable-Python-Disable-Webui #2881

2015-04-13 Thread Apache Jenkins Server
See 




Re: Review Request 32749: Add -Wno-unused-local-typedef for clang 3.6

2015-04-13 Thread Till Toenshoff

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32749/#review79954
---

Ship it!



configure.ac


This is a bit unusual. We haven't marked our branches that way so far but I 
kinda like it.


- Till Toenshoff


On April 13, 2015, 9:01 p.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32749/
> ---
> 
> (Updated April 13, 2015, 9:01 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-2550
> https://issues.apache.org/jira/browse/MESOS-2550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add -Wno-unused-local-typedef for clang 3.6
> 
> 
> Diffs
> -
> 
>   configure.ac 868c0413a2e2307ae8ab2211f56874595759b139 
> 
> Diff: https://reviews.apache.org/r/32749/diff/
> 
> 
> Testing
> ---
> 
> For series:
> 
> ../configure --disable-java --disable-python CC=clang CXX=clang++
> `make check` passed on ArchLinux with clang version 3.6 as host compiler. 
> CXXFLAGS contains the typedef. Didn't build without patches.
> `make check` on ArchLinux with GCC 4.9
> 
> ../configure
> `make distcheck` on CentOS 7 (GCC 4.8)
> 
> ../configure
> `make distcheck` with OS X (Clang 3.5). Verified no 
> '-Wno-unused-local-typedef'.
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>



Re: Review Request 32748: libprocess: Add -Wno-unused-local-typedef for clang 3.6

2015-04-13 Thread Till Toenshoff

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32748/#review79956
---

Ship it!


Ship It!

- Till Toenshoff


On April 13, 2015, 9:01 p.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32748/
> ---
> 
> (Updated April 13, 2015, 9:01 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-2550
> https://issues.apache.org/jira/browse/MESOS-2550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> libprocess: Add -Wno-unused-local-typedef for clang 3.6
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/configure.ac a126ecfc5211710bc34690daaae65e6817beb222 
> 
> Diff: https://reviews.apache.org/r/32748/diff/
> 
> 
> Testing
> ---
> 
> See last in series: #32749
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>



Re: Review Request 32891: Support for entering and configuring a Linux chroot.

2015-04-13 Thread Jie Yu

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32891/#review79947
---



src/linux/fs.cpp


Copy comments from the other review, can you support `Option 
options` for fs::mount as well?


- Jie Yu


On April 6, 2015, 6 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32891/
> ---
> 
> (Updated April 6, 2015, 6 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Jay Buffington, Jie Yu, and James Peach.
> 
> 
> Bugs: MESOS-2350
> https://issues.apache.org/jira/browse/MESOS-2350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for entering and configuring a Linux chroot.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp d7832a4b3761c48be6c1ccef58a30ee31c70dc1b 
>   src/linux/fs.cpp 1c9cf3f2ffead37148e4f6a81cefdbb97f679b09 
> 
> Diff: https://reviews.apache.org/r/32891/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>



Re: Review Request 32749: Add -Wno-unused-local-typedef for clang 3.6

2015-04-13 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32749/#review79946
---


Patch looks great!

Reviews applied: [32747, 32748, 32749]

All tests passed.

- Mesos ReviewBot


On April 13, 2015, 9:01 p.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32749/
> ---
> 
> (Updated April 13, 2015, 9:01 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-2550
> https://issues.apache.org/jira/browse/MESOS-2550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add -Wno-unused-local-typedef for clang 3.6
> 
> 
> Diffs
> -
> 
>   configure.ac 868c0413a2e2307ae8ab2211f56874595759b139 
> 
> Diff: https://reviews.apache.org/r/32749/diff/
> 
> 
> Testing
> ---
> 
> For series:
> 
> ../configure --disable-java --disable-python CC=clang CXX=clang++
> `make check` passed on ArchLinux with clang version 3.6 as host compiler. 
> CXXFLAGS contains the typedef. Didn't build without patches.
> `make check` on ArchLinux with GCC 4.9
> 
> ../configure
> `make distcheck` on CentOS 7 (GCC 4.8)
> 
> ../configure
> `make distcheck` with OS X (Clang 3.5). Verified no 
> '-Wno-unused-local-typedef'.
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>



Re: mesos git commit: Improved compile time of by adding cpp files for flag headers.

2015-04-13 Thread Benjamin Mahler
I accidentally forgot to amend this commit with the wrapping fixes. Pushed
those out separately just now.

On Thu, Apr 9, 2015 at 6:38 PM,  wrote:

> Repository: mesos
> Updated Branches:
>   refs/heads/master 5028490f9 -> 743e9e739
>
>
> Improved compile time of by adding cpp files for flag headers.
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Review: https://reviews.apache.org/r/32558
>
>
> Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
> Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/743e9e73
> Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/743e9e73
> Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/743e9e73
>
> Branch: refs/heads/master
> Commit: 743e9e739c34ceff59db06725de6185cf64a8838
> Parents: 5028490
> Author: Cody Maloney 
> Authored: Thu Apr 9 18:24:44 2015 -0700
> Committer: Benjamin Mahler 
> Committed: Thu Apr 9 18:38:17 2015 -0700
>
> --
>  src/Makefile.am|   3 +
>  src/logging/flags.cpp  |  51 
>  src/logging/flags.hpp  |  40 +--
>  src/master/allocator/allocator.hpp |   1 +
>  src/master/flags.cpp   | 318 
>  src/master/flags.hpp   | 345 +
>  src/slave/flags.cpp| 389 +
>  src/slave/flags.hpp| 428 +---
>  8 files changed, 769 insertions(+), 806 deletions(-)
> --
>
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/743e9e73/src/Makefile.am
> --
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 9c01f5d..fa609da 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -316,10 +316,12 @@ libmesos_no_3rdparty_la_SOURCES =
>  \
> files/files.cpp \
> hook/manager.cpp\
> local/local.cpp \
> +   logging/flags.cpp   \
> logging/logging.cpp \
> master/contender.cpp\
> master/constants.cpp\
> master/detector.cpp \
> +   master/flags.cpp\
> master/http.cpp \
> master/master.cpp   \
> master/metrics.cpp  \
> @@ -335,6 +337,7 @@ libmesos_no_3rdparty_la_SOURCES =
>  \
> scheduler/scheduler.cpp \
> slave/constants.cpp \
> slave/gc.cpp\
> +   slave/flags.cpp \
> slave/http.cpp  \
> slave/metrics.cpp   \
> slave/monitor.cpp   \
>
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/743e9e73/src/logging/flags.cpp
> --
> diff --git a/src/logging/flags.cpp b/src/logging/flags.cpp
> new file mode 100644
> index 000..43c4f3c
> --- /dev/null
> +++ b/src/logging/flags.cpp
> @@ -0,0 +1,51 @@
> +/**
> + * Licensed to the Apache Software Foundation (ASF) under one
> + * or more contributor license agreements.  See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership.  The ASF licenses this file
> + * to you under the Apache License, Version 2.0 (the
> + * "License"); you may not use this file except in compliance
> + * with the License.  You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include "logging/flags.hpp"
> +
> +
> +mesos::internal::logging::Flags::Flags()
> +{
> +  add(&Flags::quiet, "quiet", "Disable logging to stderr", false);
> +
> +  add(&Flags::logging_level, "logging_level",
> +  "Log message at or above

Re: Review Request 32891: Support for entering and configuring a Linux chroot.

2015-04-13 Thread Jie Yu

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32891/#review79936
---

Ship it!


Could you add a TODO for adding a unit test for fs::enter? You probably can 
construct the new root by copying files from host file systems and do a clone 
using subprocess.


src/linux/fs.hpp


Do you want to expose these functions, or these are just helper functions 
used by fs::enter? Let's not expose these functions until we find a use case.



src/linux/fs.hpp


Can you add some comments (i.e., a short summary) about this interface?

More specifically, you should call out in the comments that this function 
should only be used with mount namespace.



src/linux/fs.cpp


One additional line above.



src/linux/fs.cpp


```
namespace internal {

struct Mount
{
  ...
};

struct Symlink
{
  ...
};

} // namespace internal {
```

Does the above work?



src/linux/fs.cpp


Put '{' to the next line.



src/linux/fs.cpp


Mind putting this function as well as createStandardDevices into internal 
namespace as they are not exposed.



src/linux/fs.cpp


One blank line above.



src/linux/fs.cpp


"/tmp exists in the new root and is writable;..."


- Jie Yu


On April 6, 2015, 6 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32891/
> ---
> 
> (Updated April 6, 2015, 6 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Jay Buffington, Jie Yu, and James Peach.
> 
> 
> Bugs: MESOS-2350
> https://issues.apache.org/jira/browse/MESOS-2350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for entering and configuring a Linux chroot.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp d7832a4b3761c48be6c1ccef58a30ee31c70dc1b 
>   src/linux/fs.cpp 1c9cf3f2ffead37148e4f6a81cefdbb97f679b09 
> 
> Diff: https://reviews.apache.org/r/32891/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>



Re: Review Request 32747: libprocess: Place noreturn attribute correctly for C11

2015-04-13 Thread Cody Maloney

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32747/
---

(Updated April 13, 2015, 9:20 p.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Update description to include that this is changing a patch to libev, not 
libprocess itself.


Bugs: MESOS-2550
https://issues.apache.org/jira/browse/MESOS-2550


Repository: mesos


Description (updated)
---

libprocess: Place bundled libev noreturn attribute correctly for C11

The bundled libev uses different noreturn attributes based on whether
it is compiled with C11 or not. The C11 codepath places the noreturn 
attribute in an incorrect (but accepted by GCC) location. Move it to 
the standard location which is accepted by all supported compilers.


Diffs
-

  3rdparty/libprocess/3rdparty/libev-4.15.patch 
2b94532634bf4fb69523cdfa26254de6c537d689 

Diff: https://reviews.apache.org/r/32747/diff/


Testing
---

See last in series: #32749


Thanks,

Cody Maloney



Re: Review Request 32747: libprocess: Place noreturn attribute correctly for C11

2015-04-13 Thread Till Toenshoff

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32747/#review79942
---

Ship it!


Let's add to summary and/or description that we are targetting libev 
specifically with this patch.

- Till Toenshoff


On April 13, 2015, 9:01 p.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32747/
> ---
> 
> (Updated April 13, 2015, 9:01 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-2550
> https://issues.apache.org/jira/browse/MESOS-2550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> libprocess: Place noreturn attribute correctly for C11
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/libev-4.15.patch 
> 2b94532634bf4fb69523cdfa26254de6c537d689 
> 
> Diff: https://reviews.apache.org/r/32747/diff/
> 
> 
> Testing
> ---
> 
> See last in series: #32749
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>



Re: Review Request 32975: MESOS-1790 Adds chown option to CommandInfo.URI

2015-04-13 Thread Jim Klucar


> On April 13, 2015, 5:20 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, line 710
> > 
> >
> > I know you just followed the example of other tests in the file, but 
> > this test (and other tests in this file) should really ensure that any temp 
> > files are properly cleaned up in even if the test fails. Current if the 
> > test fails for whatever reason, the cleanup on #756 won't be executed 
> > leaking this file.
> > 
> > You could use 'environment->mkdtemp()' instead of 'os::mktemp()' to get 
> > the cleanup for free in the short term.
> > 
> > s/os::mktemp()/environment->mkdtemp()/
> > 
> > In the long term, TemporaryDirectoryTest fixture should really have a 
> > mkdtemp() method on it that deletes temporary directories on TearDown. If 
> > you want to do this and fix the tests in this file, please do that in 
> > another review.

I've fixed this for the two new tests using environment->mkdtemp(). Should I 
submit a new JIRA for the TemporaryDirectoryTest fix?


> On April 13, 2015, 5:20 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, line 708
> > 
> >
> > s/archive/archived/ ?

fixed as suggested


> On April 13, 2015, 5:20 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, line 709
> > 
> >
> > kill extraneous space.
> > 
> > s/tar  gzip/tar gzip/

fixed as suggested


> On April 13, 2015, 5:20 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, line 728
> > 
> >
> > unused?

removed


> On April 13, 2015, 5:20 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, line 751
> > 
> >
> > s/chowntestuser/'chowntestuser'/
> > s/returned/returns/

fixed as suggested


> On April 13, 2015, 5:20 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, line 755
> > 
> >
> > s/cleanup/Cleanup/
> > s/file/file./

fixed as suggested


> On April 13, 2015, 5:20 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, line 762
> > 
> >
> > s/archive/archived/ ?

fixed as suggested


> On April 13, 2015, 5:20 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, line 782
> > 
> >
> > unused?

removed


> On April 13, 2015, 5:20 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, line 805
> > 
> >
> > s/despite/Despite/
> > 
> > s/chowntestuser/'chowntestuser'/
> > 
> > s/process/fetching/

fixed as suggested


> On April 13, 2015, 5:20 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, line 809
> > 
> >
> > s/cleanup/Cleanup/
> > s/file/file./

fixed as suggested


- Jim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32975/#review79898
---


On April 13, 2015, 4:55 p.m., Jim Klucar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32975/
> ---
> 
> (Updated April 13, 2015, 4:55 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1790
> https://issues.apache.org/jira/browse/MESOS-1790
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added chown to CommandInfo.URI protocol buffer as an optional
> boolean that defaults to true, the current chown behavior.
> 
> The fetcher was updated to skip the os::chown operation if the chown
> boolean is set to false.
> 
> No documentation was updated.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3c592d5ab3092ecbeddfaff95e0c1addc3ac58f8 
>   src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b 
>   src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
>   src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e 
> 
> Diff: https://reviews.apache.org/r/32975/diff/
> 
> 
> Testing
> ---
> 
> Unit testing this functionality is difficult because it would require that 
> the user running the test to have permission to chown a file to someone other 
> than themselves. I didn't want to add that as a requirement to build. I added 
> the new field to the existing test cases just to see that they populate.
> 
> 
> T

Re: Review Request 32748: libprocess: Add -Wno-unused-local-typedef for clang 3.6

2015-04-13 Thread Cody Maloney

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32748/
---

(Updated April 13, 2015, 9:01 p.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Remove unnecessary template in check pointed out by till.


Bugs: MESOS-2550
https://issues.apache.org/jira/browse/MESOS-2550


Repository: mesos


Description
---

libprocess: Add -Wno-unused-local-typedef for clang 3.6


Diffs (updated)
-

  3rdparty/libprocess/configure.ac a126ecfc5211710bc34690daaae65e6817beb222 

Diff: https://reviews.apache.org/r/32748/diff/


Testing
---

See last in series: #32749


Thanks,

Cody Maloney



Re: Review Request 32749: Add -Wno-unused-local-typedef for clang 3.6

2015-04-13 Thread Cody Maloney

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32749/
---

(Updated April 13, 2015, 9:01 p.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Remove unnecessary template in check pointed out by till.


Bugs: MESOS-2550
https://issues.apache.org/jira/browse/MESOS-2550


Repository: mesos


Description
---

Add -Wno-unused-local-typedef for clang 3.6


Diffs (updated)
-

  configure.ac 868c0413a2e2307ae8ab2211f56874595759b139 

Diff: https://reviews.apache.org/r/32749/diff/


Testing
---

For series:

../configure --disable-java --disable-python CC=clang CXX=clang++
`make check` passed on ArchLinux with clang version 3.6 as host compiler. 
CXXFLAGS contains the typedef. Didn't build without patches.
`make check` on ArchLinux with GCC 4.9

../configure
`make distcheck` on CentOS 7 (GCC 4.8)

../configure
`make distcheck` with OS X (Clang 3.5). Verified no '-Wno-unused-local-typedef'.


Thanks,

Cody Maloney



Re: Review Request 32747: libprocess: Place noreturn attribute correctly for C11

2015-04-13 Thread Cody Maloney

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32747/
---

(Updated April 13, 2015, 9:01 p.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Add mesos group


Bugs: MESOS-2550
https://issues.apache.org/jira/browse/MESOS-2550


Repository: mesos


Description
---

libprocess: Place noreturn attribute correctly for C11


Diffs
-

  3rdparty/libprocess/3rdparty/libev-4.15.patch 
2b94532634bf4fb69523cdfa26254de6c537d689 

Diff: https://reviews.apache.org/r/32747/diff/


Testing
---

See last in series: #32749


Thanks,

Cody Maloney



Re: Review Request 32975: MESOS-1790 Adds chown option to CommandInfo.URI

2015-04-13 Thread Jim Klucar


> On April 8, 2015, 9:57 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, lines 112-116
> > 
> >
> > Add a note/todo here mentioning why you are setting these fields but 
> > not doing any asserts/expects on them.
> 
> Jim Klucar wrote:
> With the type_utils.cpp change, the EXPECT_EQ on lines 140/141 should be 
> checking the chown field. Should I still add text regarding how we can't 
> check this capability without assuming the user running the test has 
> permission to chown to some other user?
> 
> Vinod Kone wrote:
> Yes. Because the EXPECT on #140 just checks that the chown field is 
> properly propagated to fetcher environment but not that fetcher actually 
> skips chown if chown is false.
> 
> If you write a FetcherTest (see examples later in the file) that actually 
> runs a fetcher process, would you able to test the semantics? I'm thinking 
> that you can create a non-existent user (random string) and set chown to 
> false. Once fetcher process finishes, you can check that the owner of the 
> fetched file hasn't changed to the user. Does that make sense?
> 
> Jim Klucar wrote:
> This makes sense. I'll create the test, back out the change, see the test 
> fail, then add the change back in.

I ended up creating two new tests. Both extract a framework, one sets chown 
true and expects a failure due to the unknown user name. The other sets chown 
to false and expects the fetcher to finish without error.


- Jim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32975/#review79433
---


On April 13, 2015, 4:55 p.m., Jim Klucar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32975/
> ---
> 
> (Updated April 13, 2015, 4:55 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1790
> https://issues.apache.org/jira/browse/MESOS-1790
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added chown to CommandInfo.URI protocol buffer as an optional
> boolean that defaults to true, the current chown behavior.
> 
> The fetcher was updated to skip the os::chown operation if the chown
> boolean is set to false.
> 
> No documentation was updated.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3c592d5ab3092ecbeddfaff95e0c1addc3ac58f8 
>   src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b 
>   src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
>   src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e 
> 
> Diff: https://reviews.apache.org/r/32975/diff/
> 
> 
> Testing
> ---
> 
> Unit testing this functionality is difficult because it would require that 
> the user running the test to have permission to chown a file to someone other 
> than themselves. I didn't want to add that as a requirement to build. I added 
> the new field to the existing test cases just to see that they populate.
> 
> 
> Thanks,
> 
> Jim Klucar
> 
>



Seeking High IOPS Applications for Large Datasets and Low Latency

2015-04-13 Thread Kevin Kramer (GS)
Flo and Matt from Mesosphere suggested I query this list.   I got some great
responses off the user@ list over the past few weeks and a few people
suggested I ping the dev list.

 

We are putting the finishing touches on a hyperconverged system that can
push 10M IOPS at <200 microsecond latency for datasets between 5 and 280 TB
per node.   I'm looking for people building applications that want to
leverage this kind of horsepower without having to shard their data across
10s or 100s of machines.  We find most other approaches are struggling to
perform at all over 500K-1M IOPS where we provide consistent performance up
to 10M IOPS.  My challenge is that not all apps want this much I/O at scale.

 

We fully support grid computing, we just think the nodes could/should be a
"little" larger for analytics workloads.

 

I have a bit more background on what we are building below.  We are in
market a validation and feedback phase with beta product now and a
production product coming later this year.  

 

I am not looking for sales targets, more for market validation and use case
discussions.  I appreciate any ideas that I could get from the Mesos
community as I know you are all looking at large scale computing challenges.

 

Cheers,

Kevin

 

 

Kevin Kramer

 

Graphite Systems

"10M+ IOPS in a single low-latency 20-140+TB Appliance"

kkra...@graphitesystems.com

+1 (415) 475-1987

 

 

 

Background:

Graphite Systems is a 2.5 year old stealth mode company funded by NEA.   We
are seeking market feedback, advice, and a couple of testing partners as we
ready our product for market in the Hyperconverged systems space.  

We are still working with beta product and are not selling the product at
this time (product GA will be later this year so this is
advisory/brainstorming and not sales engagements at this time).


The short story is we at 10x today's approaches like Exadata at around 10%
of the cost: 

. 10X the "in-memory" capacity of today's servers (5.5TB of RAM plus
20-280TB of FnRAM accessible at 60+GB/s)

. 10X the speed of typical Flash Arrays (69GB/s) for larger datasets


. 10X maximum IOPS of other converged systems (~10M IOPS)

. 5-8X lower latency than the leading Flash Arrays (little
degradation for larger than 8K blocks to make this 10-20X for larger block
sizes)


We are a server/system and not storage - we have 60 cores and 5.5TB RAM
available in the system to process Application, OLAP and Datawarehouse
workloads while providing access to this 20-280TB datastore for IOPS
intensive applications with very low latency.  Many flash array vendors are
struggling for 1M IOPS and 1-3 mS latency and can't scale above 20/30/40TB.
Graphite is looking at 10M+ IOPS and .13-.20 mS (130-200 microseconds)
latency for larger datasets - 20-280TB of FnRAM.   We can do things like
read a sector immediately after writing it which is a challenge for most
other solutions.

 



Re: Review Request 33109: Allow setting environment variables in mesos-execute

2015-04-13 Thread Adam B

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33109/#review79925
---


Thanks for the patch! I'm a little busy at ApacheCon this week, but will try to 
review asap. Maybe others can make a pass sooner.

- Adam B


On April 13, 2015, 9:42 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33109/
> ---
> 
> (Updated April 13, 2015, 9:42 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-2023
> https://issues.apache.org/jira/browse/MESOS-2023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting environment variables in mesos-execute
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 84f70dccbc2c5dd43f68105d967f4488c82f582b 
> 
> Diff: https://reviews.apache.org/r/33109/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 32998: Split committer's guide into code reviewing and committing documents.

2015-04-13 Thread haosdent huang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32998/#review79912
---



docs/effective-code-reviewing.md


Should we add a requirement that "pass unit tests before post reviews" here?


- haosdent huang


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/32998/
> ---
> 
> (Updated April 9, 2015, 12:30 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Jie Yu, Niklas Nielsen, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-2581
> https://issues.apache.org/jira/browse/MESOS-2581
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> "Committer's Guide" was too generic. This names the documents after "what" 
> the reader is looking for: doing effective reviews, and how to commit changes 
> (for committers only).
> 
> 
> Diffs
> -
> 
>   docs/committers-guide.md c016ee9cb3290d7788ed258904547b59bbea4f11 
>   docs/committing.md PRE-CREATION 
>   docs/effective-code-reviewing.md PRE-CREATION 
>   docs/home.md 6ab61f85aa7d62e0f19267b886dffb4e0aa826ea 
> 
> Diff: https://reviews.apache.org/r/32998/diff/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Build failed in Jenkins: Mesos-Trunk-Ubuntu-Build-Out-Of-Src-Disable-Java-Disable-Python-Disable-Webui #2880

2015-04-13 Thread Apache Jenkins Server
See 
<https://builds.apache.org/job/Mesos-Trunk-Ubuntu-Build-Out-Of-Src-Disable-Java-Disable-Python-Disable-Webui/2880/changes>

Changes:

[vinodkone] Should install apr-util-devel in CentOS 6.5.

--
[...truncated 73720 lines...]
IRe-registered executor on proserpina.apache.org
0413 17:58:32.181589 31730 exec.cpp:241] Executor::reregistered took 91445ns
I0413 17:58:32.181962 20773 slave.cpp:2503] Handling status update TASK_LOST 
(UUID: 60f127b7-b58a-461e-95c7-8ea7b79b7912) for task 
f7f156f8-f9ba-4ee4-bcc2-9a434e9a65c2 of framework 
20150413-175831-3176252227-60468-20747- from @0.0.0.0:0
I0413 17:58:32.182065 20773 slave.cpp:4519] Terminating task 
f7f156f8-f9ba-4ee4-bcc2-9a434e9a65c2
I0413 17:58:32.206502 20764 status_update_manager.cpp:317] Received status 
update TASK_LOST (UUID: 60f127b7-b58a-461e-95c7-8ea7b79b7912) for task 
f7f156f8-f9ba-4ee4-bcc2-9a434e9a65c2 of framework 
20150413-175831-3176252227-60468-20747-
I0413 17:58:32.206568 20764 status_update_manager.cpp:494] Creating 
StatusUpdate stream for task f7f156f8-f9ba-4ee4-bcc2-9a434e9a65c2 of framework 
20150413-175831-3176252227-60468-20747-
I0413 17:58:32.206914 20765 containerizer.cpp:950] Destroying container 
'95035a3f-79ce-453b-935f-a0e0b650f34e'
I0413 17:58:32.207056 20764 status_update_manager.hpp:346] Checkpointing UPDATE 
for status update TASK_LOST (UUID: 60f127b7-b58a-461e-95c7-8ea7b79b7912) for 
task f7f156f8-f9ba-4ee4-bcc2-9a434e9a65c2 of framework 
20150413-175831-3176252227-60468-20747-
I0413 17:58:32.294944 20764 status_update_manager.cpp:371] Forwarding update 
TASK_LOST (UUID: 60f127b7-b58a-461e-95c7-8ea7b79b7912) for task 
f7f156f8-f9ba-4ee4-bcc2-9a434e9a65c2 of framework 
20150413-175831-3176252227-60468-20747- to the slave
W0413 17:58:32.295214 20776 slave.cpp:2713] Dropping status update TASK_LOST 
(UUID: 60f127b7-b58a-461e-95c7-8ea7b79b7912) for task 
f7f156f8-f9ba-4ee4-bcc2-9a434e9a65c2 of framework 
20150413-175831-3176252227-60468-20747- sent by status update manager 
because the slave is in RECOVERING state
I0413 17:58:32.295289 20776 slave.cpp:2685] Status update manager successfully 
handled status update TASK_LOST (UUID: 60f127b7-b58a-461e-95c7-8ea7b79b7912) 
for task f7f156f8-f9ba-4ee4-bcc2-9a434e9a65c2 of framework 
20150413-175831-3176252227-60468-20747-
I0413 17:58:32.350344 20767 containerizer.cpp:1159] Executor for container 
'95035a3f-79ce-453b-935f-a0e0b650f34e' has exited
I0413 17:58:32.351619 20771 slave.cpp:3194] Executor 
'f7f156f8-f9ba-4ee4-bcc2-9a434e9a65c2' of framework 
20150413-175831-3176252227-60468-20747- terminated with signal Killed
I0413 17:58:32.351910 20747 sched.cpp:1589] Asked to stop the driver
I0413 17:58:32.352051 20747 master.cpp:788] Master terminating
I0413 17:58:32.352062 20763 sched.cpp:831] Stopping framework 
'20150413-175831-3176252227-60468-20747-'
I0413 17:58:32.352313 20771 hierarchical.hpp:470] Removed slave 
20150413-175831-3176252227-60468-20747-S0
W0413 17:58:32.352293 20747 master.cpp:4707] Removing task 
f7f156f8-f9ba-4ee4-bcc2-9a434e9a65c2 with resources cpus(*):2; mem(*):1024; 
disk(*):1024; ports(*):[31000-32000] of framework 
20150413-175831-3176252227-60468-20747- on slave 
20150413-175831-3176252227-60468-20747-S0 at slave(231)@67.195.81.189:60468 
(proserpina.apache.org) in non-terminal state TASK_STAGING
I0413 17:58:32.353174 20776 hierarchical.hpp:354] Removed framework 
20150413-175831-3176252227-60468-20747-
I0413 17:58:32.358019 20775 slave.cpp:509] Slave terminating
[   OK ] MesosContainerizerSlaveRecoveryTest.ResourceStatistics (846 ms)
[--] 1 test from MesosContainerizerSlaveRecoveryTest (846 ms total)

[--] 1 test from MasterContenderDetectorTest
[ RUN  ] MasterContenderDetectorTest.File
Using temporary directory '/tmp/MasterContenderDetectorTest_File_HXHDNc'
I0413 17:58:32.486649 20747 leveldb.cpp:176] Opened db in 122.163085ms
I0413 17:58:32.528734 20747 leveldb.cpp:183] Compacted db in 42.000124ms
I0413 17:58:32.528802 20747 leveldb.cpp:198] Created db iterator in 24914ns
I0413 17:58:32.528820 20747 leveldb.cpp:204] Seeked to beginning of db in 2504ns
I0413 17:58:32.528830 20747 leveldb.cpp:273] Iterated through 0 keys in the db 
in 393ns
I0413 17:58:32.528878 20747 replica.cpp:744] Replica recovered with log 
positions 0 -> 0 with 1 holes and 0 unlearned
I0413 17:58:32.529469 20770 recover.cpp:449] Starting replica recovery
I0413 17:58:32.529829 20770 recover.cpp:475] Replica is in EMPTY status
I0413 17:58:32.531082 20774 replica.cpp:641] Replica in EMPTY status received a 
broadcasted recover request
I0413 17:58:32.531353 20769 recover.cpp:195] Received a recover response from a 
replica in EMPTY status
I0413 17:58:32.532062 20772 recover.cpp:566] Updating replica status to STARTING
I0413 17:58:32.533360 20765 master.cpp:359] Master 
20150413-175832-3176252227-60468-20747 (proserp

Re: Review Request 32585: Replaced Framework.id with Framework.id() in Master/Slave.

2015-04-13 Thread Kapil Arya


> On April 13, 2015, 1:47 p.m., Ben Mahler wrote:
> > src/master/master.hpp, lines 1100-1103
> > 
> >
> > Did you look back at https://reviews.apache.org/r/19176/ when doing 
> > this?
> > 
> > What was the motivation for making this function instead of a field? 
> > Either way (1) the 'id' cannot be changed, and (2) there are two ways to 
> > access the ID: id() and info.id(). Is there any benefit I'm missing? Just 
> > seems like unnecessary churn in the code to me.
> > 
> > Note that this change means the `Slave` and `Framework` structs now 
> > store their ID's differently as well, which is unfortunate.

I did look ar 19176 and part of the upgrade path is motivated by your earlier 
review :-).

For the id, I guess the alternative is to remove id() altogether and replace it 
with info.id() since the latter is always supposed to be valid. If this is 
preferred, I can create a new RR to address it.

Lastly, I am not sure if I understand what you mean by the 'Slave' struct. Do 
you mean `SlaveID` used for the `Slave` struct used in src/master/? I haven't 
looked closely at that but I can see if we can remove SlaveID from that struct 
as well (in favor of SlaveInfo.id()).


- Kapil


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32585/#review79904
---


On April 7, 2015, 12:59 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32585/
> ---
> 
> (Updated April 7, 2015, 12:59 p.m.)
> 
> 
> Review request for mesos, Adam B and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2557
> https://issues.apache.org/jira/browse/MESOS-2557
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Framework.id() extracts and returns FrameworkID from Framework.info.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp e1a87d646e9690e39a9e84ae383622018ce80401 
>   src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d 
>   src/master/master.cpp dccd7c635da4b7031cd109bd84e7f17b31777ef1 
>   src/master/validation.cpp 2f2e4ea8ea123c5a0d01446cdec8b308ea60932e 
>   src/slave/http.cpp 5f0c39afd2fe9a89eb1df0052af8ab422306f30e 
>   src/slave/slave.hpp 19e6b44bc344c0ca509674803f401cbb4e1f47ae 
>   src/slave/slave.cpp c7e65a6c095963feaa9d5fdbb519c68f8f761d16 
> 
> Diff: https://reviews.apache.org/r/32585/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> TODO: Test for upgrade path.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 32585: Replaced Framework.id with Framework.id() in Master/Slave.

2015-04-13 Thread Ben Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32585/#review79904
---



src/master/master.hpp


Did you look back at https://reviews.apache.org/r/19176/ when doing this?

What was the motivation for making this function instead of a field? Either 
way (1) the 'id' cannot be changed, and (2) there are two ways to access the 
ID: id() and info.id(). Is there any benefit I'm missing? Just seems like 
unnecessary churn in the code to me.

Note that this change means the `Slave` and `Framework` structs now store 
their ID's differently as well, which is unfortunate.


- Ben Mahler


On April 7, 2015, 4:59 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32585/
> ---
> 
> (Updated April 7, 2015, 4:59 p.m.)
> 
> 
> Review request for mesos, Adam B and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2557
> https://issues.apache.org/jira/browse/MESOS-2557
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Framework.id() extracts and returns FrameworkID from Framework.info.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp e1a87d646e9690e39a9e84ae383622018ce80401 
>   src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d 
>   src/master/master.cpp dccd7c635da4b7031cd109bd84e7f17b31777ef1 
>   src/master/validation.cpp 2f2e4ea8ea123c5a0d01446cdec8b308ea60932e 
>   src/slave/http.cpp 5f0c39afd2fe9a89eb1df0052af8ab422306f30e 
>   src/slave/slave.hpp 19e6b44bc344c0ca509674803f401cbb4e1f47ae 
>   src/slave/slave.cpp c7e65a6c095963feaa9d5fdbb519c68f8f761d16 
> 
> Diff: https://reviews.apache.org/r/32585/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> TODO: Test for upgrade path.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 32975: MESOS-1790 Adds chown option to CommandInfo.URI

2015-04-13 Thread Ian Downes

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32975/#review79902
---



src/tests/fetcher_tests.cpp


Why not use a random username here, like a UUID. Or, check if the user 
exist.



src/tests/fetcher_tests.cpp


unused?



src/tests/fetcher_tests.cpp


unused?



src/tests/fetcher_tests.cpp


indentation



src/tests/fetcher_tests.cpp


unused?



src/tests/fetcher_tests.cpp


unused?



src/tests/fetcher_tests.cpp


indentation


- Ian Downes


On April 13, 2015, 9:55 a.m., Jim Klucar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32975/
> ---
> 
> (Updated April 13, 2015, 9:55 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1790
> https://issues.apache.org/jira/browse/MESOS-1790
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added chown to CommandInfo.URI protocol buffer as an optional
> boolean that defaults to true, the current chown behavior.
> 
> The fetcher was updated to skip the os::chown operation if the chown
> boolean is set to false.
> 
> No documentation was updated.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3c592d5ab3092ecbeddfaff95e0c1addc3ac58f8 
>   src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b 
>   src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
>   src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e 
> 
> Diff: https://reviews.apache.org/r/32975/diff/
> 
> 
> Testing
> ---
> 
> Unit testing this functionality is difficult because it would require that 
> the user running the test to have permission to chown a file to someone other 
> than themselves. I didn't want to add that as a requirement to build. I added 
> the new field to the existing test cases just to see that they populate.
> 
> 
> Thanks,
> 
> Jim Klucar
> 
>



Re: Review Request 27113: Libprocess benchmark cleanup

2015-04-13 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27113/
---

(Updated April 13, 2015, 5:43 p.m.)


Review request for Cody Maloney, Joerg Schad and Michael Park.


Changes
---

addressed Cody's comments.


Bugs: MESOS-1980
https://issues.apache.org/jira/browse/MESOS-1980


Repository: mesos


Description
---

Clean up Libprocess for readability / extensibility.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/benchmarks.cpp 
a927e4ecd8c8955cd9f85e716173a73a9a21c6cd 

Diff: https://reviews.apache.org/r/27113/diff/


Testing
---

make check


Thanks,

Joris Van Remoortere



Re: Review Request 32975: MESOS-1790 Adds chown option to CommandInfo.URI

2015-04-13 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32975/#review79900
---


Patch looks great!

Reviews applied: [32975]

All tests passed.

- Mesos ReviewBot


On April 13, 2015, 4:55 p.m., Jim Klucar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32975/
> ---
> 
> (Updated April 13, 2015, 4:55 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1790
> https://issues.apache.org/jira/browse/MESOS-1790
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added chown to CommandInfo.URI protocol buffer as an optional
> boolean that defaults to true, the current chown behavior.
> 
> The fetcher was updated to skip the os::chown operation if the chown
> boolean is set to false.
> 
> No documentation was updated.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3c592d5ab3092ecbeddfaff95e0c1addc3ac58f8 
>   src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b 
>   src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
>   src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e 
> 
> Diff: https://reviews.apache.org/r/32975/diff/
> 
> 
> Testing
> ---
> 
> Unit testing this functionality is difficult because it would require that 
> the user running the test to have permission to chown a file to someone other 
> than themselves. I didn't want to add that as a requirement to build. I added 
> the new field to the existing test cases just to see that they populate.
> 
> 
> Thanks,
> 
> Jim Klucar
> 
>



Re: Review Request 32975: MESOS-1790 Adds chown option to CommandInfo.URI

2015-04-13 Thread Vinod Kone

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32975/#review79898
---



src/tests/fetcher_tests.cpp


s/archive/archived/ ?



src/tests/fetcher_tests.cpp


kill extraneous space.

s/tar  gzip/tar gzip/



src/tests/fetcher_tests.cpp


I know you just followed the example of other tests in the file, but this 
test (and other tests in this file) should really ensure that any temp files 
are properly cleaned up in even if the test fails. Current if the test fails 
for whatever reason, the cleanup on #756 won't be executed leaking this file.

You could use 'environment->mkdtemp()' instead of 'os::mktemp()' to get the 
cleanup for free in the short term.

s/os::mktemp()/environment->mkdtemp()/

In the long term, TemporaryDirectoryTest fixture should really have a 
mkdtemp() method on it that deletes temporary directories on TearDown. If you 
want to do this and fix the tests in this file, please do that in another 
review.



src/tests/fetcher_tests.cpp


unused?



src/tests/fetcher_tests.cpp


s/chowntestuser/'chowntestuser'/
s/returned/returns/



src/tests/fetcher_tests.cpp


s/cleanup/Cleanup/
s/file/file./



src/tests/fetcher_tests.cpp


s/archive/archived/ ?



src/tests/fetcher_tests.cpp


s/tar  gzip/tar gzip/



src/tests/fetcher_tests.cpp


unused?



src/tests/fetcher_tests.cpp


s/despite/Despite/

s/chowntestuser/'chowntestuser'/

s/process/fetching/



src/tests/fetcher_tests.cpp


s/cleanup/Cleanup/
s/file/file./


- Vinod Kone


On April 13, 2015, 4:55 p.m., Jim Klucar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32975/
> ---
> 
> (Updated April 13, 2015, 4:55 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1790
> https://issues.apache.org/jira/browse/MESOS-1790
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added chown to CommandInfo.URI protocol buffer as an optional
> boolean that defaults to true, the current chown behavior.
> 
> The fetcher was updated to skip the os::chown operation if the chown
> boolean is set to false.
> 
> No documentation was updated.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3c592d5ab3092ecbeddfaff95e0c1addc3ac58f8 
>   src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b 
>   src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
>   src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e 
> 
> Diff: https://reviews.apache.org/r/32975/diff/
> 
> 
> Testing
> ---
> 
> Unit testing this functionality is difficult because it would require that 
> the user running the test to have permission to chown a file to someone other 
> than themselves. I didn't want to add that as a requirement to build. I added 
> the new field to the existing test cases just to see that they populate.
> 
> 
> Thanks,
> 
> Jim Klucar
> 
>



Re: Review Request 32975: MESOS-1790 Adds chown option to CommandInfo.URI

2015-04-13 Thread Jim Klucar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32975/
---

(Updated April 13, 2015, 4:55 p.m.)


Review request for mesos.


Bugs: MESOS-1790
https://issues.apache.org/jira/browse/MESOS-1790


Repository: mesos


Description
---

Added chown to CommandInfo.URI protocol buffer as an optional
boolean that defaults to true, the current chown behavior.

The fetcher was updated to skip the os::chown operation if the chown
boolean is set to false.

No documentation was updated.


Diffs (updated)
-

  include/mesos/mesos.proto 3c592d5ab3092ecbeddfaff95e0c1addc3ac58f8 
  src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b 
  src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
  src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e 

Diff: https://reviews.apache.org/r/32975/diff/


Testing
---

Unit testing this functionality is difficult because it would require that the 
user running the test to have permission to chown a file to someone other than 
themselves. I didn't want to add that as a requirement to build. I added the 
new field to the existing test cases just to see that they populate.


Thanks,

Jim Klucar



Re: Review Request 33111: Should install apr-util-devel in CentOS 6.5

2015-04-13 Thread Timothy St. Clair

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33111/#review79896
---

Ship it!


Ship It!

- Timothy St. Clair


On April 12, 2015, 9:33 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33111/
> ---
> 
> (Updated April 12, 2015, 9:33 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2611
> https://issues.apache.org/jira/browse/MESOS-2611
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Should install apr-util-devel in CentOS 6.5
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md b1d08e68bfce8097468e244f219bd178b6a33f57 
> 
> Diff: https://reviews.apache.org/r/33111/diff/
> 
> 
> Testing
> ---
> 
> apr-utils-devel should be apr-util-devel
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 33111: Should install apr-util-devel in CentOS 6.5

2015-04-13 Thread Vinod Kone

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33111/#review79895
---

Ship it!


Ship It!

- Vinod Kone


On April 12, 2015, 9:33 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33111/
> ---
> 
> (Updated April 12, 2015, 9:33 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2611
> https://issues.apache.org/jira/browse/MESOS-2611
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Should install apr-util-devel in CentOS 6.5
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md b1d08e68bfce8097468e244f219bd178b6a33f57 
> 
> Diff: https://reviews.apache.org/r/33111/diff/
> 
> 
> Testing
> ---
> 
> apr-utils-devel should be apr-util-devel
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 32975: MESOS-1790 Adds chown option to CommandInfo.URI

2015-04-13 Thread Jim Klucar


> On April 8, 2015, 9:57 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, lines 112-116
> > 
> >
> > Add a note/todo here mentioning why you are setting these fields but 
> > not doing any asserts/expects on them.
> 
> Jim Klucar wrote:
> With the type_utils.cpp change, the EXPECT_EQ on lines 140/141 should be 
> checking the chown field. Should I still add text regarding how we can't 
> check this capability without assuming the user running the test has 
> permission to chown to some other user?
> 
> Vinod Kone wrote:
> Yes. Because the EXPECT on #140 just checks that the chown field is 
> properly propagated to fetcher environment but not that fetcher actually 
> skips chown if chown is false.
> 
> If you write a FetcherTest (see examples later in the file) that actually 
> runs a fetcher process, would you able to test the semantics? I'm thinking 
> that you can create a non-existent user (random string) and set chown to 
> false. Once fetcher process finishes, you can check that the owner of the 
> fetched file hasn't changed to the user. Does that make sense?

This makes sense. I'll create the test, back out the change, see the test fail, 
then add the change back in.


- Jim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32975/#review79433
---


On April 9, 2015, 4:40 p.m., Jim Klucar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32975/
> ---
> 
> (Updated April 9, 2015, 4:40 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1790
> https://issues.apache.org/jira/browse/MESOS-1790
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added chown to CommandInfo.URI protocol buffer as an optional
> boolean that defaults to true, the current chown behavior.
> 
> The fetcher was updated to skip the os::chown operation if the chown
> boolean is set to false.
> 
> No documentation was updated.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3c592d5ab3092ecbeddfaff95e0c1addc3ac58f8 
>   src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b 
>   src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
>   src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e 
> 
> Diff: https://reviews.apache.org/r/32975/diff/
> 
> 
> Testing
> ---
> 
> Unit testing this functionality is difficult because it would require that 
> the user running the test to have permission to chown a file to someone other 
> than themselves. I didn't want to add that as a requirement to build. I added 
> the new field to the existing test cases just to see that they populate.
> 
> 
> Thanks,
> 
> Jim Klucar
> 
>



Re: Review Request 30774: Fetcher Cache

2015-04-13 Thread Bernd Mathiske

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30774/
---

(Updated April 13, 2015, 5:45 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy 
Chen.


Changes
---

Addressed the latest 2 issues.


Bugs: MESOS-2057, MESOS-2069, MESOS-2070, MESOS-2072, MESOS-2073, and MESOS-2074
https://issues.apache.org/jira/browse/MESOS-2057
https://issues.apache.org/jira/browse/MESOS-2069
https://issues.apache.org/jira/browse/MESOS-2070
https://issues.apache.org/jira/browse/MESOS-2072
https://issues.apache.org/jira/browse/MESOS-2073
https://issues.apache.org/jira/browse/MESOS-2074


Repository: mesos


Description
---

Almost all of the functionality in epic MESOS-336. Downloaded files from 
CommandInfo::URIs can now be cached in a cache directory designated by a slave 
flag. This only happens when asked for by an extra flag in the URI and is thus 
backwards-compatible. The cache has a size limit also given by a new slave 
flag. Cache-resident files are evicted as necessary to make space for newly 
fetched ones. Concurrent attempts to cache the same URI leads to only one 
download. The fetcher program remains external for safety reasons, but is now 
augmented with more elaborate parameters packed into a JSON object to implement 
specific fetch actions for all of the above. Additional testing includes 
fetching from (mock) HDFS and coverage of the new features.


Diffs (updated)
-

  docs/configuration.md 54c4e31ed6dfed3c23d492c19a301ce119a0519b 
  docs/fetcher-cache-internals.md PRE-CREATION 
  docs/fetcher.md PRE-CREATION 
  include/mesos/fetcher/fetcher.proto 311af9aebc6a85dadba9dbeffcf7036b70896bcc 
  include/mesos/mesos.proto 3a8e8bf303e0576c212951f6028af77e54d93537 
  include/mesos/type_utils.hpp cdf5864389a72002b538c263d70bcade2bdffa45 
  src/Makefile.am d15a37365bcdd5c3906160b46b389635b38b1673 
  src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b 
  src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
  src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec 
  src/slave/containerizer/docker.hpp 6893684e6d199a5d69fc8bba8e60c4acaae9c3c9 
  src/slave/containerizer/docker.cpp f9fb07806e3b7d7d2afc1be3b8756eac23b32dcd 
  src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
  src/slave/containerizer/fetcher.cpp 9e9e9d0eb6b0801d53dec3baea32a4cd4acdd5e2 
  src/slave/containerizer/mesos/containerizer.hpp 
ae61a0fcd19f2ba808624312401f020121baf5d4 
  src/slave/containerizer/mesos/containerizer.cpp 
e4136095fca55637864f495098189ab3ad8d8fe7 
  src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
  src/slave/flags.cpp 35f56252cfda5011d21aa188f33cc3e68a694968 
  src/slave/slave.cpp a0595f93ce4720f5b9926326d01210460ccb0667 
  src/tests/docker_containerizer_tests.cpp 
c772d4c836de18b0e87636cb42200356d24ec73d 
  src/tests/fetcher_cache_tests.cpp PRE-CREATION 
  src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e 
  src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4 
  src/tests/mesos.cpp fc534e9febed1e293076e00e0f5c3879a78df90f 

Diff: https://reviews.apache.org/r/30774/diff/


Testing
---

make check

--- longer Description: ---

-Replaces all other reviews for the fetcher cache except those related to 
stout: 30006, 30033, 30034, 30036, 30037, 30039, 30124, 30173, 30614, 30616, 
30618, 30621, 30626. See descriptions of those. In dependency order:

30033: Removes the fetcher env tests since these won't be needed any more when 
the fetcher uses JSON in a single env var as a parameter. They never tested 
anything that won't be covered by other tests anyway.

30034: Makes the code structure of all fetcher tests the same. Instead of 
calling the run method of the fetcher directly, calling through fetch(). Also 
removes all uses of I/O redirection, which is not really needed for debugging, 
and thus the next patch can refactor fetch() and run(). (The latter comes in 
two varieties, which complicates matters without much benefit.)

30036: Extends the CommandInfo::URI protobuf with a boolean "caching" field 
that will later cause fetcher cache actions. Also introduces the notion of a 
cache directory to the fetcher info protobuf. And then propagates these 
additions throughout the rest of the code base where applicable. This includes 
passing the slave ID all the way down to the place where the cache dir name is 
constructed.

30037: Extends the fetcher info protobuf with "actions" (fetch directly 
bypassing the cache, fetch through the cache, retrieve from the cache). 
Switches the basis for dealing with uris to "items", which contain the uri, the 
action, and potentially a cache file name. Refactors fetch() and run(), so 
there is only one of each. Introduces about half of the actual cache logic, 
including a hashmap

Re: Review Request 30774: Fetcher Cache

2015-04-13 Thread Bernd Mathiske


> On March 18, 2015, 11:05 a.m., Timothy Chen wrote:
> > src/slave/containerizer/fetcher.cpp, line 491
> > 
> >
> > Why not just mock _fetch and do a barrier on it by giving it a promise 
> > in test?
> 
> Bernd Mathiske wrote:
> "just mock _fetch" is more work and harder to understand.
> 
> It would also function, but then you would need to touch test code every 
> time you change _fetch(). Furthermore, it would not be as clear why we wait 
> for this particular call.

Meanwhile I tried mocking _fetch, but it does not work. See the 
related/duplicate issue below. Let's drop this one here now so we can keep the 
comments on the same topic and code region in one place going forward, OK?


- Bernd


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30774/#review76902
---


On April 10, 2015, 4:33 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30774/
> ---
> 
> (Updated April 10, 2015, 4:33 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-2057, MESOS-2069, MESOS-2070, MESOS-2072, MESOS-2073, and 
> MESOS-2074
> https://issues.apache.org/jira/browse/MESOS-2057
> https://issues.apache.org/jira/browse/MESOS-2069
> https://issues.apache.org/jira/browse/MESOS-2070
> https://issues.apache.org/jira/browse/MESOS-2072
> https://issues.apache.org/jira/browse/MESOS-2073
> https://issues.apache.org/jira/browse/MESOS-2074
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Almost all of the functionality in epic MESOS-336. Downloaded files from 
> CommandInfo::URIs can now be cached in a cache directory designated by a 
> slave flag. This only happens when asked for by an extra flag in the URI and 
> is thus backwards-compatible. The cache has a size limit also given by a new 
> slave flag. Cache-resident files are evicted as necessary to make space for 
> newly fetched ones. Concurrent attempts to cache the same URI leads to only 
> one download. The fetcher program remains external for safety reasons, but is 
> now augmented with more elaborate parameters packed into a JSON object to 
> implement specific fetch actions for all of the above. Additional testing 
> includes fetching from (mock) HDFS and coverage of the new features.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 54c4e31ed6dfed3c23d492c19a301ce119a0519b 
>   docs/fetcher-cache-internals.md PRE-CREATION 
>   docs/fetcher.md PRE-CREATION 
>   include/mesos/fetcher/fetcher.proto 
> 311af9aebc6a85dadba9dbeffcf7036b70896bcc 
>   include/mesos/mesos.proto 3a8e8bf303e0576c212951f6028af77e54d93537 
>   include/mesos/type_utils.hpp cdf5864389a72002b538c263d70bcade2bdffa45 
>   src/Makefile.am fa609da08e23d6595a3f6d2efddd3e333b6c78f1 
>   src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b 
>   src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
>   src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec 
>   src/slave/containerizer/docker.hpp 6893684e6d199a5d69fc8bba8e60c4acaae9c3c9 
>   src/slave/containerizer/docker.cpp f9fb07806e3b7d7d2afc1be3b8756eac23b32dcd 
>   src/slave/containerizer/fetcher.hpp 
> 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
>   src/slave/containerizer/fetcher.cpp 
> 9e9e9d0eb6b0801d53dec3baea32a4cd4acdd5e2 
>   src/slave/containerizer/mesos/containerizer.hpp 
> ae61a0fcd19f2ba808624312401f020121baf5d4 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e4136095fca55637864f495098189ab3ad8d8fe7 
>   src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
>   src/slave/flags.cpp 35f56252cfda5011d21aa188f33cc3e68a694968 
>   src/slave/slave.cpp 9fec023b643d410f4d511fa6f80e9835bab95b7e 
>   src/tests/docker_containerizer_tests.cpp 
> c772d4c836de18b0e87636cb42200356d24ec73d 
>   src/tests/fetcher_cache_tests.cpp PRE-CREATION 
>   src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e 
>   src/tests/mesos.hpp 0e98572a62ae05437bd2bc800c370ad1a0c43751 
>   src/tests/mesos.cpp 02cbb4b8cf1206d0f32d160addc91d7e0f1ab28b 
> 
> Diff: https://reviews.apache.org/r/30774/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> --- longer Description: ---
> 
> -Replaces all other reviews for the fetcher cache except those related to 
> stout: 30006, 30033, 30034, 30036, 30037, 30039, 30124, 30173, 30614, 30616, 
> 30618, 30621, 30626. See descriptions of those. In dependency order:
> 
> 30033: Removes the fetcher env tests since these won't be needed any more 
> when the fetcher uses JSON in a single env var as a parameter. They never 
> tested anything that won't be 

Re: Review Request 30774: Fetcher Cache

2015-04-13 Thread Bernd Mathiske


> On April 12, 2015, 11:11 p.m., Timothy Chen wrote:
> > src/tests/fetcher_cache_tests.cpp, line 308
> > 
> >
> > Not sure why you picked an arbitrary number 5 here, why not let it be 
> > passed in?

OK, I will add an explanation in a comment. Two requirements need to be met by 
this constant.
- It needs to be larger than the expected number of status updates. We might 
choose something much larger than 5, but all tests run just fine with 5.
- It needs to be finite. Otherwise we will keep waiting for updates when none 
arrive due to a bug.

However, if we passed this constant in, then we would need to explain it at all 
the call sites, i.e. multiple times instead of only once. But the situation is 
exactly the same every time. So I will refrain from that.


- Bernd


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30774/#review79838
---


On April 10, 2015, 4:33 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30774/
> ---
> 
> (Updated April 10, 2015, 4:33 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-2057, MESOS-2069, MESOS-2070, MESOS-2072, MESOS-2073, and 
> MESOS-2074
> https://issues.apache.org/jira/browse/MESOS-2057
> https://issues.apache.org/jira/browse/MESOS-2069
> https://issues.apache.org/jira/browse/MESOS-2070
> https://issues.apache.org/jira/browse/MESOS-2072
> https://issues.apache.org/jira/browse/MESOS-2073
> https://issues.apache.org/jira/browse/MESOS-2074
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Almost all of the functionality in epic MESOS-336. Downloaded files from 
> CommandInfo::URIs can now be cached in a cache directory designated by a 
> slave flag. This only happens when asked for by an extra flag in the URI and 
> is thus backwards-compatible. The cache has a size limit also given by a new 
> slave flag. Cache-resident files are evicted as necessary to make space for 
> newly fetched ones. Concurrent attempts to cache the same URI leads to only 
> one download. The fetcher program remains external for safety reasons, but is 
> now augmented with more elaborate parameters packed into a JSON object to 
> implement specific fetch actions for all of the above. Additional testing 
> includes fetching from (mock) HDFS and coverage of the new features.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 54c4e31ed6dfed3c23d492c19a301ce119a0519b 
>   docs/fetcher-cache-internals.md PRE-CREATION 
>   docs/fetcher.md PRE-CREATION 
>   include/mesos/fetcher/fetcher.proto 
> 311af9aebc6a85dadba9dbeffcf7036b70896bcc 
>   include/mesos/mesos.proto 3a8e8bf303e0576c212951f6028af77e54d93537 
>   include/mesos/type_utils.hpp cdf5864389a72002b538c263d70bcade2bdffa45 
>   src/Makefile.am fa609da08e23d6595a3f6d2efddd3e333b6c78f1 
>   src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b 
>   src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
>   src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec 
>   src/slave/containerizer/docker.hpp 6893684e6d199a5d69fc8bba8e60c4acaae9c3c9 
>   src/slave/containerizer/docker.cpp f9fb07806e3b7d7d2afc1be3b8756eac23b32dcd 
>   src/slave/containerizer/fetcher.hpp 
> 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
>   src/slave/containerizer/fetcher.cpp 
> 9e9e9d0eb6b0801d53dec3baea32a4cd4acdd5e2 
>   src/slave/containerizer/mesos/containerizer.hpp 
> ae61a0fcd19f2ba808624312401f020121baf5d4 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e4136095fca55637864f495098189ab3ad8d8fe7 
>   src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
>   src/slave/flags.cpp 35f56252cfda5011d21aa188f33cc3e68a694968 
>   src/slave/slave.cpp 9fec023b643d410f4d511fa6f80e9835bab95b7e 
>   src/tests/docker_containerizer_tests.cpp 
> c772d4c836de18b0e87636cb42200356d24ec73d 
>   src/tests/fetcher_cache_tests.cpp PRE-CREATION 
>   src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e 
>   src/tests/mesos.hpp 0e98572a62ae05437bd2bc800c370ad1a0c43751 
>   src/tests/mesos.cpp 02cbb4b8cf1206d0f32d160addc91d7e0f1ab28b 
> 
> Diff: https://reviews.apache.org/r/30774/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> --- longer Description: ---
> 
> -Replaces all other reviews for the fetcher cache except those related to 
> stout: 30006, 30033, 30034, 30036, 30037, 30039, 30124, 30173, 30614, 30616, 
> 30618, 30621, 30626. See descriptions of those. In dependency order:
> 
> 30033: Removes the fetcher env tests since these won't be needed any more 
> when the fetcher uses JSON in a single env var 

Re: Changing Mesos Minimum Compiler Version

2015-04-13 Thread Bernd Mathiske
+1

> On Apr 10, 2015, at 6:02 PM, Michael Park  wrote:
> 
> +1
> 
> On 9 April 2015 at 17:33, Alexander Gallego  wrote:
> 
>> This is amazing for native devs/frameworks.
>> 
>> Sent from my iPhone
>> 
>>> On Apr 9, 2015, at 5:16 PM, Joris Van Remoortere 
>> wrote:
>>> 
>>> +1
>>> 
 On Thu, Apr 9, 2015 at 2:14 PM, Cody Maloney 
>> wrote:
 As discussed in the last community meeting, we'd like to bump the
>> minimum required compiler version from GCC 4.4 to GCC 4.8.
 
 The overall goals are to make Mesos development safer, faster, and
>> reduce the maintenance burden. Currently a lot of stout has different
>> codepaths for Pre-C++11 and Post-C++11compilers.
 
 Progress will be tracked in the JIRA: MESOS-2604
 
 The resulting supported compiler versions will be:
 GCC 4.8, GCC 4.9
 Clang 3.5, Clang 3.6
 
 For reference
 Compilers by Distribution Version: http://goo.gl/p1t1ls
 
 C++11 features supported by each compiler:
 https://gcc.gnu.org/projects/cxx0x.html
 http://clang.llvm.org/cxx_status.html
>>> 
>> 



Re: Changing Mesos Minimum Compiler Version

2015-04-13 Thread Bernd Mathiske
Yeah! +1

> On Apr 10, 2015, at 6:02 PM, Michael Park  wrote:
> 
> +1
> 
> On 9 April 2015 at 17:33, Alexander Gallego  wrote:
> 
>> This is amazing for native devs/frameworks.
>> 
>> Sent from my iPhone
>> 
>>> On Apr 9, 2015, at 5:16 PM, Joris Van Remoortere 
>> wrote:
>>> 
>>> +1
>>> 
 On Thu, Apr 9, 2015 at 2:14 PM, Cody Maloney 
>> wrote:
 As discussed in the last community meeting, we'd like to bump the
>> minimum required compiler version from GCC 4.4 to GCC 4.8.
 
 The overall goals are to make Mesos development safer, faster, and
>> reduce the maintenance burden. Currently a lot of stout has different
>> codepaths for Pre-C++11 and Post-C++11compilers.
 
 Progress will be tracked in the JIRA: MESOS-2604
 
 The resulting supported compiler versions will be:
 GCC 4.8, GCC 4.9
 Clang 3.5, Clang 3.6
 
 For reference
 Compilers by Distribution Version: http://goo.gl/p1t1ls
 
 C++11 features supported by each compiler:
 https://gcc.gnu.org/projects/cxx0x.html
 http://clang.llvm.org/cxx_status.html
>>> 
>>