[ 
https://issues.apache.org/jira/browse/MESOS-2736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14586541#comment-14586541
 ] 

Marco Massenzio commented on MESOS-2736:
----------------------------------------

Following the comments on my [patch|   ] here are a few opinions as to how to 
upgrade {{MasterInfo}}.

1. [~benjaminhindman] says: "We need to make 'ip' an optional field and replace 
it by an 'address' field which includes both the 'hostname' or 'ip' and the 
port that can be parsed by our Address."

2. [[email protected]] says: "I prefer we just add a new optional field (say 
ipAddress of type string)."

3. [~bmahler] says: "How about adding an 'Address' message, which can contain 
'ip' and 'port' for now?

{code}
message Address {
  required string ip;
  required uint32 port;

  // Later we can add 'hostname' or 'public_hostname', etc!
}
{code}

In the future, we can further use this in other protobufs to have a conistent 
representation (e.g. SlaveInfo only has 'hostname' and 'port' currently, no 
'ip'!). That way, you can add an address to MasterInfo:

{code}
message MasterInfo {
  required string id = 1;
  required uint32 ip = 2;
  required uint32 port = 3 [default = 5050];
  optional string pid = 4;
  optional string hostname = 5;

  optional Address address = 6;
}
{code}

There is also an overriding desire to use "automagic" Protobuf <--> JSON 
conversion, instead of custom {{parse}}/{{model}} functions (other existing 
ones are expected to be refactored away in due course).

My main concern is that we also spare a thought to the users of this API and 
that we consider a suitable JSON representation that is easy to parse, 
meaningful and not confusing to users.

We also have to bear in mind that, like it or not, we are stuck with the 
{{required uint32 ip}} field - much as we'd like otherwise.

Considering the above, I would suggest that the {{Address}} approach (while the 
most desirable in terms of "future-proofing" and "encapsulation") unfortunately 
would risk causing some confusion on the part of users, who would find the same 
information in two places (we should also consider the additional burden of 
keeping them both "in sync"; but that's a minor, one-off matter).

So, I like it best, but I'm afraid that, given the overriding desire to 
auto-generate the JSON, we follow the other suggestion, to add a simple 
{{optional string ip_address}} that carries the IP address (be it as a tuple of 
octets -{{10.10.123.224}}- or as an IPv6 string -{{FE80::0202:B3FF:FE1E:8329}}).

So this is what I would propose:
{code}
message MasterInfo {
  required string id = 1;
  required uint32 ip = 2;
  required uint32 port = 3 [default = 5050];
  optional string pid = 4;
  optional string hostname = 5;

  optional string ip_address = 6;
}
{code}

The JSON would look something like:
{code}
 {
  "id": "12345-abcde",
  "ip": 349876543,
  "port": 5050,
  "pid": "[email protected]:5050"
  "hostname": "master-334.mesos.myorg.com",
  "ip_address": "10.10.123.224"
}
{code}

We can document what the {{uint32 ip}} is (and maybe, even sample Java/Python 
code to decode) or just tell people **not** to use it and simply ignore it. Or 
something.

Anyone strongly objecting to this?

**NOTE** Personally, I think the original approach (and/or the use of an 
{{Address}} message) combined with us defining a "clean" JSON interface to be 
more desirable (and a preferable goal to wanting to refactor out 
{{model}}/{{parse}} custom methods) - by decoupling the JSON from our internal 
Protobufs, it would us avoid the implementation to "leak" through the API.

Here is what a "clean" JSON would look like:
{code}
 {
  "id": "12345-abcde",
  "pid": "[email protected]:5050",
  // We could in future add more info about the Master
  "quota": "supported",
  "version": "0.23",
  "status": "running", 
  "address": {
    "port": 5050,
    "hostname": "master-334.mesos.myorg.com",
    "ip": "10.10.123.224",
    "subnet": "10.10.0.0/16"
    // whatever
  }
}
{code}

This would be entirely decoupled from what we choose to implement internally.

> Upgrade the design of MasterInfo
> --------------------------------
>
>                 Key: MESOS-2736
>                 URL: https://issues.apache.org/jira/browse/MESOS-2736
>             Project: Mesos
>          Issue Type: Improvement
>            Reporter: Marco Massenzio
>            Assignee: Marco Massenzio
>
> Currently, the {{MasterInfo}} PB only supports an {{ip}} field as an 
> {{int32}}.
> Beyond making it harder (and opaque; open to subtle bugs) for languages other 
> than C/C++ to decode into an IPv4 octets, this does not allow Mesos to 
> support IPv6 Master nodes.
> We should consider ways to upgrade it in ways that permit us to support both 
> IPv4 / IPv6 nodes, and, possibly, in a way that makes it easy for languages 
> such as Java/Python that already have PB support, so could easily deserialize 
> this information.
> See also MESOS-2709 for more info.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to