Re: Review Request 54999: Fixed test flakiness due to floating point conversions.

2016-12-29 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [54999]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Dec. 28, 2016, 9:23 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54999/
> ---
> 
> (Updated Dec. 28, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Bannier.
> 
> 
> Bugs: MESOS-6837
> https://issues.apache.org/jira/browse/MESOS-6837
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `FaultToleranceTest.FrameworkReregister` and
> `MasterTest.FailoverAgentReregisterFirst` both examined a timestamp
> value returned by an HTTP endpoint. Such values are the result of
> several conversions (`double` to `string` to `JSON::Value`); this might
> result in the returned value being an integer one larger/smaller than we
> expect. Hence, make the comparison within an epsilon of 1.
> 
> A similar issue in `SlaveTest.StateEndpoint` was fixed in
> 595c929f2816b713b4c36ce1bd23a7767afe8135.
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> 24747fab20f5b107b9c23a271b753e83f05bbee3 
>   src/tests/master_tests.cpp 2d0cd8244ded44e76f0eee3d87327ff526db5208 
>   src/tests/slave_tests.cpp 67a6aed8c66a21c94c106b52dff75cbdc41fcf69 
> 
> Diff: https://reviews.apache.org/r/54999/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Note that I wasn't able to repro the test failure on my laptop, but it was 
> observed on CI.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55068: Added Capabilities to Framework struct of Hierarchical allocator.

2016-12-29 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [55021, 55066, 55068]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Dec. 28, 2016, 8:32 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55068/
> ---
> 
> (Updated Dec. 28, 2016, 8:32 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Individual capabilities are collected into this struct and used in
> allocator. This is step toward multi-role support of allocator.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> a6424d624864155e1c87a28a63b784512c5c8722 
>   src/master/allocator/mesos/hierarchical.cpp 
> 91b1ec43940a788459f045ca4a4b82d4e8373bca 
> 
> Diff: https://reviews.apache.org/r/55068/diff/
> 
> 
> Testing
> ---
> 
> make check on Ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.

2016-12-29 Thread Aaron Wood 


> On Dec. 27, 2016, 1:25 p.m., Till Toenshoff wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp, line 81
> > 
> >
> > As you have no further use for the returned value, I guess we could 
> > simply spare the temporary and make it a bit more concise and also 
> > C++'esque by prventing the C-style cast;
> > 
> > ```
> >   if (posix_memalign(static_cast(>address), 16, 
> > stack->size) !=
> >   0) {
> > return -1;
> >   }
> > ```

Sure, I will address this in combination with Jie's comments next week!


- Aaron


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


On Dec. 29, 2016, 7:03 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
> 
> (Updated Dec. 29, 2016, 7:03 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently in the Linux launcher when the stack is allocated and prepared for 
> a call to clone() it is not properly aligned. This is not an issue for x86 or 
> x64 but for ARM64/AArch64 it is because of the requirement of having the 
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack 
> to have a 16 byte aligned stack, it is not enforced. An explanation of the 
> stack and requirements for ARM64 can be found here 
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
>  (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be 
> quad-word aligned.)
> 
> Additionally, the way that the stack is currently allocated and passed to 
> clone() accidentally chops off one entry, making a stack overflow using those 
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix 
> both the issue of the stack overflow issue as well as the SIGBUS crash. We 
> should also net better performance from having the stack aligned.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/linux.hpp 530f1a55b 
> 
> Diff: https://reviews.apache.org/r/54996/diff/
> 
> 
> Testing
> ---
> 
> Built Mesos from source and am currently running it in a test cluster. 
> Launched both Docker and Mesos tasks via Marathon without any resulting crash 
> (initial crash only happened with Mesos containerizer + linux_launcher, not 
> with the posix_launcher).
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.

2016-12-29 Thread Aaron Wood 

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

(Updated Dec. 29, 2016, 7:03 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description (updated)
---

Currently in the Linux launcher when the stack is allocated and prepared for a 
call to clone() it is not properly aligned. This is not an issue for x86 or x64 
but for ARM64/AArch64 it is because of the requirement of having the stack 
aligned to a 16 byte boundary. While x86 and x64 also expect the stack to have 
a 16 byte aligned stack, it is not enforced. An explanation of the stack and 
requirements for ARM64 can be found here 
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf 
(specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be 
quad-word aligned.)

Additionally, the way that the stack is currently allocated and passed to 
clone() accidentally chops off one entry, making a stack overflow using those 
missing 8 bytes a possibility. Fixing this while aligning the memory will fix 
both the issue of the stack overflow issue as well as the SIGBUS crash. We 
should also net better performance from having the stack aligned.


Diffs
-

  3rdparty/stout/include/stout/os/linux.hpp 530f1a55b 

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


Testing
---

Built Mesos from source and am currently running it in a test cluster. Launched 
both Docker and Mesos tasks via Marathon without any resulting crash (initial 
crash only happened with Mesos containerizer + linux_launcher, not with the 
posix_launcher).


Thanks,

Aaron Wood

Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.

2016-12-29 Thread Jie Yu 

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




3rdparty/stout/include/stout/os/linux.hpp (lines 76 - 77)


`Stack` is also used in src/linux/ns.hpp. We need to update there as well. 
Would you mind running a make check (or sudo make check) after applying this 
patch? That'll expose the error.

I would probably move all the allocation/deallocation logic to `os::Stack` 
so that we don't have to impl this multiple times.
```
class Stack
{
public:
  // Allocate a stack.
  static Try create(size_t size);
  
  // Explicitly free the stack. The destructor
  // won't free the allocated stack.
  void deallocate();

private:
  explicit Stack(size_t size);
  
  size_t size;
  char* address;
};
```


- Jie Yu


On Dec. 22, 2016, 9:24 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
> 
> (Updated Dec. 22, 2016, 9:24 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently in the Linux launcher when the stack is allocated and prepared for 
> a call to clone() it is not properly aligned. This is not an issue for x86 or 
> x64 but for ARM64/AArch64 it is because of the requirement of having the 
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack 
> to have a 16 byte aligned stack, it is not enforced. An explanation of the 
> stack and requirements for ARM64 can be found here 
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
>  (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be 
> quad-word aligned.)
> 
> Additionally, the way that the stack is currently allocated and passed to 
> clone() accidentally chops off one entry, making a stack overflow using those 
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix 
> both the issue of the stack overflow issue as well as the SIGBUS crash.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/linux.hpp 530f1a55b 
> 
> Diff: https://reviews.apache.org/r/54996/diff/
> 
> 
> Testing
> ---
> 
> Built Mesos from source and am currently running it in a test cluster. 
> Launched both Docker and Mesos tasks via Marathon without any resulting crash 
> (initial crash only happened with Mesos containerizer + linux_launcher, not 
> with the posix_launcher).
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 55065: Added support for s390x architecture

2016-12-29 Thread Mesos ReviewBot 

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On Dec. 28, 2016, 8:24 a.m., Ayanampudi Varsha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55065/
> ---
> 
> (Updated Dec. 28, 2016, 8:24 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-6742
> https://issues.apache.org/jira/browse/MESOS-6742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for s390x architecture
> There are 2 issues addressed in this patch:
> 1. LdcacheTest.Parse test case fails on s390x machines.
> 2. From the value of flag docker_registry in slave/flags.cpp, amd64 images 
> get downloaded due to which test cases fail on s390x with "Exec format Error"
> 
> 
> Diffs
> -
> 
>   src/linux/ldcache.cpp 6260496672cd0226a24ea1b34003a48af9678a63 
>   src/slave/flags.cpp 1eccea920338032173be4df6c374ec50dbd2eaf3 
> 
> Diff: https://reviews.apache.org/r/55065/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ayanampudi Varsha
> 
>

Re: Review Request 50629: Allow using protobuf 3.0 (MESOS-5186).

2016-12-29 Thread Deshi Xiao 

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



good for me.

- Deshi Xiao


On 七月 30, 2016, 6 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50629/
> ---
> 
> (Updated 七月 30, 2016, 6 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Deshi Xiao.
> 
> 
> Bugs: MESOS-5186
> https://issues.apache.org/jira/browse/MESOS-5186
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix updated the requirement in `python/interface/setup.py.in`
> so that protobuf 3.0 is allowed.
> 
> 
> Diffs
> -
> 
>   src/python/interface/setup.py.in 037c2ec8e63f497f7029a847a7a0d7b72e6f36fa 
> 
> Diff: https://reviews.apache.org/r/50629/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>