<div dir="ltr">Hi Serge,<div><br></div><div>See embedded comments at the end.<br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jan 9, 2014 at 9:26 PM, Serge Aleynikov <span dir="ltr"><<a href="mailto:serge@aleynikov.org" target="_blank">serge@aleynikov.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><font face="arial, helvetica, sans-serif">Kenneth,</font></div><div><font face="arial, helvetica, sans-serif"><br>
</font></div><div>
<font face="arial, helvetica, sans-serif">Thank you for the thorough response! Your reasoning is appealing and I'd like to engage in the discussion on how we can move on to include this functionality in the distribution, as I believe the community will benefit greatly from being able to run an Erlang node listening on </font><span style="font-family:arial,helvetica,sans-serif">multiple</span><span style="font-family:arial,helvetica,sans-serif"> </span><span style="font-family:arial,helvetica,sans-serif">transports concurrently.</span></div>
<div><font face="arial, helvetica, sans-serif"><br></font></div><div><font face="arial, helvetica, sans-serif">The changes in the patch (<a href="https://github.com/erlang/otp/pull/121" target="_blank">https://github.com/erlang/otp/pull/121</a></font><span style="font-family:arial,helvetica,sans-serif">) cover the following areas:</span></div>
<div><font face="arial, helvetica, sans-serif"><br></font></div><div><font face="arial, helvetica, sans-serif">1. Extension of the EPMD distributed protocol </font><span style="font-family:arial,helvetica,sans-serif">(here we can safely add new set of commands while being fully backward compatible)</span></div>
<div><font face="arial, helvetica, sans-serif"> erts/doc/src/erl_dist_protocol.xml</font></div><div><font face="arial, helvetica, sans-serif"><br></font></div><div>
<font face="arial, helvetica, sans-serif">2. Modification of the EPMD daemon to support the new protocol version and to keep track of the multiple protocols/ports per node </font><span style="font-family:arial,helvetica,sans-serif">(this is also done in a backward compatible way. Nodes supporting new protocol version communicate using new spec, and old nodes communicate using old spec)</span></div>
<div><font face="arial, helvetica, sans-serif"> erts/epmd/src/epmd.h</font></div><div><font face="arial, helvetica, sans-serif"> erts/epmd/src/epmd.c</font></div><div>
<font face="arial, helvetica, sans-serif"><br></font></div><div><font face="arial, helvetica, sans-serif">3. Modification of the EPMD command-line tool to talk to the local EPMD daemon using the new protocol version </font><span style="font-family:arial,helvetica,sans-serif">(the command-line tool uses new protocol version. This is reasonable, since it's installed together with the version of VM supporting the new EPMD protocol features)</span></div>
<div><font face="arial, helvetica, sans-serif"> erts/epmd/src/epmd_cli.c</font></div><div><font face="arial, helvetica, sans-serif"> erts/epmd/src/epmd_cli.c</font></div><div>
<font face="arial, helvetica, sans-serif"> erts/epmd/src/epmd_int.h</font></div><div><font face="arial, helvetica, sans-serif"> erts/epmd/src/epmd_srv.c</font></div><div><font face="arial, helvetica, sans-serif"><br>
</font></div><div><font face="arial, helvetica, sans-serif">4. Updates of the UDS distribution example code. I believe the example in the current release is </font><span style="font-family:arial,helvetica,sans-serif">outdated (I was unable to compile it), but I patched it anyway to illustrate the usage of the new </span><span style="font-family:arial,helvetica,sans-serif">protocol)</span></div>
<div><font face="arial, helvetica, sans-serif"> lib/kernel/examples/uds_dist/src/dist_selector.erl</font></div><div><font face="arial, helvetica, sans-serif"> lib/kernel/examples/uds_dist/src/uds_dist.erl</font></div>
<div><font face="arial, helvetica, sans-serif"> lib/kernel/examples/uds_dist/src/uds_server.erl</font></div><div><font face="arial, helvetica, sans-serif"><br></font></div><div>
<font face="arial, helvetica, sans-serif">5. Change of the net_kernel transport pluggable architecture to allow support of the new EPMD protocol.</font></div><div><font face="arial, helvetica, sans-serif">5.1 Net_kernel support for the new protocol.</font></div>
<div><font face="arial, helvetica, sans-serif">5.2 TCPv4 support of the new protocol</font></div><div><font face="arial, helvetica, sans-serif">5.3 TCPv6 support of the new protocol</font></div>
<div><font face="arial, helvetica, sans-serif">5.4 TLS support of the new protocol</font></div><div><font face="arial, helvetica, sans-serif"> lib/kernel/src/net_kernel.erl</font></div>
<div><font face="arial, helvetica, sans-serif"> lib/kernel/src/dist_util.erl</font></div><div><font face="arial, helvetica, sans-serif"> lib/kernel/src/inet6_tcp_dist.erl</font></div>
<div><font face="arial, helvetica, sans-serif"> lib/kernel/src/inet_tcp_dist.erl</font></div><div><font face="arial, helvetica, sans-serif"> lib/kernel/src/net_kernel.erl</font></div>
<div><font face="arial, helvetica, sans-serif"> lib/ssl/src/inet_tls_dist.erl</font></div><div><font face="arial, helvetica, sans-serif"> lib/ssl/src/ssl_tls_dist_proxy.erl</font></div>
<div><font face="arial, helvetica, sans-serif"> lib/kernel/doc/src/net_kernel.xml</font></div><div><font face="arial, helvetica, sans-serif"> </font></div><div>
<font face="arial, helvetica, sans-serif">It seems to me that we could roll it out in two phases:</font></div><div><font face="arial, helvetica, sans-serif"><br></font></div><div>
<font face="arial, helvetica, sans-serif">1. Implement 1,2,3 in the first phase. This would allow to deploy a version with EPMD supporting old and new protocols.</font></div><div><font face="arial, helvetica, sans-serif">2. Implement changes to net_kernel and transports (4,5) to be able to talk to new EPMDs </font></div>
<div><font face="arial, helvetica, sans-serif"><br></font></div><div><font face="arial, helvetica, sans-serif">If agreed, I can break up my patch in two, corresponding to this plan.</font></div>
<div><font face="arial, helvetica, sans-serif"><br></font></div><div><font face="arial, helvetica, sans-serif">What do you think?</font></div></div></div></blockquote><div><br></div><div>I think it is a good idea to roll out this in 2 or more phases but I think we need think a bit more about this before we can give a more feedback on exactly what to include in the different phases.</div>
<div><br></div><div>Will be back with more comments/suggestions soon.</div><div><br></div><div>Regards Kenneth </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">
<div><span class="HOEnZb"><font color="#888888"><div><font face="arial, helvetica, sans-serif"><br>
</font></div><div><font face="arial, helvetica, sans-serif">Serge</font></div><div style="font-family:arial,helvetica,sans-serif"><br></div></font></span></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra">
<br><br><div class="gmail_quote">
On Thu, Jan 9, 2014 at 10:53 AM, Kenneth Lundin <span dir="ltr"><<a href="mailto:kenneth.lundin@gmail.com" target="_blank">kenneth.lundin@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jan 3, 2014 at 4:30 PM, Serge Aleynikov <span dir="ltr"><<a href="mailto:serge@aleynikov.org" target="_blank">serge@aleynikov.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div style="font-family:arial,helvetica,sans-serif">
I'd like to inquire the status of the following pull request:</div><div style="font-family:arial,helvetica,sans-serif">
<a href="https://github.com/erlang/otp/pull/121" target="_blank">https://github.com/erlang/otp/pull/121</a><br></div><div style="font-family:arial,helvetica,sans-serif"><br></div><div style="font-family:arial,helvetica,sans-serif">
Has it been reviewed?</div><div style="font-family:arial,helvetica,sans-serif"><br></div><div style="font-family:arial,helvetica,sans-serif">Thanks!</div><span><font color="#888888"><div style="font-family:arial,helvetica,sans-serif">
<br></div><div style="font-family:arial,helvetica,sans-serif">Serge</div></font></span></div>
<br></blockquote><span style="font-family:arial,sans-serif;font-size:12.727272033691406px">Hi Serge,</span><div style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">
We have discussed your contribution at the OTP technical board and here are the results:</div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">
<div>Inclusion in OTP</div><div>----------------------- </div><div><br></div><div>Your patch touches many source files which are central for the Erlang distribution and we feel it would require quite a lot of work from our side as well to assure that nothing in the current distribution gets broken or incompatible. It would also require quite deep involvement from us giving more detailed feedback regarding interfaces, semantics etc. We can not prioritize this at the moment so we will not include your contribution now (which means REJECT for now).</div>
<div><br></div><div>But we really want to add this kind of functionality and want to encourage you (and others) to continue the good work in this area for possible inclusion in OTP at a later stage. See more info and thoughts below.</div>
<div><br></div><div><br></div></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">General thoughts</div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">-----------------------</div>
<div style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">Enhancing/extending the Erlang distribution to support heterogenous communication protocols is something we have discussed many times and it is something that we really would like to have.</div>
<div style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">1) We would like a plugin architecture where different transport protocols can be easily plugged in without having to change the basic Erlang distribution or the VM.</div>
<div style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">2) It should be possible to run different protocols towards different nodes</div>
<div style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">3) should be possible to have different timeouts (NET_TICK_TIME) per connection</div>
<div style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">4) interesting protocols would be UDS, TCP, TLS over TCP, SCTP, ...</div>
<div style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">5) it should be possible to look up nodes in different ways and maybe not only via contacting epmd on the host name which is part of the node name. I.e having the hostname as part of the node name is not always what you want, depending on protocol to use.</div>
<div style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">....</div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">
<br></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">Your specific contribution</div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">-----------------------------------</div>
<div style="font-family:arial,sans-serif;font-size:12.727272033691406px">In your pull request you have implemented fully or partly 1,2, 4, and we see it as a good start.</div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">
<br></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">A general comment is that the diffs are showing differences on many places in the code where you actually haven't changed anything except tabs, line breaks and spaces. This makes it more difficult to sort out what you have changed and added. We would like a clean diff without unnecessary touches lines which has no relevance for the new function. If a formatting change is needed or suggested it should be a separate commit/pull request.</div>
<div style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">Regarding details in your code we have not had the time to take a deeper look but from a quick glance it looks good. The questionmarks are more about how to introduce this new functionality in a safe way and what more is needed that you have not addressed so far.</div>
<div style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br></div><div style="font-family:arial,sans-serif;font-size:12.727272033691406px">
<div>Suggested way forward</div><div>------------------------------ </div><div><br></div><div><div><div>I think that one approach that would make it easier to include this kind of functionality soon would be to make minimal changes to the existing distribution code and only add or change so that it is possible to support heterogenous protocols with code that initially can live as a separate extension that don't need to be part of the OTP distro. </div>
<div>In this way the functionality can be further developed and used together with an ordinary OTP distro (unpatched) and can easily be included later (hopefully within a year).</div><div><br></div><div>Maybe you think that you already have done those minimal changes, but I suggest you take another look and see if it is possible to find a way to make minimal or now changes to net_kernel and epmd etc. which we could include and then have the rest as a separate "application"/extension project at github that can be included later.</div>
<div><br></div><div>Just some thoughts:</div><div>Maybe let the old distribution work as before and let the user choose "new_distro" in some way and will only then get to run the new code which allows heterogenous protocols another new EPMD or an EPMD written in Erlang or allow for plugins of different EPMD implementations that can run in parallell. The communication towards EPMD maybe needs to be secure as well and then it might be much easier to implement in Erlang.</div>
<div><br></div><div>I (we) are more than willing to discuss more about how to find this minimal patch which we think we can "safely" include and which makes it possible to develop and use the rest as pure extensions to OTP in a first step.</div>
</div></div><div><br></div><div style="font-family:arial;font-size:small"><span style="font-family:arial,sans-serif;font-size:12.727272033691406px">Regards Kenneth, Erlang/OTP Ericsson</span> </div></div></div></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div></div>