You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Adam B <ad...@mesosphere.io> on 2014/03/13 08:51:53 UTC
Review Request 19176: Eliminated Framework.id in favor of Framework.info.id.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19176/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Niklas Nielsen, and Vinod Kone.
Bugs: MESOS-905
https://issues.apache.org/jira/browse/MESOS-905
Repository: mesos-git
Description
-------
Eliminated the Framework.id FrameworkID field in master and slave, and replaced each with a Framework.id() accessor that references Framework.FrameworkInfo.id.
Some messages (RunTaskMessage, ExecutorRegisteredMessage) will no longer need the FrameworkID member, but it is 'required' and cannot be easily deprecated. Due to rolling upgrades, we must (a) continue to set the FrameworkID in the messages, in case the recipient is on an older version that needs it; and (b) merge the FrameworkID into the FrameworkInfo.id, in case we are receiving a message from an older sender.
Diffs
-----
src/master/http.cpp a019f15
src/master/master.hpp 49a3e15
src/master/master.cpp 2a40333
src/messages/messages.proto c26a3d0
src/slave/http.cpp 594032d
src/slave/slave.hpp 01b80df
src/slave/slave.cpp 6abb95d
Diff: https://reviews.apache.org/r/19176/diff/
Testing
-------
make check on ubuntu(Mint14)/gcc4.7.2.
Thanks,
Adam B
Re: Review Request 19176: Eliminated Framework.id in favor of
Framework.info.id.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19176/#review37364
-----------------------------------------------------------
Bad patch!
Reviews applied: [19176]
Failed command: git apply --index 19176.patch
Error:
error: patch failed: src/master/master.cpp:605
error: src/master/master.cpp: patch does not apply
error: patch failed: src/slave/slave.cpp:3016
error: src/slave/slave.cpp: patch does not apply
- Mesos ReviewBot
On March 13, 2014, 11:27 p.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19176/
> -----------------------------------------------------------
>
> (Updated March 13, 2014, 11:27 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-905
> https://issues.apache.org/jira/browse/MESOS-905
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Eliminated the Framework.id FrameworkID field in master and slave, and replaced each with a Framework.id() accessor that references Framework.FrameworkInfo.id.
> Some messages (RunTaskMessage, ExecutorRegisteredMessage) will no longer need the FrameworkID member, but it is 'required' and cannot be easily deprecated. Due to rolling upgrades, we must (a) continue to set the FrameworkID in the messages, in case the recipient is on an older version that needs it; and (b) merge the FrameworkID into the FrameworkInfo.id, in case we are receiving a message from an older sender.
>
>
> Diffs
> -----
>
> src/master/http.cpp a019f15
> src/master/master.hpp 49a3e15
> src/master/master.cpp 2a40333
> src/messages/messages.proto c26a3d0
> src/slave/http.cpp 594032d
> src/slave/slave.hpp 01b80df
> src/slave/slave.cpp 6abb95d
>
> Diff: https://reviews.apache.org/r/19176/diff/
>
>
> Testing
> -------
>
> make check on ubuntu(Mint14)/gcc4.7.2.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 19176: Eliminated Framework.id in favor of
Framework.info.id.
Posted by Adam B <ad...@mesosphere.io>.
> On March 20, 2014, 12:27 p.m., Ben Mahler wrote:
> > Hey Adam, looks pretty good overall!
> >
> > I'm wondering if we can make the diff smaller for reviewers as we really don't want to make any mistakes in the Master/Slave code. Per my comment below, can we preserve the 'FrameworkID' member inside the 'Framework' structs? There are also some unrelated style changes in this diff that would be great to pull out and depend on in this change for easier reviews, but up to you!
Definitely. I got a bit overzealous in my cleanup. Removed the changes that are unnecessary for this review, and I can make further reviews for the rest. Thanks for reviewing!
> On March 20, 2014, 12:27 p.m., Ben Mahler wrote:
> > src/master/master.hpp, lines 658-661
> > <https://reviews.apache.org/r/19176/diff/2/?file=529300#file529300line658>
> >
> > It looks like we could really minimize the number of changes we need to make to the Master by keeping the 'id' field for convenient access. Keeping your constructor change above as is though.
> >
> > There are still two ways to get the id with this new code: via 'id()' and 'info.id()', so I'm not sure what we're gaining by introducing the id accessor?
Sure, leaving the 'id' field there, but copying from info.id() in the Framework() constructor.
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19176/#review37944
-----------------------------------------------------------
On March 28, 2014, 2:59 p.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19176/
> -----------------------------------------------------------
>
> (Updated March 28, 2014, 2:59 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-905
> https://issues.apache.org/jira/browse/MESOS-905
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Eliminated the Framework.id FrameworkID field in master and slave, and replaced each with a Framework.id() accessor that references Framework.FrameworkInfo.id.
> Removed redundant FrameworkID parameters when FrameworkInfo parameters also exist with valid contained FrameworkID.
> Some messages (RunTaskMessage, ExecutorRegisteredMessage) will no longer need the FrameworkID member, but it is 'required' and cannot be easily deprecated. Due to rolling upgrades, we must (a) continue to set the FrameworkID in the messages, in case the recipient is on an older version that needs it; and (b) merge the FrameworkID into the FrameworkInfo.id, in case we are receiving a message from an older sender.
>
>
> Diffs
> -----
>
> src/master/master.hpp 0c7c520
> src/master/master.cpp 6da7766
> src/slave/slave.hpp 01b80df
> src/slave/slave.cpp d8d3e0f
>
> Diff: https://reviews.apache.org/r/19176/diff/
>
>
> Testing
> -------
>
> make check on ubuntu(Mint14)/gcc4.7.2.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 19176: Eliminated Framework.id in favor of
Framework.info.id.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19176/#review37944
-----------------------------------------------------------
Hey Adam, looks pretty good overall!
I'm wondering if we can make the diff smaller for reviewers as we really don't want to make any mistakes in the Master/Slave code. Per my comment below, can we preserve the 'FrameworkID' member inside the 'Framework' structs? There are also some unrelated style changes in this diff that would be great to pull out and depend on in this change for easier reviews, but up to you!
src/master/master.hpp
<https://reviews.apache.org/r/19176/#comment69789>
It looks like we could really minimize the number of changes we need to make to the Master by keeping the 'id' field for convenient access. Keeping your constructor change above as is though.
There are still two ways to get the id with this new code: via 'id()' and 'info.id()', so I'm not sure what we're gaining by introducing the id accessor?
src/master/master.cpp
<https://reviews.apache.org/r/19176/#comment69783>
How about we add a little comment above here saying:
// Assign a new FrameworkID.
Also if you leave '// mutable copy', can you update it to '// Mutable copy.'?
src/slave/slave.hpp
<https://reviews.apache.org/r/19176/#comment69791>
Ditto here.
- Ben Mahler
On March 20, 2014, 7:43 a.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19176/
> -----------------------------------------------------------
>
> (Updated March 20, 2014, 7:43 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-905
> https://issues.apache.org/jira/browse/MESOS-905
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Eliminated the Framework.id FrameworkID field in master and slave, and replaced each with a Framework.id() accessor that references Framework.FrameworkInfo.id.
> Removed redundant FrameworkID parameters when FrameworkInfo parameters also exist with valid contained FrameworkID.
> Some messages (RunTaskMessage, ExecutorRegisteredMessage) will no longer need the FrameworkID member, but it is 'required' and cannot be easily deprecated. Due to rolling upgrades, we must (a) continue to set the FrameworkID in the messages, in case the recipient is on an older version that needs it; and (b) merge the FrameworkID into the FrameworkInfo.id, in case we are receiving a message from an older sender.
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 2e6a910
> src/master/hierarchical_allocator_process.hpp 3ec453a
> src/master/http.cpp 72d8e91
> src/master/master.hpp 0c7c520
> src/master/master.cpp 6da7766
> src/messages/messages.proto c26a3d0
> src/slave/http.cpp 594032d
> src/slave/slave.hpp 01b80df
> src/slave/slave.cpp d8d3e0f
> src/tests/allocator_tests.cpp 31cc836
> src/tests/allocator_zookeeper_tests.cpp 9ad0fa7
> src/tests/mesos.hpp f77fbfe
> src/tests/resource_offers_tests.cpp cf910e5
> src/tests/slave_recovery_tests.cpp 40a9599
>
> Diff: https://reviews.apache.org/r/19176/diff/
>
>
> Testing
> -------
>
> make check on ubuntu(Mint14)/gcc4.7.2.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 19176: Eliminated Framework.id in favor of
Framework.info.id.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19176/#review61953
-----------------------------------------------------------
Adam, do you still want this in? It has been inactive for a while, so how about closing it for now (we can always grab the diff/reopen when you get time to land this).
- Niklas Nielsen
On April 23, 2014, 6:55 p.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19176/
> -----------------------------------------------------------
>
> (Updated April 23, 2014, 6:55 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-905
> https://issues.apache.org/jira/browse/MESOS-905
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Eliminated the Framework.id FrameworkID field in master and slave, and replaced each with a Framework.id() accessor that references Framework.FrameworkInfo.id.
> Removed redundant FrameworkID parameters when FrameworkInfo parameters also exist with valid contained FrameworkID.
> Some messages (RunTaskMessage, ExecutorRegisteredMessage) will no longer need the FrameworkID member, but it is 'required' and cannot be easily deprecated. Due to rolling upgrades, we must (a) continue to set the FrameworkID in the messages, in case the recipient is on an older version that needs it; and (b) merge the FrameworkID into the FrameworkInfo.id, in case we are receiving a message from an older sender.
>
>
> Diffs
> -----
>
> src/master/master.hpp f567a43
> src/master/master.cpp 0335b34
> src/slave/slave.hpp 438e5b5
> src/slave/slave.cpp b673fd6
>
> Diff: https://reviews.apache.org/r/19176/diff/
>
>
> Testing
> -------
>
> make check on ubuntu(Mint14)/gcc4.7.2.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 19176: Eliminated Framework.id in favor of
Framework.info.id.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19176/#review41499
-----------------------------------------------------------
Hey Adam, it looks like you're trying to also not rely on FrameworkID in RunTaskMessage?
If so, are you planning to remove it? I would say let's assume it is set for now, because this requires 3 phases anyway:
1. Version N: Move 'required' to 'optional', keep setting it.
2. Version N+1: Keep as 'optional', keep setting it. Handle it being unset.
3. Version N+2: (Now N-1 has 'optional' and can handle unset), remove the field.
Since we're currently aiming at Version N, let's keep assuming it is set when the slave receives the message.
src/master/master.cpp
<https://reviews.apache.org/r/19176/#comment74943>
Do you need this utils::copy()?
FrameworkInfo frameworkInfo_ = frameworkInfo;
Looks like the copy constructor would be used either way, anything I'm missing here?
src/slave/slave.cpp
<https://reviews.apache.org/r/19176/#comment74948>
This is pretty concerning! :(
Please follow up on MESOS-906.
src/slave/slave.cpp
<https://reviews.apache.org/r/19176/#comment74955>
Are you looking to remove the redundant FrameworkID from the RunTaskMessage? If so, please leave a TODO in messages.proto. Let's try to make it clear in the code that we were doing a deprecation! How are you planning to do this since frameworkId is required?
Otherwise, no need to avoid using the passed in 'frameworkId'.
src/slave/slave.cpp
<https://reviews.apache.org/r/19176/#comment74949>
utils::copy not needed?
src/slave/slave.cpp
<https://reviews.apache.org/r/19176/#comment74956>
This seems unnecessary given the if condition?
src/slave/slave.cpp
<https://reviews.apache.org/r/19176/#comment74958>
No need for utils::copy?
src/slave/slave.cpp
<https://reviews.apache.org/r/19176/#comment74957>
Please remove this one as well.
- Ben Mahler
On April 24, 2014, 1:55 a.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19176/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 1:55 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-905
> https://issues.apache.org/jira/browse/MESOS-905
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Eliminated the Framework.id FrameworkID field in master and slave, and replaced each with a Framework.id() accessor that references Framework.FrameworkInfo.id.
> Removed redundant FrameworkID parameters when FrameworkInfo parameters also exist with valid contained FrameworkID.
> Some messages (RunTaskMessage, ExecutorRegisteredMessage) will no longer need the FrameworkID member, but it is 'required' and cannot be easily deprecated. Due to rolling upgrades, we must (a) continue to set the FrameworkID in the messages, in case the recipient is on an older version that needs it; and (b) merge the FrameworkID into the FrameworkInfo.id, in case we are receiving a message from an older sender.
>
>
> Diffs
> -----
>
> src/master/master.hpp f567a43
> src/master/master.cpp 0335b34
> src/slave/slave.hpp 438e5b5
> src/slave/slave.cpp b673fd6
>
> Diff: https://reviews.apache.org/r/19176/diff/
>
>
> Testing
> -------
>
> make check on ubuntu(Mint14)/gcc4.7.2.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 19176: Eliminated Framework.id in favor of
Framework.info.id.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19176/#review61955
-----------------------------------------------------------
Bad patch!
Reviews applied: [19176]
Failed command: ./support/apply-review.sh -n -r 19176
Error:
2014-11-18 18:10:59 URL:https://reviews.apache.org/r/19176/diff/raw/ [6933/6933] -> "19176.patch" [1]
error: patch failed: src/master/master.hpp:614
error: src/master/master.hpp: patch does not apply
error: patch failed: src/master/master.cpp:1009
error: src/master/master.cpp: patch does not apply
error: patch failed: src/slave/slave.hpp:103
error: src/slave/slave.hpp: patch does not apply
error: patch failed: src/slave/slave.cpp:926
error: src/slave/slave.cpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On April 24, 2014, 1:55 a.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19176/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 1:55 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-905
> https://issues.apache.org/jira/browse/MESOS-905
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Eliminated the Framework.id FrameworkID field in master and slave, and replaced each with a Framework.id() accessor that references Framework.FrameworkInfo.id.
> Removed redundant FrameworkID parameters when FrameworkInfo parameters also exist with valid contained FrameworkID.
> Some messages (RunTaskMessage, ExecutorRegisteredMessage) will no longer need the FrameworkID member, but it is 'required' and cannot be easily deprecated. Due to rolling upgrades, we must (a) continue to set the FrameworkID in the messages, in case the recipient is on an older version that needs it; and (b) merge the FrameworkID into the FrameworkInfo.id, in case we are receiving a message from an older sender.
>
>
> Diffs
> -----
>
> src/master/master.hpp f567a43
> src/master/master.cpp 0335b34
> src/slave/slave.hpp 438e5b5
> src/slave/slave.cpp b673fd6
>
> Diff: https://reviews.apache.org/r/19176/diff/
>
>
> Testing
> -------
>
> make check on ubuntu(Mint14)/gcc4.7.2.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 19176: Eliminated Framework.id in favor of Framework.info.id.
Posted by Benjamin Mahler <be...@gmail.com>.
I will take a look, thanks Adam!
On Wed, Apr 23, 2014 at 6:55 PM, Adam B <ad...@mesosphere.io> wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19176/
> -----------------------------------------------------------
>
> (Updated April 23, 2014, 6:55 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Changes
> -------
>
> Rebased. Please review.
>
>
> Bugs: MESOS-905
> https://issues.apache.org/jira/browse/MESOS-905
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Eliminated the Framework.id FrameworkID field in master and slave, and
> replaced each with a Framework.id() accessor that references
> Framework.FrameworkInfo.id.
> Removed redundant FrameworkID parameters when FrameworkInfo parameters
> also exist with valid contained FrameworkID.
> Some messages (RunTaskMessage, ExecutorRegisteredMessage) will no longer
> need the FrameworkID member, but it is 'required' and cannot be easily
> deprecated. Due to rolling upgrades, we must (a) continue to set the
> FrameworkID in the messages, in case the recipient is on an older version
> that needs it; and (b) merge the FrameworkID into the FrameworkInfo.id, in
> case we are receiving a message from an older sender.
>
>
> Diffs (updated)
> -----
>
> src/master/master.hpp f567a43
> src/master/master.cpp 0335b34
> src/slave/slave.hpp 438e5b5
> src/slave/slave.cpp b673fd6
>
> Diff: https://reviews.apache.org/r/19176/diff/
>
>
> Testing
> -------
>
> make check on ubuntu(Mint14)/gcc4.7.2.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 19176: Eliminated Framework.id in favor of
Framework.info.id.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19176/
-----------------------------------------------------------
(Updated April 23, 2014, 6:55 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
Rebased. Please review.
Bugs: MESOS-905
https://issues.apache.org/jira/browse/MESOS-905
Repository: mesos-git
Description
-------
Eliminated the Framework.id FrameworkID field in master and slave, and replaced each with a Framework.id() accessor that references Framework.FrameworkInfo.id.
Removed redundant FrameworkID parameters when FrameworkInfo parameters also exist with valid contained FrameworkID.
Some messages (RunTaskMessage, ExecutorRegisteredMessage) will no longer need the FrameworkID member, but it is 'required' and cannot be easily deprecated. Due to rolling upgrades, we must (a) continue to set the FrameworkID in the messages, in case the recipient is on an older version that needs it; and (b) merge the FrameworkID into the FrameworkInfo.id, in case we are receiving a message from an older sender.
Diffs (updated)
-----
src/master/master.hpp f567a43
src/master/master.cpp 0335b34
src/slave/slave.hpp 438e5b5
src/slave/slave.cpp b673fd6
Diff: https://reviews.apache.org/r/19176/diff/
Testing
-------
make check on ubuntu(Mint14)/gcc4.7.2.
Thanks,
Adam B
Re: Review Request 19176: Eliminated Framework.id in favor of
Framework.info.id.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19176/#review38961
-----------------------------------------------------------
Patch looks great!
Reviews applied: [19176]
All tests passed.
- Mesos ReviewBot
On March 28, 2014, 9:59 p.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19176/
> -----------------------------------------------------------
>
> (Updated March 28, 2014, 9:59 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-905
> https://issues.apache.org/jira/browse/MESOS-905
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Eliminated the Framework.id FrameworkID field in master and slave, and replaced each with a Framework.id() accessor that references Framework.FrameworkInfo.id.
> Removed redundant FrameworkID parameters when FrameworkInfo parameters also exist with valid contained FrameworkID.
> Some messages (RunTaskMessage, ExecutorRegisteredMessage) will no longer need the FrameworkID member, but it is 'required' and cannot be easily deprecated. Due to rolling upgrades, we must (a) continue to set the FrameworkID in the messages, in case the recipient is on an older version that needs it; and (b) merge the FrameworkID into the FrameworkInfo.id, in case we are receiving a message from an older sender.
>
>
> Diffs
> -----
>
> src/master/master.hpp 0c7c520
> src/master/master.cpp 6da7766
> src/slave/slave.hpp 01b80df
> src/slave/slave.cpp d8d3e0f
>
> Diff: https://reviews.apache.org/r/19176/diff/
>
>
> Testing
> -------
>
> make check on ubuntu(Mint14)/gcc4.7.2.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 19176: Eliminated Framework.id in favor of
Framework.info.id.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19176/
-----------------------------------------------------------
(Updated March 28, 2014, 2:59 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
Simplified my overzealous changelist down to the core requirement of removing FrameworkID from the Master::Framework and Slave::Framework constructors, pulling the id from the FrameworkInfo.
Also ensured that the FrameworkInfo always has the FrameworkID set by the time we construct a Framework out of it. In Slave::runTask, we must merge the FrameworkID into the FrameworkInfo, in case we're receiving a message from an older master; and in Slave::recoverFramework we must merge in case the checkpointed state is older.
The following (removed) changes can be handled in subsequent reviews:
- Remove the redundant FrameworkID from allocator->frameworkAdded/frameworkActivated calls.
- Fix unnecessary extra looping in reregisterFramework to add the framework's tasks from each activated slave.
- Move newFramework/Id() into private scope.
- (Cannot deprecate required FrameworkID fields in RunTaskMessage/ExecutorRegisteredMessage, because required is forever.)
- Various style changes can be batched in lint cleanup reviews.
Bugs: MESOS-905
https://issues.apache.org/jira/browse/MESOS-905
Repository: mesos-git
Description
-------
Eliminated the Framework.id FrameworkID field in master and slave, and replaced each with a Framework.id() accessor that references Framework.FrameworkInfo.id.
Removed redundant FrameworkID parameters when FrameworkInfo parameters also exist with valid contained FrameworkID.
Some messages (RunTaskMessage, ExecutorRegisteredMessage) will no longer need the FrameworkID member, but it is 'required' and cannot be easily deprecated. Due to rolling upgrades, we must (a) continue to set the FrameworkID in the messages, in case the recipient is on an older version that needs it; and (b) merge the FrameworkID into the FrameworkInfo.id, in case we are receiving a message from an older sender.
Diffs (updated)
-----
src/master/master.hpp 0c7c520
src/master/master.cpp 6da7766
src/slave/slave.hpp 01b80df
src/slave/slave.cpp d8d3e0f
Diff: https://reviews.apache.org/r/19176/diff/
Testing
-------
make check on ubuntu(Mint14)/gcc4.7.2.
Thanks,
Adam B
Re: Review Request 19176: Eliminated Framework.id in favor of
Framework.info.id.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19176/#review37868
-----------------------------------------------------------
Patch looks great!
Reviews applied: [19176]
All tests passed.
- Mesos ReviewBot
On March 20, 2014, 7:43 a.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19176/
> -----------------------------------------------------------
>
> (Updated March 20, 2014, 7:43 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-905
> https://issues.apache.org/jira/browse/MESOS-905
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Eliminated the Framework.id FrameworkID field in master and slave, and replaced each with a Framework.id() accessor that references Framework.FrameworkInfo.id.
> Removed redundant FrameworkID parameters when FrameworkInfo parameters also exist with valid contained FrameworkID.
> Some messages (RunTaskMessage, ExecutorRegisteredMessage) will no longer need the FrameworkID member, but it is 'required' and cannot be easily deprecated. Due to rolling upgrades, we must (a) continue to set the FrameworkID in the messages, in case the recipient is on an older version that needs it; and (b) merge the FrameworkID into the FrameworkInfo.id, in case we are receiving a message from an older sender.
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 2e6a910
> src/master/hierarchical_allocator_process.hpp 3ec453a
> src/master/http.cpp 72d8e91
> src/master/master.hpp 0c7c520
> src/master/master.cpp 6da7766
> src/messages/messages.proto c26a3d0
> src/slave/http.cpp 594032d
> src/slave/slave.hpp 01b80df
> src/slave/slave.cpp d8d3e0f
> src/tests/allocator_tests.cpp 31cc836
> src/tests/allocator_zookeeper_tests.cpp 9ad0fa7
> src/tests/mesos.hpp f77fbfe
> src/tests/resource_offers_tests.cpp cf910e5
> src/tests/slave_recovery_tests.cpp 40a9599
>
> Diff: https://reviews.apache.org/r/19176/diff/
>
>
> Testing
> -------
>
> make check on ubuntu(Mint14)/gcc4.7.2.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 19176: Eliminated Framework.id in favor of
Framework.info.id.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19176/
-----------------------------------------------------------
(Updated March 20, 2014, 12:43 a.m.)
Review request for mesos and Ben Mahler.
Changes
-------
Rebased and removed redundant FrameworkID from Allocator::frameworkAdded/frameworkActivated.
Bugs: MESOS-905
https://issues.apache.org/jira/browse/MESOS-905
Repository: mesos-git
Description (updated)
-------
Eliminated the Framework.id FrameworkID field in master and slave, and replaced each with a Framework.id() accessor that references Framework.FrameworkInfo.id.
Removed redundant FrameworkID parameters when FrameworkInfo parameters also exist with valid contained FrameworkID.
Some messages (RunTaskMessage, ExecutorRegisteredMessage) will no longer need the FrameworkID member, but it is 'required' and cannot be easily deprecated. Due to rolling upgrades, we must (a) continue to set the FrameworkID in the messages, in case the recipient is on an older version that needs it; and (b) merge the FrameworkID into the FrameworkInfo.id, in case we are receiving a message from an older sender.
Diffs (updated)
-----
src/master/allocator.hpp 2e6a910
src/master/hierarchical_allocator_process.hpp 3ec453a
src/master/http.cpp 72d8e91
src/master/master.hpp 0c7c520
src/master/master.cpp 6da7766
src/messages/messages.proto c26a3d0
src/slave/http.cpp 594032d
src/slave/slave.hpp 01b80df
src/slave/slave.cpp d8d3e0f
src/tests/allocator_tests.cpp 31cc836
src/tests/allocator_zookeeper_tests.cpp 9ad0fa7
src/tests/mesos.hpp f77fbfe
src/tests/resource_offers_tests.cpp cf910e5
src/tests/slave_recovery_tests.cpp 40a9599
Diff: https://reviews.apache.org/r/19176/diff/
Testing
-------
make check on ubuntu(Mint14)/gcc4.7.2.
Thanks,
Adam B
Re: Review Request 19176: Eliminated Framework.id in favor of
Framework.info.id.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19176/
-----------------------------------------------------------
(Updated March 13, 2014, 11:27 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
It makes sense for bmahler to shepherd this since he's also working in the master right now to integrate the registrar. -benh
Bugs: MESOS-905
https://issues.apache.org/jira/browse/MESOS-905
Repository: mesos-git
Description
-------
Eliminated the Framework.id FrameworkID field in master and slave, and replaced each with a Framework.id() accessor that references Framework.FrameworkInfo.id.
Some messages (RunTaskMessage, ExecutorRegisteredMessage) will no longer need the FrameworkID member, but it is 'required' and cannot be easily deprecated. Due to rolling upgrades, we must (a) continue to set the FrameworkID in the messages, in case the recipient is on an older version that needs it; and (b) merge the FrameworkID into the FrameworkInfo.id, in case we are receiving a message from an older sender.
Diffs
-----
src/master/http.cpp a019f15
src/master/master.hpp 49a3e15
src/master/master.cpp 2a40333
src/messages/messages.proto c26a3d0
src/slave/http.cpp 594032d
src/slave/slave.hpp 01b80df
src/slave/slave.cpp 6abb95d
Diff: https://reviews.apache.org/r/19176/diff/
Testing
-------
make check on ubuntu(Mint14)/gcc4.7.2.
Thanks,
Adam B