@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 <[email protected]> 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: <[email protected]> > Date: Sat, Sep 17, 2016 at 11:27 PM > Subject: mesos git commit: Fixed a bug in getRootContainerId due to > protobuf copying issue. > To: [email protected] > > > 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 <[email protected]> > Authored: Sat Sep 17 14:22:31 2016 -0700 > Committer: Jie Yu <[email protected]> > 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
