You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benno Evers <be...@mesosphere.com> on 2019/12/03 18:29:41 UTC

Re: Review Request 71815: Made the default executors connect via domain sockets if available.

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

(Updated Dec. 3, 2019, 6:29 p.m.)


Review request for mesos and Benjamin Bannier.


Changes
-------

Add command executor test.


Summary (updated)
-----------------

Made the default executors connect via domain sockets if available.


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


Repository: mesos


Description
-------

Added support for the `MESOS_DOMAIN_SOCKET` environment variable
to the default executor. If this variable is present, the executor
will use the specified domain socket to connect to the agent,
as opposed to the IP address encoded in the agent PID.


Diffs (updated)
-----

  src/executor/executor.cpp b4126037ed3e14a93d4acb9d02b62115ba4ef690 
  src/launcher/default_executor.cpp 521494a2770192e39b5159624f057e01f2690c8a 
  src/tests/command_executor_tests.cpp c7f7711a7b085bdacd0f16b16a6c85cc47e2045c 
  src/tests/default_executor_tests.cpp 49c4e3b37bde848dc7a4fd02a1458c650a84a16f 


Diff: https://reviews.apache.org/r/71815/diff/3/

Changes: https://reviews.apache.org/r/71815/diff/2-3/


Testing
-------

Ran the new test.


Thanks,

Benno Evers


Re: Review Request 71815: Made the default executors connect via domain sockets if available.

Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71815/#review219209
-----------------------------------------------------------


Fix it, then Ship it!





src/executor/executor.cpp
Lines 243 (patched)
<https://reviews.apache.org/r/71815/#comment307317>

    Can you introduce a constant for this `108` and the one below? We should add documentation where this value comes from to that constant.



src/tests/command_executor_tests.cpp
Lines 511-512 (patched)
<https://reviews.apache.org/r/71815/#comment307318>

    These flags are only present and active later in the chain. If you have time you could reorder the patches so we get a clean bisect, but I don't think that this is critical.


- Benjamin Bannier


On Jan. 10, 2020, 2:42 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71815/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2020, 2:42 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-10039
>     https://issues.apache.org/jira/browse/MESOS-10039
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added support for the `MESOS_DOMAIN_SOCKET` environment variable
> to the default executor. If this variable is present, the executor
> will use the specified domain socket to connect to the agent,
> as opposed to the IP address encoded in the agent PID.
> 
> 
> Diffs
> -----
> 
>   src/executor/executor.cpp dfa58206b65a6827ad4b958879c19a093a672e80 
>   src/launcher/default_executor.cpp 521494a2770192e39b5159624f057e01f2690c8a 
>   src/tests/command_executor_tests.cpp c7f7711a7b085bdacd0f16b16a6c85cc47e2045c 
>   src/tests/default_executor_tests.cpp 49c4e3b37bde848dc7a4fd02a1458c650a84a16f 
> 
> 
> Diff: https://reviews.apache.org/r/71815/diff/4/
> 
> 
> Testing
> -------
> 
> Ran the new test.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 71815: Made the default executors connect via domain sockets if available.

Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71815/
-----------------------------------------------------------

(Updated Jan. 10, 2020, 1:42 a.m.)


Review request for mesos and Benjamin Bannier.


Changes
-------

Used new `relative_path()` function from stout.


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


Repository: mesos


Description
-------

Added support for the `MESOS_DOMAIN_SOCKET` environment variable
to the default executor. If this variable is present, the executor
will use the specified domain socket to connect to the agent,
as opposed to the IP address encoded in the agent PID.


Diffs (updated)
-----

  src/executor/executor.cpp dfa58206b65a6827ad4b958879c19a093a672e80 
  src/launcher/default_executor.cpp 521494a2770192e39b5159624f057e01f2690c8a 
  src/tests/command_executor_tests.cpp c7f7711a7b085bdacd0f16b16a6c85cc47e2045c 
  src/tests/default_executor_tests.cpp 49c4e3b37bde848dc7a4fd02a1458c650a84a16f 


Diff: https://reviews.apache.org/r/71815/diff/4/

Changes: https://reviews.apache.org/r/71815/diff/3-4/


Testing
-------

Ran the new test.


Thanks,

Benno Evers