[jira] [Commented] (MESOS-2736) Upgrade the design of MasterInfo

["Marco Massenzio (JIRA)" <ji...@apache.org>]

    [ 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. [~vinod@twitter.com] 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": "master@10.10.123.224: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": "master@10.10.123.224: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)