@Neil, when

```
rootContainerId = rootContainerId.parent();
```

protobuf would try to call `ContaienrID::Clear()` first and then perform
`ContainerID::CopyFrom`. Because the parent has been broken after
`ContainerID::Clear()`, so the `ContainerID::CopyFrom` would get an empty
value. I think this is not a bug of protobuf and we should avoid using

```
Message = Message.xx();
```

On Mon, Sep 19, 2016 at 3:42 PM, Neil Conway <neil.con...@gmail.com> wrote:

> Hi Jie,
>
> Do you have more details on what exactly the problem is here? If
> protobuf is unable to copy/merge nested messages in general, that
> seems like something that might crop up elsewhere.
>
> Perhaps we can (a) file a JIRA (ideally with a self-contained
> test-case), and/or (c) report the problem to upstream?
>
> Neil
>
> ---------- Forwarded message ----------
> From:  <ji...@apache.org>
> Date: Sat, Sep 17, 2016 at 11:27 PM
> Subject: mesos git commit: Fixed a bug in getRootContainerId due to
> protobuf copying issue.
> To: comm...@mesos.apache.org
>
>
> Repository: mesos
> Updated Branches:
>   refs/heads/master a4fd86bce -> be81a924a
>
>
> Fixed a bug in getRootContainerId due to protobuf copying issue.
>
> It looks like protobuf is not so great dealing with nesting messages
> when doing merge or copy. This patch uses an extra copy to bypass that
> issue in the protobuf.
>
> Review: https://reviews.apache.org/r/51992
>
>
> Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
> Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/be81a924
> Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/be81a924
> Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/be81a924
>
> Branch: refs/heads/master
> Commit: be81a924a9e9414ec98f8c9a87a5391dad865146
> Parents: a4fd86b
> Author: Jie Yu <yujie....@gmail.com>
> Authored: Sat Sep 17 14:22:31 2016 -0700
> Committer: Jie Yu <yujie....@gmail.com>
> Committed: Sat Sep 17 14:25:33 2016 -0700
>
> ----------------------------------------------------------------------
>  src/slave/containerizer/mesos/utils.hpp | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/be81a924/
> src/slave/containerizer/mesos/utils.hpp
> ----------------------------------------------------------------------
> diff --git a/src/slave/containerizer/mesos/utils.hpp
> b/src/slave/containerizer/mesos/utils.hpp
> index 2bb55c1..178ebf3 100644
> --- a/src/slave/containerizer/mesos/utils.hpp
> +++ b/src/slave/containerizer/mesos/utils.hpp
> @@ -27,7 +27,12 @@ static ContainerID getRootContainerId(const
> ContainerID& containerId)
>  {
>    ContainerID rootContainerId = containerId;
>    while (rootContainerId.has_parent()) {
> -    rootContainerId = rootContainerId.parent();
> +    // NOTE: Looks like protobuf does not handle copying well when
> +    // nesting message is involved, because the source and the target
> +    // point to the same object. Therefore, we create a temporary
> +    // variable and use an extra copy here.
> +    ContainerID id = rootContainerId.parent();
> +    rootContainerId = id;
>    }
>
>    return rootContainerId;
>



-- 
Best Regards,
Haosdent Huang

Reply via email to