You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Zhitao Li <zh...@gmail.com> on 2017/09/01 22:53:29 UTC
Review Request 62040: Also log attached path in agent.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62040/
-----------------------------------------------------------
Review request for mesos, Benjamin Mahler and Jason Lai.
Bugs: MESOS-7899
https://issues.apache.org/jira/browse/MESOS-7899
Repository: mesos
Description
-------
The same physical directory will be mounted to multiple logical paths,
so we also log the logical path in agent.
Diffs
-----
src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094
src/slave/slave.cpp df920ec07cd59c7ba6baccfc1c20ed3809f187d6
Diff: https://reviews.apache.org/r/62040/diff/1/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 62040: Also log attached virtual path in agent.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62040/#review184926
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/slave.cpp
Lines 939-943 (original), 947-953 (patched)
<https://reviews.apache.org/r/62040/#comment261122>
Can you open and close the quotes on the same line?
```
if (result.isReady()) {
VLOG(1) << "Successfully attached '" << path << "'"
<< " to virtual path '" << virtualPath << "'";
} else {
LOG(ERROR) << "Failed to attach '" << path << "'
<< " to virtual path '" << virtualPath << "': "
<< (result.isFailed() ? result.failure() : "discarded");
}
```
- Benjamin Mahler
On Sept. 7, 2017, 9:16 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62040/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2017, 9:16 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Jason Lai.
>
>
> Bugs: MESOS-7899
> https://issues.apache.org/jira/browse/MESOS-7899
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The same physical directory will be mounted to multiple virtual paths,
> so log which virtual path in agent log.
>
>
> Diffs
> -----
>
> src/slave/constants.hpp 435c37e28987119c4ef43c9dad61ac052020acd8
> src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094
> src/slave/slave.cpp df920ec07cd59c7ba6baccfc1c20ed3809f187d6
>
>
> Diff: https://reviews.apache.org/r/62040/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 62040: Also log attached virtual path in agent.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62040/
-----------------------------------------------------------
(Updated Sept. 11, 2017, 11:16 p.m.)
Review request for mesos, Benjamin Mahler and Jason Lai.
Bugs: MESOS-7899
https://issues.apache.org/jira/browse/MESOS-7899
Repository: mesos
Description
-------
The same physical directory will be mounted to multiple virtual paths,
so log which virtual path in agent log.
Diffs (updated)
-----
src/slave/constants.hpp 435c37e28987119c4ef43c9dad61ac052020acd8
src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094
src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd
Diff: https://reviews.apache.org/r/62040/diff/3/
Changes: https://reviews.apache.org/r/62040/diff/2-3/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 62040: Also log attached virtual path in agent.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62040/
-----------------------------------------------------------
(Updated Sept. 7, 2017, 9:16 p.m.)
Review request for mesos, Benjamin Mahler and Jason Lai.
Summary (updated)
-----------------
Also log attached virtual path in agent.
Bugs: MESOS-7899
https://issues.apache.org/jira/browse/MESOS-7899
Repository: mesos
Description (updated)
-------
The same physical directory will be mounted to multiple virtual paths,
so log which virtual path in agent log.
Diffs (updated)
-----
src/slave/constants.hpp 435c37e28987119c4ef43c9dad61ac052020acd8
src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094
src/slave/slave.cpp df920ec07cd59c7ba6baccfc1c20ed3809f187d6
Diff: https://reviews.apache.org/r/62040/diff/2/
Changes: https://reviews.apache.org/r/62040/diff/1-2/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 62040: Also log attached virtual path in agent.
Posted by Zhitao Li <zh...@gmail.com>.
> On Sept. 6, 2017, 8:57 p.m., Benjamin Mahler wrote:
> > src/slave/slave.hpp
> > Lines 377-378 (original), 377-379 (patched)
> > <https://reviews.apache.org/r/62040/diff/1/?file=1813268#file1813268line377>
> >
> > How about path and virtualPath here and "virtual path" in the logging? We probably should have picked these names in the files API as well, feel free to make that change there if you like.
Also done in https://reviews.apache.org/r/62174
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62040/#review184732
-----------------------------------------------------------
On Sept. 7, 2017, 9:16 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62040/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2017, 9:16 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Jason Lai.
>
>
> Bugs: MESOS-7899
> https://issues.apache.org/jira/browse/MESOS-7899
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The same physical directory will be mounted to multiple virtual paths,
> so log which virtual path in agent log.
>
>
> Diffs
> -----
>
> src/slave/constants.hpp 435c37e28987119c4ef43c9dad61ac052020acd8
> src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094
> src/slave/slave.cpp df920ec07cd59c7ba6baccfc1c20ed3809f187d6
>
>
> Diff: https://reviews.apache.org/r/62040/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 62040: Also log attached path in agent.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62040/#review184732
-----------------------------------------------------------
Looks good! Just a few minor items to take care of.
src/slave/slave.hpp
Lines 377-378 (original), 377-379 (patched)
<https://reviews.apache.org/r/62040/#comment260926>
How about path and virtualPath here and "virtual path" in the logging? We probably should have picked these names in the files API as well, feel free to make that change there if you like.
src/slave/slave.cpp
Lines 807 (patched)
<https://reviews.apache.org/r/62040/#comment260927>
SLAVE_LOG_VIRTUAL_PATH
Do you want to pull this up into slave/constants.hpp so that others can read it?
src/slave/slave.cpp
Line 940 (original), 947 (patched)
<https://reviews.apache.org/r/62040/#comment260928>
How about:
Successfully attached '/foo/bar' to virtual path '/bar'
We said "file" but this can be a directory too, so maybe let's say "path" or nothing?
- Benjamin Mahler
On Sept. 1, 2017, 10:53 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62040/
> -----------------------------------------------------------
>
> (Updated Sept. 1, 2017, 10:53 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Jason Lai.
>
>
> Bugs: MESOS-7899
> https://issues.apache.org/jira/browse/MESOS-7899
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The same physical directory will be mounted to multiple logical paths,
> so we also log the logical path in agent.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 7d07868451e93d34ba694d40216c1e4036fd4094
> src/slave/slave.cpp df920ec07cd59c7ba6baccfc1c20ed3809f187d6
>
>
> Diff: https://reviews.apache.org/r/62040/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>