Re: Review Request 42780: Changed the NetClsIsolatorTest to check for net_cls handles.

[Avinash sridharan]

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

(Updated Jan. 31, 2016, 6:23 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Changed the NetClsIsolatorTest to check for net_cls handles.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
6510553b412ae70f981949754c207fb5f7e1c220 

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


Testing (updated)
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 43023: Avoid construction of temporary strings in `NumberWriter` for doubles.

[Michael Park]

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

(Updated Jan. 31, 2016, 4:51 a.m.)


Review request for mesos, Benjamin Hindman and Joris Van Remoortere.


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


Repository: mesos


Description (updated)
---

With this patch + https://reviews.apache.org/r/43024/, the number of calls to 
`operator new` and `operator delete` were reduced by roughly 1/3.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp 
addec8ec6504e2a8f5b838fce3ebd4db224ab022 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 43023: Avoid construction of temporary strings in `NumberWriter` for doubles.

[Michael Park]

> On Jan. 31, 2016, 1:43 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, line 162
> > 
> >
> > I assume we have some evidence that this optimization is warranted?
> 
> Michael Park wrote:
> With this patch + https://reviews.apache.org/r/43024/, the # of calls to 
> `operator new` and `operator delete` reduces by roughly 1/3.
> 
> Neil Conway wrote:
> Can we add this to the commit message?

Yep, added to the description for now.


> On Jan. 31, 2016, 1:43 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, line 173
> > 
> >
> > This seems a little more subtle than is warranted.
> 
> Michael Park wrote:
> The `i` part perhaps? Is it better if we were to call it `back`?
> ```
> *stream_ << buffer << (buffer[back] == '.' ? "0" : "");
> ```
> 
> Neil Conway wrote:
> Yeah -- not clear that "i" never points to the NUL character for 
> non-empty strings, etc. Probably would be clearer without the trinary 
> expression.

The ternary expression was there from before though, that's not something to be 
"warranted" right?
Are you ok with this if I were to `s/i/back/`? or is it not sufficient?


- Michael


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


On Jan. 31, 2016, 12:06 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43023/
> ---
> 
> (Updated Jan. 31, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4566
> https://issues.apache.org/jira/browse/MESOS-4566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp 
> addec8ec6504e2a8f5b838fce3ebd4db224ab022 
> 
> Diff: https://reviews.apache.org/r/43023/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 43024: Avoid unnecessary string copies in `json` for protobuf messages.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43023, 43024]

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

- Mesos ReviewBot


On Jan. 31, 2016, 12:06 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43024/
> ---
> 
> (Updated Jan. 31, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4566
> https://issues.apache.org/jira/browse/MESOS-4566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 6d7d033884cc6ed451e25bfc4638ac9186bbbcf5 
> 
> Diff: https://reviews.apache.org/r/43024/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 43023: Avoid construction of temporary strings in `NumberWriter` for doubles.

[Neil Conway]

> On Jan. 31, 2016, 1:43 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, line 173
> > 
> >
> > This seems a little more subtle than is warranted.
> 
> Michael Park wrote:
> The `i` part perhaps? Is it better if we were to call it `back`?
> ```
> *stream_ << buffer << (buffer[back] == '.' ? "0" : "");
> ```

Yeah -- not clear that "i" never points to the NUL character for non-empty 
strings, etc. Probably would be clearer without the trinary expression.


> On Jan. 31, 2016, 1:43 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, line 162
> > 
> >
> > I assume we have some evidence that this optimization is warranted?
> 
> Michael Park wrote:
> With this patch + https://reviews.apache.org/r/43024/, the # of calls to 
> `operator new` and `operator delete` reduces by roughly 1/3.

Can we add this to the commit message?


- Neil


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


On Jan. 31, 2016, 12:06 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43023/
> ---
> 
> (Updated Jan. 31, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4566
> https://issues.apache.org/jira/browse/MESOS-4566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp 
> addec8ec6504e2a8f5b838fce3ebd4db224ab022 
> 
> Diff: https://reviews.apache.org/r/43023/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 43023: Avoid construction of temporary strings in `NumberWriter` for doubles.

[Michael Park]

> On Jan. 31, 2016, 1:43 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, line 173
> > 
> >
> > This seems a little more subtle than is warranted.

The `i` part perhaps? Is it better if we were to call it `back`?
```
*stream_ << buffer << (buffer[back] == '.' ? "0" : "");
```


- Michael


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


On Jan. 31, 2016, 12:06 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43023/
> ---
> 
> (Updated Jan. 31, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4566
> https://issues.apache.org/jira/browse/MESOS-4566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp 
> addec8ec6504e2a8f5b838fce3ebd4db224ab022 
> 
> Diff: https://reviews.apache.org/r/43023/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 43023: Avoid construction of temporary strings in `NumberWriter` for doubles.

[Michael Park]

> On Jan. 31, 2016, 1:43 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, line 162
> > 
> >
> > I assume we have some evidence that this optimization is warranted?

With this patch + https://reviews.apache.org/r/43024/, the # of calls to 
`operator new` and `operator delete` reduces by roughly 1/3.


- Michael


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


On Jan. 31, 2016, 12:06 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43023/
> ---
> 
> (Updated Jan. 31, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4566
> https://issues.apache.org/jira/browse/MESOS-4566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp 
> addec8ec6504e2a8f5b838fce3ebd4db224ab022 
> 
> Diff: https://reviews.apache.org/r/43023/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 43023: Avoid construction of temporary strings in `NumberWriter` for doubles.

[Neil Conway]

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




3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp (line 151)


Not yours, but I think initializing this buffer to `{}` is not good style. 
(It suggests there might be code paths in which the buffer is not subsequently 
assigned to. If we leave it uninitialized, we get a compiler warning if we try 
to read from it before setting it, which is actually what we want.)



3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp (line 162)


I assume we have some evidence that this optimization is warranted?



3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp (line 173)


This seems a little more subtle than is warranted.


- Neil Conway


On Jan. 31, 2016, 12:06 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43023/
> ---
> 
> (Updated Jan. 31, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4566
> https://issues.apache.org/jira/browse/MESOS-4566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp 
> addec8ec6504e2a8f5b838fce3ebd4db224ab022 
> 
> Diff: https://reviews.apache.org/r/43023/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 42988: Changed ZooKeeper reconnection logic to retry more aggressively.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42987, 42988]

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

- Mesos ReviewBot


On Jan. 30, 2016, 10:20 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42988/
> ---
> 
> (Updated Jan. 30, 2016, 10:20 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4546
> https://issues.apache.org/jira/browse/MESOS-4546
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous implementation of `GroupProcess` tried to establish a single
> ZooKeeper connection on startup, but didn't attempt to retry. ZooKeeper will
> retry internally, but it only retries by attempting to reconnect to a list of
> previously resolved IPs; it doesn't attempt to re-resolve those IPs to pickup
> updates to DNS configuration. Because DNS configuration can be quite dynamic,
> we now close the current Zk handle and open a new one if we've seen a
> successful `zookeeper_init` but haven't been connected within the ZooKeeper
> session timeout.
> 
> 
> Diffs
> -
> 
>   src/tests/group_tests.cpp 77349465e0163c8aa6bed6deefe3f98efb442f3d 
>   src/zookeeper/group.hpp cf82fec290a2fa9bec122539c2eb0f12b45c2fb2 
>   src/zookeeper/group.cpp 2ae3193e0e138c90b205d45400d80e80853e1b99 
>   src/zookeeper/zookeeper.cpp 3c4fdad972dcd1728c52a05970646c713dcf98c8 
> 
> Diff: https://reviews.apache.org/r/42988/diff/
> 
> 
> Testing
> ---
> 
> make check, on both OSX and Arch Linux. Manually configured a situation in 
> which the Mesos agent uses stale DNS information in a loop: validated that 
> without the patch, we don't pickup DNS changes, whereas with the patch, we do.
> 
> Also added a new unit test. Verified that the test fails w/o this patch 
> applied and passes deterministically (`gtest_repeat=100`) with the patch 
> applied.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42986: Updated webui to show count for tasks in `TASK_RUNNING` state.

[Kapil Arya]

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

(Updated Jan. 30, 2016, 7:28 p.m.)


Review request for mesos, Ben Mahler and Joris Van Remoortere.


Changes
---

Updated variable names.


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


Repository: mesos


Description (updated)
---

Previously, it was showing the count for tasks in `TASK_STARTING` state.
Also, renamed "Started" to "Running" in the webui.


Diffs (updated)
-

  src/webui/master/static/home.html f6d2f7d0923553ba0bb88e52f541c4e8c740c7c1 
  src/webui/master/static/js/controllers.js 
36865f1cd6a4518ba30cd6e64d4d7347762066ec 
  src/webui/master/static/slave.html ad37ee2d1fab60e40762bf8c313754e1c59fe2a3 

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


Testing
---

Launched 100 sleeps tasks and verified the status in webui.


Thanks,

Kapil Arya

Review Request 43023: Avoid construction of temporary strings in `NumberWriter` for doubles.

[Michael Park]

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

Review request for mesos, Benjamin Hindman and Joris Van Remoortere.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp 
addec8ec6504e2a8f5b838fce3ebd4db224ab022 

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


Testing
---

`make check`


Thanks,

Michael Park

Review Request 43024: Avoid unnecessary string copies in `json` for protobuf messages.

[Michael Park]

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

Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
6d7d033884cc6ed451e25bfc4638ac9186bbbcf5 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 42910: Added a note about revocable resources beyond quota in the user doc.

[Joris Van Remoortere]

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


Ship it!





docs/quota.md (line 281)


s/has/have/



docs/quota.md (line 289)


s/may/will/


- Joris Van Remoortere


On Jan. 29, 2016, 10:58 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42910/
> ---
> 
> (Updated Jan. 29, 2016, 10:58 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/quota.md 2762557ad1cec0655a27c4dda4016e07c7f63cd9 
> 
> Diff: https://reviews.apache.org/r/42910/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

[Timothy Chen]

> On Jan. 30, 2016, 7:31 p.m., Joerg Schad wrote:
> > src/docker/executor.cpp, line 135
> > 
> >
> > I am not sure why we are printing this, but shouldn't we at least check 
> > for errors on os::shell?

Thought I alrady removed all of these, good catch.


- Timothy


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


On Jan. 30, 2016, 5:25 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43015/
> ---
> 
> (Updated Jan. 30, 2016, 5:25 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed persistent volumes with docker tasks.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
> 
> Diff: https://reviews.apache.org/r/43015/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

[Timothy Chen]

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

(Updated Jan. 30, 2016, 10:43 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Fixed persistent volumes with docker tasks.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
  src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

[Timothy Chen]

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




src/slave/containerizer/docker.cpp (line 992)


It can't


- Timothy Chen


On Jan. 30, 2016, 5:25 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43015/
> ---
> 
> (Updated Jan. 30, 2016, 5:25 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed persistent volumes with docker tasks.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
> 
> Diff: https://reviews.apache.org/r/43015/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 42988: Changed ZooKeeper reconnection logic to retry more aggressively.

[Neil Conway]

> On Jan. 30, 2016, 9:53 p.m., Joris Van Remoortere wrote:
> > src/tests/group_tests.cpp, lines 451-452
> > 
> >
> > Maybe a comment explaining that we're triggering the timeout? Or is 
> > this too self-explanatory?

Done.


- Neil


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


On Jan. 30, 2016, 10:20 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42988/
> ---
> 
> (Updated Jan. 30, 2016, 10:20 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4546
> https://issues.apache.org/jira/browse/MESOS-4546
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous implementation of `GroupProcess` tried to establish a single
> ZooKeeper connection on startup, but didn't attempt to retry. ZooKeeper will
> retry internally, but it only retries by attempting to reconnect to a list of
> previously resolved IPs; it doesn't attempt to re-resolve those IPs to pickup
> updates to DNS configuration. Because DNS configuration can be quite dynamic,
> we now close the current Zk handle and open a new one if we've seen a
> successful `zookeeper_init` but haven't been connected within the ZooKeeper
> session timeout.
> 
> 
> Diffs
> -
> 
>   src/tests/group_tests.cpp 77349465e0163c8aa6bed6deefe3f98efb442f3d 
>   src/zookeeper/group.hpp cf82fec290a2fa9bec122539c2eb0f12b45c2fb2 
>   src/zookeeper/group.cpp 2ae3193e0e138c90b205d45400d80e80853e1b99 
>   src/zookeeper/zookeeper.cpp 3c4fdad972dcd1728c52a05970646c713dcf98c8 
> 
> Diff: https://reviews.apache.org/r/42988/diff/
> 
> 
> Testing
> ---
> 
> make check, on both OSX and Arch Linux. Manually configured a situation in 
> which the Mesos agent uses stale DNS information in a loop: validated that 
> without the patch, we don't pickup DNS changes, whereas with the patch, we do.
> 
> Also added a new unit test. Verified that the test fails w/o this patch 
> applied and passes deterministically (`gtest_repeat=100`) with the patch 
> applied.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42988: Changed ZooKeeper reconnection logic to retry more aggressively.

[Neil Conway]

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

(Updated Jan. 30, 2016, 10:20 p.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Revise comments, per code review. Add a `CHECK_NONE` assert to 
`startConnection`.


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


Repository: mesos


Description
---

The previous implementation of `GroupProcess` tried to establish a single
ZooKeeper connection on startup, but didn't attempt to retry. ZooKeeper will
retry internally, but it only retries by attempting to reconnect to a list of
previously resolved IPs; it doesn't attempt to re-resolve those IPs to pickup
updates to DNS configuration. Because DNS configuration can be quite dynamic,
we now close the current Zk handle and open a new one if we've seen a
successful `zookeeper_init` but haven't been connected within the ZooKeeper
session timeout.


Diffs (updated)
-

  src/tests/group_tests.cpp 77349465e0163c8aa6bed6deefe3f98efb442f3d 
  src/zookeeper/group.hpp cf82fec290a2fa9bec122539c2eb0f12b45c2fb2 
  src/zookeeper/group.cpp 2ae3193e0e138c90b205d45400d80e80853e1b99 
  src/zookeeper/zookeeper.cpp 3c4fdad972dcd1728c52a05970646c713dcf98c8 

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


Testing
---

make check, on both OSX and Arch Linux. Manually configured a situation in 
which the Mesos agent uses stale DNS information in a loop: validated that 
without the patch, we don't pickup DNS changes, whereas with the patch, we do.

Also added a new unit test. Verified that the test fails w/o this patch applied 
and passes deterministically (`gtest_repeat=100`) with the patch applied.


Thanks,

Neil Conway

Re: Review Request 42988: Changed ZooKeeper reconnection logic to retry more aggressively.

[Joris Van Remoortere]

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


Fix it, then Ship it!




Thanks for taking this on Neil!
As we found out, this code is not the easiest to reason through.
I left some issues for places we may be able to make it easier to read through 
the state assertions for the next set of readers.


src/tests/group_tests.cpp (lines 445 - 446)


Can we add a short comment as to the state we're trying to achieve here?

I think it will help readers of the test.



src/tests/group_tests.cpp (lines 451 - 452)


Maybe a comment explaining that we're triggering the timeout? Or is this 
too self-explanatory?



src/zookeeper/group.cpp (lines 128 - 137)


Not yours:
Can we add a comment that we don't need to clean up the `delay` `Timer`s 
because they won't be invoked if libprocess can no longer get a 
`ProcessReference` to this Actor?



src/zookeeper/group.cpp (line 154)


Should we s/promptly/within the sessionTimeout/ to be more clear?



src/zookeeper/group.cpp (lines 154 - 159)


Some places we refer to `ZK` as in Zookeeper. Other places we refer to the 
handle `zk` as in the variable.
This introduces a third `Zk`. Can we keep the code consistent with just the 
2 names above?
We could say either the `ZK handle` or the ``zk` handle`?
Here and elsewhere in your patch.



src/zookeeper/group.cpp (lines 365 - 366)


Can we explain that a timer always exists during a fresh connection, and a 
reconnect?
Maybe we can point to a top level comment where you explain the DNS 
stale-ness problem.



src/zookeeper/group.cpp (lines 367 - 368)


Comment along these lines:
Once we are connected, we will be notified of a disconnect through the 
`reconnecting` callback, at which point we will re-establish a timer (per the 
DNS stale-ness issue).



src/zookeeper/group.cpp (line 464)


Comment along the lines of:
This assertion tests that we only receive a single `reconnecting` callback 
for the `connected -> disconnected` state transition in the zookeeper client.


- Joris Van Remoortere


On Jan. 30, 2016, 1:16 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42988/
> ---
> 
> (Updated Jan. 30, 2016, 1:16 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4546
> https://issues.apache.org/jira/browse/MESOS-4546
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous implementation of `GroupProcess` tried to establish a single
> ZooKeeper connection on startup, but didn't attempt to retry. ZooKeeper will
> retry internally, but it only retries by attempting to reconnect to a list of
> previously resolved IPs; it doesn't attempt to re-resolve those IPs to pickup
> updates to DNS configuration. Because DNS configuration can be quite dynamic,
> we now close the current Zk handle and open a new one if we've seen a
> successful `zookeeper_init` but haven't been connected within the ZooKeeper
> session timeout.
> 
> 
> Diffs
> -
> 
>   src/tests/group_tests.cpp 77349465e0163c8aa6bed6deefe3f98efb442f3d 
>   src/zookeeper/group.hpp cf82fec290a2fa9bec122539c2eb0f12b45c2fb2 
>   src/zookeeper/group.cpp 2ae3193e0e138c90b205d45400d80e80853e1b99 
>   src/zookeeper/zookeeper.cpp 3c4fdad972dcd1728c52a05970646c713dcf98c8 
> 
> Diff: https://reviews.apache.org/r/42988/diff/
> 
> 
> Testing
> ---
> 
> make check, on both OSX and Arch Linux. Manually configured a situation in 
> which the Mesos agent uses stale DNS information in a loop: validated that 
> without the patch, we don't pickup DNS changes, whereas with the patch, we do.
> 
> Also added a new unit test. Verified that the test fails w/o this patch 
> applied and passes deterministically (`gtest_repeat=100`) with the patch 
> applied.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42987: Renamed a parameter for the sake of clarity.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On Jan. 29, 2016, 11:47 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42987/
> ---
> 
> (Updated Jan. 29, 2016, 11:47 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We use several different "timeouts" in this code and the "Zk session timeout"
> has a very specific meaning in this context.
> 
> 
> Diffs
> -
> 
>   src/zookeeper/group.hpp cf82fec290a2fa9bec122539c2eb0f12b45c2fb2 
>   src/zookeeper/group.cpp 2ae3193e0e138c90b205d45400d80e80853e1b99 
>   src/zookeeper/zookeeper.hpp 573ff5bca56cbb7efb98f6eb4f1796a2bc176b5e 
>   src/zookeeper/zookeeper.cpp 3c4fdad972dcd1728c52a05970646c713dcf98c8 
> 
> Diff: https://reviews.apache.org/r/42987/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43014: Fix ShasumTest.SHA512SimpleFile failed on centos7.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43014]

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

- Mesos ReviewBot


On Jan. 30, 2016, 5:03 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43014/
> ---
> 
> (Updated Jan. 30, 2016, 5:03 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Vinod Kone.
> 
> 
> Bugs: MESOS-4556
> https://issues.apache.org/jira/browse/MESOS-4556
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ShasumTest.SHA512SimpleFile failed on centos7.
> 
> 
> Diffs
> -
> 
>   src/common/command_utils.cpp 722d0b42a995a6a5a6cfd08f3c356dabd1080cd1 
>   src/tests/common/command_utils_tests.cpp 
> e81c724fc28717f5cb1849d75a066b6da713cd44 
> 
> Diff: https://reviews.apache.org/r/43014/diff/
> 
> 
> Testing
> ---
> 
> # check in CentOS 7.
> sudo ./bin/mesos-tests.sh --gtest_filter="ShasumTest.SHA512SimpleFile"
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

[Joerg Schad]

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



Thanks for fixing this! Is there a ticket with the issue? I will do a more 
complete review tomorrow.


src/docker/executor.cpp (line 135)


I am not sure why we are printing this, but shouldn't we at least check for 
errors on os::shell?



src/slave/containerizer/docker.cpp (line 522)


Just for  my understanding: We assume the docker containerizer to only run 
on linux? Maybe we could add a short comment.



src/slave/containerizer/docker.cpp (line 992)


This looks like it could fit in a single line.


- Joerg Schad


On Jan. 30, 2016, 5:25 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43015/
> ---
> 
> (Updated Jan. 30, 2016, 5:25 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed persistent volumes with docker tasks.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
> 
> Diff: https://reviews.apache.org/r/43015/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 43014: Fix ShasumTest.SHA512SimpleFile failed on centos7.

[Jojy Varghese]

> On Jan. 30, 2016, 5:11 p.m., Jie Yu wrote:
> > src/common/command_utils.cpp, line 172
> > 
> >
> > On ubuntu, we don't have sha512sum but shasum (and sha512).
> > 
> > Here is what I am thinking:
> > 1) we need to introduce another command util 'which' that searches PATH 
> > and returns the full path to the command (just shelling out to 'which' 
> > command).
> > 2) Then, we use 'which' to check for shasum, sha512sum, etc., and then 
> > use the one that's available.
> 
> haosdent huang wrote:
> which ubuntu version you use, I use ubuntu 14.04. Alos could found 
> `sha512sum` in `/usr/bin/sha512sum`.

I thought `which` will work only if the command is in the path.


- Jojy


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


On Jan. 30, 2016, 5:03 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43014/
> ---
> 
> (Updated Jan. 30, 2016, 5:03 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Vinod Kone.
> 
> 
> Bugs: MESOS-4556
> https://issues.apache.org/jira/browse/MESOS-4556
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ShasumTest.SHA512SimpleFile failed on centos7.
> 
> 
> Diffs
> -
> 
>   src/common/command_utils.cpp 722d0b42a995a6a5a6cfd08f3c356dabd1080cd1 
>   src/tests/common/command_utils_tests.cpp 
> e81c724fc28717f5cb1849d75a066b6da713cd44 
> 
> Diff: https://reviews.apache.org/r/43014/diff/
> 
> 
> Testing
> ---
> 
> # check in CentOS 7.
> sudo ./bin/mesos-tests.sh --gtest_filter="ShasumTest.SHA512SimpleFile"
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 43015: Fixed persistent volumes with docker tasks.

[Timothy Chen]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Fixed persistent volumes with docker tasks.


Diffs
-

  src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
  src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
  src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 43014: Fix ShasumTest.SHA512SimpleFile failed on centos7.

[haosdent huang]

> On Jan. 30, 2016, 5:11 p.m., Jie Yu wrote:
> > src/common/command_utils.cpp, line 172
> > 
> >
> > On ubuntu, we don't have sha512sum but shasum (and sha512).
> > 
> > Here is what I am thinking:
> > 1) we need to introduce another command util 'which' that searches PATH 
> > and returns the full path to the command (just shelling out to 'which' 
> > command).
> > 2) Then, we use 'which' to check for shasum, sha512sum, etc., and then 
> > use the one that's available.

which ubuntu version you use, I use ubuntu 14.04. Alos could found `sha512sum` 
in `/usr/bin/sha512sum`.


- haosdent


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


On Jan. 30, 2016, 5:03 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43014/
> ---
> 
> (Updated Jan. 30, 2016, 5:03 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Vinod Kone.
> 
> 
> Bugs: MESOS-4556
> https://issues.apache.org/jira/browse/MESOS-4556
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ShasumTest.SHA512SimpleFile failed on centos7.
> 
> 
> Diffs
> -
> 
>   src/common/command_utils.cpp 722d0b42a995a6a5a6cfd08f3c356dabd1080cd1 
>   src/tests/common/command_utils_tests.cpp 
> e81c724fc28717f5cb1849d75a066b6da713cd44 
> 
> Diff: https://reviews.apache.org/r/43014/diff/
> 
> 
> Testing
> ---
> 
> # check in CentOS 7.
> sudo ./bin/mesos-tests.sh --gtest_filter="ShasumTest.SHA512SimpleFile"
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 43014: Fix ShasumTest.SHA512SimpleFile failed on centos7.

[Jie Yu]

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




src/common/command_utils.cpp (line 172)


On ubuntu, we don't have sha512sum but shasum (and sha512).

Here is what I am thinking:
1) we need to introduce another command util 'which' that searches PATH and 
returns the full path to the command (just shelling out to 'which' command).
2) Then, we use 'which' to check for shasum, sha512sum, etc., and then use 
the one that's available.


- Jie Yu


On Jan. 30, 2016, 5:03 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43014/
> ---
> 
> (Updated Jan. 30, 2016, 5:03 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Vinod Kone.
> 
> 
> Bugs: MESOS-4556
> https://issues.apache.org/jira/browse/MESOS-4556
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ShasumTest.SHA512SimpleFile failed on centos7.
> 
> 
> Diffs
> -
> 
>   src/common/command_utils.cpp 722d0b42a995a6a5a6cfd08f3c356dabd1080cd1 
>   src/tests/common/command_utils_tests.cpp 
> e81c724fc28717f5cb1849d75a066b6da713cd44 
> 
> Diff: https://reviews.apache.org/r/43014/diff/
> 
> 
> Testing
> ---
> 
> # check in CentOS 7.
> sudo ./bin/mesos-tests.sh --gtest_filter="ShasumTest.SHA512SimpleFile"
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 43014: Fix ShasumTest.SHA512SimpleFile failed on centos7.

[haosdent huang]

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

Review request for mesos, Jie Yu, Jojy Varghese, and Vinod Kone.


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


Repository: mesos


Description
---

Fix ShasumTest.SHA512SimpleFile failed on centos7.


Diffs
-

  src/common/command_utils.cpp 722d0b42a995a6a5a6cfd08f3c356dabd1080cd1 
  src/tests/common/command_utils_tests.cpp 
e81c724fc28717f5cb1849d75a066b6da713cd44 

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


Testing
---

# check in CentOS 7.
sudo ./bin/mesos-tests.sh --gtest_filter="ShasumTest.SHA512SimpleFile"


Thanks,

haosdent huang

Re: Review Request 42990: Suppressed AngularJS "Interpolation Error" in browser console.

[haosdent huang]

> On Jan. 30, 2016, 2:19 a.m., Mesos ReviewBot wrote:
> > Bad review!
> > 
> > Reviews applied: []
> > 
> > Error:
> > No reviewers specified. Please find a reviewer by asking on JIRA or the 
> > mailing list.

I have a patch a bit related to this. 
https://reviews.apache.org/r/41864/diff/2#index_header


- haosdent


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


On Jan. 30, 2016, 12:05 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42990/
> ---
> 
> (Updated Jan. 30, 2016, 12:05 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The "decimalFloat" filter was being applied to undefined variables causing an 
> "Interpolation Error". We now check for 'undefined' and return 0 instead of 
> applying the filter.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 
> 36865f1cd6a4518ba30cd6e64d4d7347762066ec 
> 
> Diff: https://reviews.apache.org/r/42990/diff/
> 
> 
> Testing
> ---
> 
> Verified that no errors are generated in the browser console.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 42986: Updated webui to show count for tasks in `TASK_RUNNING` state.

[haosdent huang]

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


Fix it, then Ship it!




Ship It!


src/webui/master/static/js/controllers.js (line 226)


should we also change the variable name here?


- haosdent huang


On Jan. 29, 2016, 11:29 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42986/
> ---
> 
> (Updated Jan. 29, 2016, 11:29 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4562
> https://issues.apache.org/jira/browse/MESOS-4562
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, it was showing the count for tasks in `TASK_STARTING` state.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 
> 36865f1cd6a4518ba30cd6e64d4d7347762066ec 
> 
> Diff: https://reviews.apache.org/r/42986/diff/
> 
> 
> Testing
> ---
> 
> Launched 100 sleeps tasks and verified the status in webui.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 42794: URL query string order is defined.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42794]

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

- Mesos ReviewBot


On Jan. 30, 2016, 11:28 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42794/
> ---
> 
> (Updated Jan. 30, 2016, 11:28 a.m.)
> 
> 
> Review request for mesos, Kapil Arya and Jan Schlicht.
> 
> 
> Bugs: MESOS-3317
> https://issues.apache.org/jira/browse/MESOS-3317
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> URL query string order is defined.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/http.cpp 762da9a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 66d185e 
> 
> Diff: https://reviews.apache.org/r/42794/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in http_test.cpp have been modified -
> HTTPTest.QueryEncodeDecode to check generated query strings in URL to be 
> according alphabatic order.
> URLTest.Stringification to check generated query strings in URL to be 
> according alphabatic order.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 42794: URL query string order is defined.

[Abhishek Dasgupta]

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


Ship it!




Ship It!

- Abhishek Dasgupta


On Jan. 30, 2016, 11:28 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42794/
> ---
> 
> (Updated Jan. 30, 2016, 11:28 a.m.)
> 
> 
> Review request for mesos, Kapil Arya and Jan Schlicht.
> 
> 
> Bugs: MESOS-3317
> https://issues.apache.org/jira/browse/MESOS-3317
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> URL query string order is defined.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/http.cpp 762da9a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 66d185e 
> 
> Diff: https://reviews.apache.org/r/42794/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in http_test.cpp have been modified -
> HTTPTest.QueryEncodeDecode to check generated query strings in URL to be 
> according alphabatic order.
> URLTest.Stringification to check generated query strings in URL to be 
> according alphabatic order.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 42794: URL query string order is defined.

[Abhishek Dasgupta]

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

(Updated Jan. 30, 2016, 11:28 a.m.)


Review request for mesos, Kapil Arya and Jan Schlicht.


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


Repository: mesos


Description
---

URL query string order is defined.


Diffs (updated)
-

  3rdparty/libprocess/src/http.cpp 762da9a 
  3rdparty/libprocess/src/tests/http_tests.cpp 66d185e 

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


Testing
---

The following test cases in http_test.cpp have been modified -
HTTPTest.QueryEncodeDecode to check generated query strings in URL to be 
according alphabatic order.
URLTest.Stringification to check generated query strings in URL to be according 
alphabatic order.


Thanks,

Abhishek Dasgupta

Re: Review Request 42994: [3 of 7] Added a persistent volume test framework for shared volumes.

[Guangya Liu]

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




src/examples/persistent_shared_volume_framework.cpp (line 56)


Move this to 53



src/tests/persistent_shared_volume_framework_test.sh (line 1)


Seems this file does not have execute permssion


- Guangya Liu


On 一月 30, 2016, 12:26 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42994/
> ---
> 
> (Updated 一月 30, 2016, 12:26 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a persistent volume test framework for shared volumes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am fac17f4bac3b2ddda0384dec8b0ed6f960bd24d1 
>   src/examples/persistent_shared_volume_framework.cpp PRE-CREATION 
>   src/tests/examples_tests.cpp a9b685b0290f23461f43b12f05f6071fa46412a6 
>   src/tests/persistent_shared_volume_framework_test.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42994/diff/
> 
> 
> Testing
> ---
> 
> make check done.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 42996: [5 of 7] Add unit tests for sharing of resources.

[Guangya Liu]

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




src/tests/resources_tests.cpp (lines 2036 - 2043)


Why need this case? I think that it does not related to shared resource and 
already covered by CreatePersistentVolume.

You may want to test the shared cases here by creating a shared volume.


- Guangya Liu


On 一月 30, 2016, 1:35 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42996/
> ---
> 
> (Updated 一月 30, 2016, 1:35 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4324
> https://issues.apache.org/jira/browse/MESOS-4324
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add unit tests for sharing of resources.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> 6fae01fa1833ae05ec82618a4ae28ac5bd275bd5 
>   src/tests/mesos.hpp c2bae4767ee7372c796bfad44ed1e86db7dd3488 
>   src/tests/persistent_volume_tests.cpp 
> cbf2bcedd5b4c14107d3b50a74f161aa9395d7d0 
>   src/tests/resources_tests.cpp 4b25e82c13e4f46c73803f773db90f269c09c48a 
> 
> Diff: https://reviews.apache.org/r/42996/diff/
> 
> 
> Testing
> ---
> 
> make check done.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>