You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Anand Mazumdar <ma...@gmail.com> on 2015/07/23 06:25:37 UTC
Re: Review Request 36318: Refactored framework struct in master to
support http frameworks
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36318/
-----------------------------------------------------------
(Updated July 23, 2015, 4:25 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco Massenzio, and Vinod Kone.
Changes
-------
Addressed BenM's comments to refactor the framework struct and made pid options ( see description for a list of changes ), Also splitted the original patch into smaller chunks.
Summary (updated)
-----------------
Refactored framework struct in master to support http frameworks
Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294
Repository: mesos
Description (updated)
-------
This change refactors the framework struct in master to introduce support for
http frameworks.
- pid becomes a optional field now in the framework struct.
- added optional fields for supporting http frameworks to the framework struct
- added a subcribe(...) method that
registerFramework(...)/subscribeHttpFramework(...) call into. Similar
functionality needs to be added for reregister to call into subscribe too.
Apologies for the extended diff ( I moved the framework struct to the end of
the file as it used some functionality from the master class. The method was
templated and had to implemented in the header file )
Diffs (updated)
-----
src/common/protobuf_utils.hpp 2e827a0923de83d5cf853a12435b451cc7c55891
src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882
src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2
src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec
src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0
Diff: https://reviews.apache.org/r/36318/diff/
Testing (updated)
-------
make check + tests now go in a separate patch now.
Thanks,
Anand Mazumdar
Re: Review Request 36318: Refactored framework struct in master to
support http frameworks
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36318/#review93331
-----------------------------------------------------------
Ship it!
src/master/master.cpp (line 4841)
<https://reviews.apache.org/r/36318/#comment147708>
This needs to go up before we try to send a message (or SchedulerDriverEventTest.SubscribedSchedulerFailover fails).
To be safe, I'll also remove the logic to drop when !connected, and just log a warning instead.
- Ben Mahler
On July 28, 2015, 3:47 a.m., Anand Mazumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36318/
> -----------------------------------------------------------
>
> (Updated July 28, 2015, 3:47 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco Massenzio, and Vinod Kone.
>
>
> Bugs: MESOS-2294
> https://issues.apache.org/jira/browse/MESOS-2294
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This change refactors the framework struct in master to introduce support for
> http frameworks.
> - pid becomes a optional field now in the framework struct.
> - added optional fields for supporting http frameworks to the framework struct
>
>
> Diffs
> -----
>
> src/common/http.hpp 765860fa7d0ce354320e9d293d09719be87efca0
> src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070
> src/master/master.hpp 2c924addfb4c52d3048ee6ded13ce638145cc93f
> src/master/master.cpp a8a195df07b5a97fdba7dfc5f312bbfa85a0d510
>
> Diff: https://reviews.apache.org/r/36318/diff/
>
>
> Testing
> -------
>
> make check + tests now go in a separate patch now.
>
>
> Thanks,
>
> Anand Mazumdar
>
>
Re: Review Request 36318: Refactored framework struct in master to
support http frameworks
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36318/
-----------------------------------------------------------
(Updated July 28, 2015, 3:47 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco Massenzio, and Vinod Kone.
Changes
-------
Address review comments.
Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294
Repository: mesos
Description
-------
This change refactors the framework struct in master to introduce support for
http frameworks.
- pid becomes a optional field now in the framework struct.
- added optional fields for supporting http frameworks to the framework struct
Diffs (updated)
-----
src/common/http.hpp 765860fa7d0ce354320e9d293d09719be87efca0
src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070
src/master/master.hpp 2c924addfb4c52d3048ee6ded13ce638145cc93f
src/master/master.cpp a8a195df07b5a97fdba7dfc5f312bbfa85a0d510
Diff: https://reviews.apache.org/r/36318/diff/
Testing
-------
make check + tests now go in a separate patch now.
Thanks,
Anand Mazumdar
Re: Review Request 36318: Refactored framework struct in master to
support http frameworks
Posted by Anand Mazumdar <ma...@gmail.com>.
> On July 27, 2015, 11:54 p.m., Ben Mahler wrote:
> > Nice work Anand!
> >
> > I left feedback here, but it is all addressed in the diff I sent you over email. With the diff applied this patch looks like a shippable step to me. Note that per the comments, I also fenced off addFramework for http schedulers (much like you've already done here for failoverFramework). In both of these, we'll need to set up a readerClosed callback (the equivalent of link()). I also noticed that we'll need connection equality for this, so I'll get that added for you to work off of (i.e. Pipe::Writer equality should be enough).
> >
> > Can you confirm we have tickets for the following:
> >
> > * Authenticating the /call endpoint.
> > * Extending the existing framework rate limiting functionality to support http schedulers.
The Authentication JIRA already exists ( MESOS-2297 ). Would add the rate limiting JIRA.
> On July 27, 2015, 11:54 p.m., Ben Mahler wrote:
> > src/master/master.cpp, line 1653
> > <https://reviews.apache.org/r/36318/diff/9/?file=1021868#file1021868line1653>
> >
> > Can this really be a CHECK?
> >
> > E.g.
> >
> > HTTP framework F is subscribed.
> > A random 'pid'-based KILL is sent with framework id F.
> >
> > It seems that in this case, we should drop because it's not from the subscribed framework, but we'll instead fail this CHECK?
> >
> > How about:
> >
> > ```
> > if (framework.pid != from) {
> > ...
> > }
> > ```
> >
> > This will handle pid being None.
Good catch, fixed all the other occurences. My bad.
> On July 27, 2015, 11:54 p.m., Ben Mahler wrote:
> > src/master/master.cpp, line 1876
> > <https://reviews.apache.org/r/36318/diff/9/?file=1021868#file1021868line1876>
> >
> > Any reason you added a TODO in the above send, but not here? Seems we also have the `Framework*` here. I can imagine adding a CHECK to send to ensure that it's never called with (re-)registration messages when http.isSome()? Was this the concern?
The TODO was meant for the send(...) above when we did not have a Framework*.
My bad. Moved the comment to the other send(...)
- Anand
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36318/#review93165
-----------------------------------------------------------
On July 28, 2015, 3:47 a.m., Anand Mazumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36318/
> -----------------------------------------------------------
>
> (Updated July 28, 2015, 3:47 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco Massenzio, and Vinod Kone.
>
>
> Bugs: MESOS-2294
> https://issues.apache.org/jira/browse/MESOS-2294
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This change refactors the framework struct in master to introduce support for
> http frameworks.
> - pid becomes a optional field now in the framework struct.
> - added optional fields for supporting http frameworks to the framework struct
>
>
> Diffs
> -----
>
> src/common/http.hpp 765860fa7d0ce354320e9d293d09719be87efca0
> src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070
> src/master/master.hpp 2c924addfb4c52d3048ee6ded13ce638145cc93f
> src/master/master.cpp a8a195df07b5a97fdba7dfc5f312bbfa85a0d510
>
> Diff: https://reviews.apache.org/r/36318/diff/
>
>
> Testing
> -------
>
> make check + tests now go in a separate patch now.
>
>
> Thanks,
>
> Anand Mazumdar
>
>
Re: Review Request 36318: Refactored framework struct in master to
support http frameworks
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36318/#review93165
-----------------------------------------------------------
Nice work Anand!
I left feedback here, but it is all addressed in the diff I sent you over email. With the diff applied this patch looks like a shippable step to me. Note that per the comments, I also fenced off addFramework for http schedulers (much like you've already done here for failoverFramework). In both of these, we'll need to set up a readerClosed callback (the equivalent of link()). I also noticed that we'll need connection equality for this, so I'll get that added for you to work off of (i.e. Pipe::Writer equality should be enough).
Can you confirm we have tickets for the following:
* Authenticating the /call endpoint.
* Extending the existing framework rate limiting functionality to support http schedulers.
src/master/master.hpp (line 1236)
<https://reviews.apache.org/r/36318/#comment147484>
Let's take `Master*` first in both constructors?
src/master/master.hpp (line 1237)
<https://reviews.apache.org/r/36318/#comment147483>
This doesn't need the `_` anymore?
src/master/master.hpp (line 1248)
<https://reviews.apache.org/r/36318/#comment147482>
Is this helpful? :)
src/master/master.hpp (line 1259)
<https://reviews.apache.org/r/36318/#comment147432>
If we omit this, let's add a LOG(FATAL) here. However, not convinced that we need to omit this, it's pretty straightforward:
```
JSON::Object object = JSON::Protobuf(event);
return stringify(object);
```
At which point, we can stick the returns inside each case block? Or does the compiler not like that? Really hoping that it figures out that there's always a return due to the enum class :)
```
switch (contentType) {
case ContentType::PROTOBUF: {
return event.SerializeAsString();
}
case ContentType::JSON: {
JSON::Object object = JSON::Protobuf(event);
return stringify(object);
}
}
```
If not, we can stick an UNREACHABLE at the end.
src/master/master.hpp (lines 1265 - 1266)
<https://reviews.apache.org/r/36318/#comment147436>
Might as well just do the following, since we have to copy anyway?
```
auto encoder = recordio::Encoder<scheduler::Event>(serialize);
http = Http();
http.get().writer = writer;
http.get().encoder = encoder;
```
With https://issues.apache.org/jira/browse/MESOS-2757 we can make this look even cleaner:
```
auto encoder = recordio::Encoder<scheduler::Event>(serialize);
http = Http();
http->writer = writer;
http->encoder = encoder;
```
src/master/master.hpp (lines 1277 - 1279)
<https://reviews.apache.org/r/36318/#comment147443>
Don't bother logging this, since it's likely already closed (e.g. framework being removed). Alternatively, perhaps you only want to close here when 'connected' == true?
If you do want to keep the logging, let's make sure to print the framework information consistently with our other framework logging.
src/master/master.hpp (line 1326)
<https://reviews.apache.org/r/36318/#comment147447>
Space here between template and the <
src/master/master.hpp (lines 1331 - 1335)
<https://reviews.apache.org/r/36318/#comment147448>
How about pulling out the Event into an 'event' variable, s/event/encoded/, and avoiding the `writer_` temporary?
src/master/master.hpp (lines 1336 - 1338)
<https://reviews.apache.org/r/36318/#comment147446>
Let's print the framework information consistently (use `<< *this`). Also the colon typically comes before the reason, not the ID:
```
LOG(ERROR) << "Failed to send " << event << " to framework " << *this << ": streaming connection closed";
```
Also, do you want to be only sending when 'connected' is true? Are there cases where we expecting this to get called when 'connected' is false?
src/master/master.hpp (lines 1553 - 1560)
<https://reviews.apache.org/r/36318/#comment147450>
Can we place the 'pid' and 'http' together and document that exactly one of these is some?
Also, just to be sure, you're planning to support live upgrades from PID -> HTTP and vice versa?
src/master/master.hpp (lines 1574 - 1578)
<https://reviews.apache.org/r/36318/#comment147453>
This is a bit unwiedly, how about an if or just doing a ternary on the pid part of the output?
e.g.
```
stream << framework.id() << " (" << framework.info.name() << ")"
if (framework.pid.isSome()) {
stream << " at " << framework.pid.get();
}
return stream;
```
Should be easier to read?
src/master/master.cpp (line 956)
<https://reviews.apache.org/r/36318/#comment147490>
You don't need isSome here, (Option<T>, T) equality is well-defined:
https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp#L117
src/master/master.cpp (line 1653)
<https://reviews.apache.org/r/36318/#comment147455>
Can this really be a CHECK?
E.g.
HTTP framework F is subscribed.
A random 'pid'-based KILL is sent with framework id F.
It seems that in this case, we should drop because it's not from the subscribed framework, but we'll instead fail this CHECK?
How about:
```
if (framework.pid != from) {
...
}
```
This will handle pid being None.
src/master/master.cpp (line 1800)
<https://reviews.apache.org/r/36318/#comment147457>
Thanks, I added this for you, should disappear from the diff when you rebase.
src/master/master.cpp (line 1833)
<https://reviews.apache.org/r/36318/#comment147487>
You don't need isSome here, (Option<T>, T) equality is well-defined:
https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp#L117
src/master/master.cpp (lines 1840 - 1842)
<https://reviews.apache.org/r/36318/#comment147460>
Hm.. why can't you use Framework::send here? You already have a matching `Framework*` here. Ditto below.
src/master/master.cpp (line 1876)
<https://reviews.apache.org/r/36318/#comment147464>
Any reason you added a TODO in the above send, but not here? Seems we also have the `Framework*` here. I can imagine adding a CHECK to send to ensure that it's never called with (re-)registration messages when http.isSome()? Was this the concern?
src/master/master.cpp (line 2026)
<https://reviews.apache.org/r/36318/#comment147494>
You can swap these to avoid the .get() (since Option<T> != T is defined).
src/master/master.cpp (lines 2072 - 2075)
<https://reviews.apache.org/r/36318/#comment147495>
This fits on one line?
src/master/master.cpp (line 2102)
<https://reviews.apache.org/r/36318/#comment147496>
Hm.. there are other cases where you can use 'framework->send' instead of 'send', any reason why you avoided it?
src/master/master.cpp (line 2151)
<https://reviews.apache.org/r/36318/#comment147465>
Hm.. why can't we have this case drop the message? Seems like this could cause the master to crash in the following situation:
http framework F connected
stale framework F sends unregister message
Seems better to drop than to crash? You can just remove the .get() and this will handle the None case already.
src/master/master.cpp (lines 2179 - 2180)
<https://reviews.apache.org/r/36318/#comment147466>
Ditto here about dropping this instead.
src/master/master.cpp (lines 2199 - 2205)
<https://reviews.apache.org/r/36318/#comment147499>
Why the TODO? Can't we just do this?
```
if (framework->pid.isSome()) {
// Remove the framework from authenticated. This is safe because
// a framework will always reauthenticate before (re-)registering.
authenticated.erase(framework->pid.get());
} else {
CHECK_SOME(framework->http);
// Close the HTTP connection, which may already have
// been closed due to scheduler disconnection.
framework->http.get().writer.close();
}
```
src/master/master.cpp (lines 2287 - 2288)
<https://reviews.apache.org/r/36318/#comment147500>
Ditto just dropping instead.
src/master/master.cpp (lines 2342 - 2343)
<https://reviews.apache.org/r/36318/#comment147501>
Ditto dropping.
src/master/master.cpp (line 2348)
<https://reviews.apache.org/r/36318/#comment147502>
Let's just print `*framework` like the other cases:
```
LOG(WARNING)
<< "Ignoring launch tasks message for offers " << stringify(offerIds)
<< " from '" << from << "' because it is not from the"
<< " registered framework " << *framework;
```
src/master/master.cpp (lines 2897 - 2899)
<https://reviews.apache.org/r/36318/#comment147503>
Let's help people running across this by being a bit more specific, and can we use getOrElse?
```
// TODO(anand): We set 'pid' to UPID() for http frameworks
// as 'pid' was made optional in 0.24.0. In 0.25.0, we
// no longer have to set pid here for http frameowrks.
message.set_pid(framework->pid.getOrElse(UPID()));
```
src/master/master.cpp (lines 2980 - 2981)
<https://reviews.apache.org/r/36318/#comment147504>
Ditto here and everywhere else for dropping instead of check failure.
src/master/master.cpp (lines 3738 - 3765)
<https://reviews.apache.org/r/36318/#comment147507>
Why leave this broken..? This is pretty easy to fix:
```
// Send the latest framework pids to the slave.
hashset<FrameworkID> ids;
foreach (const Task& task, tasks) {
Framework* framework = getFramework(task.framework_id());
if (framework != NULL && !ids.contains(framework->id())) {
UpdateFrameworkMessage message;
message.mutable_framework_id()->MergeFrom(framework->id());
// TODO(anand): We set 'pid' to UPID() for http frameworks
// as 'pid' was made optional in 0.24.0. In 0.25.0, we
// no longer have to set pid here for http frameowrks.
message.set_pid(framework->pid.getOrElse(UPID()));
send(slave->pid, message);
ids.insert(framework->id());
}
}
```
src/master/master.cpp (lines 4763 - 4766)
<https://reviews.apache.org/r/36318/#comment147512>
Any reason not to add the same CHECK as failoverFramework, to fence this off for now?
src/master/master.cpp (line 4815)
<https://reviews.apache.org/r/36318/#comment147513>
How about we make this self-documenting:
```
CHECK_SOME(framework->pid) << "http framework failover not implemented";
```
:)
src/master/master.cpp (line 4960)
<https://reviews.apache.org/r/36318/#comment147514>
In the same vein, let's add a TODO for http here:
```
// TODO(anand): For http frameworks, close the connection.
```
Note that we can actually do this one, whereas unlink doesn't exist :)
- Ben Mahler
On July 25, 2015, 7:40 a.m., Anand Mazumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36318/
> -----------------------------------------------------------
>
> (Updated July 25, 2015, 7:40 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco Massenzio, and Vinod Kone.
>
>
> Bugs: MESOS-2294
> https://issues.apache.org/jira/browse/MESOS-2294
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This change refactors the framework struct in master to introduce support for
> http frameworks.
> - pid becomes a optional field now in the framework struct.
> - added optional fields for supporting http frameworks to the framework struct
>
>
> Diffs
> -----
>
> src/common/http.hpp 765860fa7d0ce354320e9d293d09719be87efca0
> src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070
> src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43
> src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca
>
> Diff: https://reviews.apache.org/r/36318/diff/
>
>
> Testing
> -------
>
> make check + tests now go in a separate patch now.
>
>
> Thanks,
>
> Anand Mazumdar
>
>
Re: Review Request 36318: Refactored framework struct in master to
support http frameworks
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36318/
-----------------------------------------------------------
(Updated July 25, 2015, 7:40 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco Massenzio, and Vinod Kone.
Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294
Repository: mesos
Description
-------
This change refactors the framework struct in master to introduce support for
http frameworks.
- pid becomes a optional field now in the framework struct.
- added optional fields for supporting http frameworks to the framework struct
Diffs (updated)
-----
src/common/http.hpp 765860fa7d0ce354320e9d293d09719be87efca0
src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070
src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43
src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca
Diff: https://reviews.apache.org/r/36318/diff/
Testing
-------
make check + tests now go in a separate patch now.
Thanks,
Anand Mazumdar
Re: Review Request 36318: Refactored framework struct in master to
support http frameworks
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36318/
-----------------------------------------------------------
(Updated July 25, 2015, 6:49 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco Massenzio, and Vinod Kone.
Changes
-------
Modified code to use the encoder from MESOS-3067
Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294
Repository: mesos
Description
-------
This change refactors the framework struct in master to introduce support for
http frameworks.
- pid becomes a optional field now in the framework struct.
- added optional fields for supporting http frameworks to the framework struct
Diffs (updated)
-----
src/common/http.hpp 765860fa7d0ce354320e9d293d09719be87efca0
src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070
src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43
src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca
Diff: https://reviews.apache.org/r/36318/diff/
Testing
-------
make check + tests now go in a separate patch now.
Thanks,
Anand Mazumdar
Re: Review Request 36318: Refactored framework struct in master to
support http frameworks
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36318/
-----------------------------------------------------------
(Updated July 24, 2015, 9:42 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco Massenzio, and Vinod Kone.
Changes
-------
Moving subscribeHttpFramework(...)/subscribe(...) in a separate patch. This patch only deals with the framework refactoring now.
Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294
Repository: mesos
Description (updated)
-------
This change refactors the framework struct in master to introduce support for
http frameworks.
- pid becomes a optional field now in the framework struct.
- added optional fields for supporting http frameworks to the framework struct
Diffs (updated)
-----
src/common/protobuf_utils.hpp 2e827a0923de83d5cf853a12435b451cc7c55891
src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882
src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2
src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec
src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0
Diff: https://reviews.apache.org/r/36318/diff/
Testing
-------
make check + tests now go in a separate patch now.
Thanks,
Anand Mazumdar
Re: Review Request 36318: Refactored framework struct in master to
support http frameworks
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36318/
-----------------------------------------------------------
(Updated July 23, 2015, 5:29 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco Massenzio, and Vinod Kone.
Changes
-------
Removed extra line-break + added the dependent review for moving struct to end of file (r36733)
Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294
Repository: mesos
Description
-------
This change refactors the framework struct in master to introduce support for
http frameworks.
- pid becomes a optional field now in the framework struct.
- added optional fields for supporting http frameworks to the framework struct
- added a subcribe(...) method that
registerFramework(...)/subscribeHttpFramework(...) call into. Similar
functionality needs to be added for reregister to call into subscribe too.
Diffs (updated)
-----
src/common/protobuf_utils.hpp 2e827a0923de83d5cf853a12435b451cc7c55891
src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882
src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2
src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec
src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0
Diff: https://reviews.apache.org/r/36318/diff/
Testing
-------
make check + tests now go in a separate patch now.
Thanks,
Anand Mazumdar
Re: Review Request 36318: Refactored framework struct in master to
support http frameworks
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36318/
-----------------------------------------------------------
(Updated July 23, 2015, 5:47 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco Massenzio, and Vinod Kone.
Changes
-------
rebased diff on top , to reduce the clutter of diff that was seen due to moving the struct to the end of file.
Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294
Repository: mesos
Description (updated)
-------
This change refactors the framework struct in master to introduce support for
http frameworks.
- pid becomes a optional field now in the framework struct.
- added optional fields for supporting http frameworks to the framework struct
- added a subcribe(...) method that
registerFramework(...)/subscribeHttpFramework(...) call into. Similar
functionality needs to be added for reregister to call into subscribe too.
Diffs (updated)
-----
src/common/protobuf_utils.hpp 2e827a0923de83d5cf853a12435b451cc7c55891
src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882
src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2
src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec
src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0
Diff: https://reviews.apache.org/r/36318/diff/
Testing
-------
make check + tests now go in a separate patch now.
Thanks,
Anand Mazumdar